pgsql-io / multicorn2

http://multicorn2.org
Other
73 stars 16 forks source link

Fix DELETE ... RETURNING (non-pk-col) under PG14 #47

Closed mfenniak closed 1 month ago

mfenniak commented 1 month ago

When performing a statement like the below under PG14:

delete from testmulticorn_write where test1 = 'test1 1 0' returning test2, test1;

The values for test2 are coming back as null under PG14, when they would have come back with the original values under PG13 and earlier. There is a specific test case covering this in the regression test suite: https://github.com/pgsql-io/multicorn2/blob/ea5b3f0bd750c3446dfadccfa9c952da0960d4f6/test3.9/expected/write_test.out#L106-L116, which is failing under PG14 and returning:

delete from testmulticorn_write where test1 = 'test1 1 0' returning test2, test1;
NOTICE:  BEGIN
NOTICE:  [test1 = test1 1 0]
NOTICE:  ['test1']
NOTICE:  DELETING: test1 1 0
NOTICE:  PRECOMMIT
NOTICE:  COMMIT
 test2 |   test1   
-------+-----------
       | test1 1 0
(1 row)

Note two differences in this output:

The cause of this appears to be that, like in #41, the changes in PG14 don't add columns to the query plan unless add_row_identity_var is used. While multicorn attempts to add the required columns that are referenced in the RETURNING clause, this appears to be insufficient in PG14 (https://github.com/pgsql-io/multicorn2/blob/ea5b3f0bd750c3446dfadccfa9c952da0960d4f6/src/multicorn.c#L610-L616).

The attached patch fixes this by using add_row_identity_var on the fields required from the returning clause. This doesn't seem 100% right because these values don't represent the "row identity"? But this causes them to be queried, which causes them to be available in the planSlot variable during the delete, which then causes them to be returned from multicornExecForeignDelete.

This is one of two test failures on PG14. The other one is very closely related, but with UPDATE statements; but there are two potential paths to fix that one so I'm going to open an issue and request feedback before proceeding.