On Fri, May 22, 2020 at 11:45 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> 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.
New thought: can we use the recently invented errsave() mechanism for
this? (Or some other error saving/deferring/rethrowing technology.)
The goal is to give UCV a lower priority than SSI violation, across
all indexes involved. So perhaps the ereport(ERROR, ... "duplicate
key value violates ...") in nbtinsert.c could become an errsave()?
Then ExecInsertIndexTuples() could loop over all inserts, giving them
all a chance to raise an SSI error, and possibly collect up to one
deferred UCV. If we make it out of the indexing loop because no SSI
(or other) error was thrown, we can then throw the UCV error, if we
collected one. But to use errsave() I suppose we'd have to pass the
ErrorSaveContext via the index AM interface...