On Thu, May 21, 2020 at 1:46 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Thu, May 21, 2020 at 9:24 AM Benny Kramek <benny@medflyt.com> wrote:
> > Tested PostgreSQL versions: 10.7, 12.2
> >
> > Hello, I have found an example where I expect to get a serialization failure
> > error, but instead receive a unique constraint error. My understanding is that
> > this is not supposed to happen starting with PostgreSQL version 9.6 because of
> > this patch:
> >
> > <https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=fcff8a575198478023ada8a48e13b50f70054766>
>
> Agreed. Thanks for the test case. I hope to look into this in a few
> days, but as a kind of Post-It note to remind me, here is your test
> case in the form of an isolation test. Clearly it hasn't quite formed
> the dangerous structure at the point the UCV is raised. Perhaps where
> that commit added the extra CheckForSerializableConflictIn() call, it
> also needs to ask the table AM to do the same sort of thing for its
> tuple? Not sure without doing some digging. That'd be similar to
> (but not the same as) the AM-assisted SSI checks I contemplated to
> make this work for exclude constraints too.
Ok I think I see what's happening here. We p-lock a page in
t_item_id_created_at_key, but not t_item_id_version_key. Then the UCV
happens on t_item_id_version_key. If you reverse the order of the
UNIQUE clauses in the CREATE statement, you get a serialization
failure instead. Perhaps to fix this we'd need a way for
_bt_insert(), having decided that it's going to raise a UCV, to run
around and tell ALL relevant indexes to perform a
CheckForSerializableConflictIn() on the pages they would touch, so
that the behaviour doesn't depend on the order we insert into indexes.
Perhaps that would fit well with the plan I mentioned on that other
thread about GIST exclusion constraints, to introduce an
"amwouldinsert()" ("amwouldhaveinserted()"?!) entry point. It'd give
AMs a chance to raise SSI errors, in error paths, to give error
reporting priority to SSI.
