On Wed, 23 Sep 2026, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> I'm thinking now that making ExecCheckIndexConstraints() require
> estate->es_snapshot was actually an unintended change of the kind that
> probably wasn't safe for back-patching, because it might affect
> extension code.
I forgot to mention this, but the issue I mentioned only exists in 18
and 19, earlier branches are not affected (at least without
extensions).
> Perhaps, instead, we should make it only do the re-fetch if
> estate->es_snapshot is not InvalidSnapshot. For logical replication,
> it actually seems unnecessary anyway, since it's only replaying an
> update that has already been successfully committed by the publisher.
That would work in 18 and 19, but is it the good solution for later
with subscriber side conflict resolution?
> I wonder if it was actually intentional that logical replication
> workers don't set es_snapshot on the EState, since they don't use it
> to scan tables, so estate->es_snapshot is not needed.
In that case, we should add least add a comment there, because right
now it contradicts the comment in CreateExecutorState without any
explanation
