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
