Another bug revealed by SQLite, the best database engine out there. In popTransaction(), we need to reset the transaction data (->willRollback, ->rollbackLogs) as early as possible, because in case a transaction is opened by the logging callback, it will get rolled back.

This is the case for both SQLite and PostgreSQL because the default InsertQuery implement starts a transaction to improve the performance of multi-inserts.

CommentFileSizeAuthor
#2 715094-reset-transaction-status-early.patch1.61 KBDamien Tournoud
#1 715094-reset-transaction-status-early.patch1.71 KBDamien Tournoud
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Priority: Normal » Critical
Status: Active » Needs review
FileSize
715094-reset-transaction-status-early.patch1.71 KB

Raising to critical, because it breaks important core features in both SQLite and PostgreSQL.

I don't know if it is useful to test that explicitly. A few tests break on SQLite and PostgreSQL because of that, at least.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
FileSize
715094-reset-transaction-status-early.patch1.61 KB

By David on IRC: there is no need to copy the $this->willRollBack variable.

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs review » Reviewed & tested by the community

Looks logical to me. Thanks Damien.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks!

But is it possible to write tests for this?

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.