Invalid SQL statement in multi-statement transaction does not abort the transaction properly

[latelatif]

If an invalid SQL statement is issued in a multi-statement transaction, the state of the transaction is set to ResultType::ABORTED and the idea is that the commit/rollback should abort the transaction. The user gets a warning when trying to run other SQL statements in the same transaction saying that the transaction is aborted and they should rollback.

On getting a rollback, the code calls AbortQueryHelper which checks for the state of the transaction and does not call abort if it is already set to ResultType::ABORTED. This results in the transaction with the invalid statement never being actually aborted.

The main reason the code waits for calling abort is that in such cases, we have to return back to the user in the context of the same transaction and hence, we cannot free up transaction context related data structures. We can get rid of the RW set and other stuff so the ideal fix is to release and give the RW set to the garbage collector as soon as one of the statements in the transaction returns failure. We can keep the transaction context till later when the user either rollbacks or commits. In either of those scenarios, we should add the transaction context data structures to the GC as well ensuring the abort of this transaction.

[latelatif]

An easy way to reproduce this issue.

CREATE TABLE
default_database=# select * from bar;
 id | year 
----+------
(0 rows)

default_database=# begin;
BEGIN
default_database=# insert into bar values(5, 400);
INSERT 0 1
default_database=# select * from bar;
 id | year 
----+------
  5 |  400
(1 row)

default_database=# insrt into bar values(5, 400);
syntax error at or near "insrt" at 1
default_database=# select * from bar;
current transaction is aborted, commands ignored until end of transaction block
default_database=# select * from bar;
current transaction is aborted, commands ignored until end of transaction block
default_database=# rollback;
ROLLBACK
default_database=# select * from bar;
 id | year 
----+------
(0 rows)

default_database=# insert into bar values(5, 400);
INSERT 0 0
default_database=# select * from bar;
 id | year 
----+------
(0 rows)

[ChTimTsubasa]

I tried these commands on the current master branch. It does not exist. The txn state stack is clean and the last two command can success.

[ChTimTsubasa]

I would add test cases to address this problem.

[ChTimTsubasa]

This issue is right about that the txn context is not explicitly aborted in the traffic cop after an invalid statement. But somehow the txn is still aborted so that another txn cannot see it. I am wondering if this is related to @poojanilangekar 's recent fix on concurrency control?

I would fix the problem to explicitly abort the traffic_cop.

[poojanilangekar]

My recent fix was for the Halloween problem #1222. So that just changed visibility in a per statement basis. Not related to what is visible across statements. Can you give me some details of where the error is within the TransactionManager?

[ChTimTsubasa]

This should be fixed in a local branch in https://github.com/tli2/peloton/tree/txn_handler, which is built on top of #1445