On Wed, Mar 03, 2021 at 09:06:11AM +0530, Amit Kapila wrote: > Michael, I want to push this patch soon unless you have any concerns.
No concerns from me... > This is required for correctness if any plugin uses 2PC and origins to > track the progress. Now, that we have exposed the two_phase option via > a plugin, it is good if we get this in as well. But I find the way of doing this feature integration strange. The patch posted on this thread is 0002 in the patch series posted in [1], and it seems to me that the test 020_twophase.pl in 0005 requires 0001~0004 to work properly, while 022_twophase_cascade.pl from 0005 needs 0006 on top of the rest. So taken as a whole, this would work, but applied independently, patches may or may not fail a portion of the tests. It seems to me that the tests should be grouped with the features they apply with, in a single commit. That's perhaps not enough to be an actual objection, but I am not really comfortable with the concept of pushing into the tree code that would remain untested until all the remaining pieces get done, as that means that we have a point in the code history where some code is around, but sits around as idle, no? If this feature is not completed within the dev cycle, it would mean that some code remains around without any actual way to know if it is useful or not, released as such. [1]: https://www.postgresql.org/message-id/cafpthdz2rigof0om0obhv1yrmymo5-sqft9fclyj-jp9shx...@mail.gmail.com -- Michael
signature.asc
Description: PGP signature