On Fri, Aug 14, 2020 at 11:02:35AM +0200, Julien Rouhaud wrote: > On Fri, Aug 14, 2020 at 04:37:46PM +0900, Michael Paquier wrote: >> + /* >> + * XXX For deterministic transaction, se should only track the >> version >> + * if the AM relies on a stable ordering. >> + */ >> + if (determ_colls) >> + { >> + /* XXX check if the AM relies on a stable ordering */ >> + recordDependencyOnCollations(&myself, determ_colls, true); >> Some cleanup needed here? Wouldn't it be better to address the issues >> with stable ordering first? > > Didn't we just agreed 3 mails ago to *not* take care of that in this patch, > and > add an extensible solution for that later? I kept the XXX comment to make it > extra clear that this will be addressed.
FWIW, I tend to prefer the approach where we put in place the necessary infrastructure first, and then have a feature rely on what we think is the most correct. This way, we avoid having any moment in the code history where we have something that we know from the start is not covered. The patch set needs a rebase. There are conflicts coming at least from pg_depend.c where I switched the code to use multi-INSERTs for catalog insertions. -- Michael
signature.asc
Description: PGP signature