Hi Stas,

> Reading through patch I’ve noticed that you deleted call to 
> SnapBuildCommitTxn()
> in DecodePrepare(). As you correctly spotted upthread there was unnecessary
> code that marked transaction as running after decoding of prepare. However 
> call
> marking it as committed before decoding of prepare IMHO is still needed as
> SnapBuildCommitTxn does some useful thing like setting base snapshot for 
> parent
> transactions which were skipped because of SnapBuildXactNeedsSkip().
> E.g. current code will crash in assert for following transaction:
> CREATE TABLE test_prepared_savepoints (a int);
> :get_with2pc_nofilter
> :get_with2pc_nofilter  <- second call will crash decoder

Thanks for taking a look!

The first ":get_with2pc_nofilter" call consumes the data appropriately.

The second ":get_with2pc_nofilter" sees that it has to skip and hence
enters the ReorderBufferForget() function in the skip code path
causing the assert. If we have to skip anyways why do we need to setup
SnapBuildCommitTxn() for such a transaction is my query? I don't see
the need for doing that for skipped transactions..

Will continue to look at this and will add this scenario to the test
cases. Further comments/feedback appreciated.

 Nikhil Sontakke                   http://www.2ndQuadrant.com/
 PostgreSQL/Postgres-XL Development, 24x7 Support, Training & Services

Reply via email to