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

Attachment: signature.asc
Description: PGP signature

Reply via email to