Обсуждение: "insert [...] on conflict" hangs on conflict on an unmentioned gist index

Hi,

The last insert of the following statements causes a busy loop that does =
not complete:

create table gist_test (
  bar text,
  foo text,
  constraint gist_test_bar_unique unique (bar),
  constraint gist_test_foo_gist_unique exclude using gist (foo with =3D)
);
insert into gist_test (bar, foo) values ('bar', 'foo');
insert into gist_test (bar, foo) values ('baz', 'foo=E2=80=99) on =
conflict on constraint gist_test_bar_unique do nothing;

Expected behaviour is a conflict on the gist_test_foo_gist_unique =
-index.

The issue is present at least in the following PostgreSQL versions:

PostgreSQL 9.5.3 on x86_64-apple-darwin15.4.0, compiled by Apple LLVM =
version 7.3.0 (clang-703.0.31), 64-bit
PostgreSQL 9.5.3 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.4.7 =
20120313 (Red Hat 4.4.7-16), 64-bit

Best regards,

- Heikki Rauhala.=

Heikki Rauhala <heikki.rauhala@reaktor.fi> writes:
> The last insert of the following statements causes a busy loop that does not complete:

> create table gist_test (
>   bar text,
>   foo text,
>   constraint gist_test_bar_unique unique (bar),
>   constraint gist_test_foo_gist_unique exclude using gist (foo with =)
> );
> insert into gist_test (bar, foo) values ('bar', 'foo');
> insert into gist_test (bar, foo) values ('baz', 'foo’) on conflict on constraint gist_test_bar_unique do nothing;

It looks to me like the ON CONFLICT code has exactly zero chance of
working with non-btree (or more generally, non-amcanunique) indexes.
Was this case ever considered during development?

            regards, tom lane

I wrote:
> It looks to me like the ON CONFLICT code has exactly zero chance of
> working with non-btree (or more generally, non-amcanunique) indexes.

Nah, I take that back --- I was expecting this to be handled somewhere
else than it is.  It looks like the actual cause is just sloppy
handling of noDupErr in ExecInsertIndexTuples; the attached fixes it
for me.

            regards, tom lane

diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index c819d19..e73bad3 100644
*** a/src/backend/executor/execIndexing.c
--- b/src/backend/executor/execIndexing.c
*************** ExecInsertIndexTuples(TupleTableSlot *sl
*** 407,413 ****
              bool        violationOK;
              CEOUC_WAIT_MODE waitMode;

!             if (noDupErr)
              {
                  violationOK = true;
                  waitMode = CEOUC_LIVELOCK_PREVENTING_WAIT;
--- 407,413 ----
              bool        violationOK;
              CEOUC_WAIT_MODE waitMode;

!             if (noDupErr && (arbiterIndexes == NIL || arbiter))
              {
                  violationOK = true;
                  waitMode = CEOUC_LIVELOCK_PREVENTING_WAIT;

On Mon, Jul 4, 2016 at 9:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Nah, I take that back --- I was expecting this to be handled somewhere
> else than it is.  It looks like the actual cause is just sloppy
> handling of noDupErr in ExecInsertIndexTuples; the attached fixes it
> for me.

I agree that that's all this was; it's uncommon to specify an
exclusion constraint by name with DO NOTHING, so we didn't catch this
until now.

Thanks for looking into this.

--
Peter Geoghegan

> On 4.7.2016, at 20:35, Peter Geoghegan <pg@heroku.com> wrote:
>=20
> On Mon, Jul 4, 2016 at 9:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Nah, I take that back --- I was expecting this to be handled =
somewhere
>> else than it is.  It looks like the actual cause is just sloppy
>> handling of noDupErr in ExecInsertIndexTuples; the attached fixes it
>> for me.
>=20
> I agree that that's all this was; it's uncommon to specify an
> exclusion constraint by name with DO NOTHING, so we didn't catch this
> until now.

Thanks for the quick response! I tested the patch with the more complex =
query that initially brought this up, which is more like:

insert into gist_test (bar, foo) values ('baz', 'foo')
on conflict on constraint gist_test_bar_unique
  do update set foo =3D excluded.foo;

The patch works as expected also for that case.

Best regards,

- Heikki.