pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #17558: 15beta2: Endless loop with UNIQUE NULLS NOT DISTINCT and INSERT ... ON CONFLICT
5+ messages / 4 participants
[nested] [flat]

* BUG #17558: 15beta2: Endless loop with UNIQUE NULLS NOT DISTINCT and INSERT ... ON CONFLICT
@ 2022-07-25 01:04  PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 5+ messages in thread

From: PG Bug reporting form @ 2022-07-25 01:04 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: postgresql@middaysomewhere.com

The following bug has been logged on the website:

Bug reference:      17558
Logged by:          Andrew Kesper
Email address:      postgresql@middaysomewhere.com
PostgreSQL version: Unsupported/Unknown
Operating system:   Debian 11 (buster)
Description:        

In PostgreSQL 15 beta, when I try to insert a NULL value which violates a
"UNIQUE NULLS NOT DISTINCT" constraint, PostgreSQL will get stuck in an
endless loop if the command includes an "ON CONFLICT" clause. 

CREATE TABLE test (val TEXT UNIQUE NULLS NOT DISTINCT);
INSERT INTO test (val) VALUES ('a') ON CONFLICT DO NOTHING; -- inserts 'a'
INSERT INTO test (val) VALUES ('a') ON CONFLICT DO NOTHING; -- does
nothing
INSERT INTO test (val) VALUES (NULL) ON CONFLICT DO NOTHING; -- inserts
NULL
INSERT INTO test (val) VALUES (NULL) ON CONFLICT DO NOTHING; --
unresponsive

PostgreSQL will max out the CPU, complain that the WAL is growing too fast,
consume all available disk space, then crash.



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #17558: 15beta2: Endless loop with UNIQUE NULLS NOT DISTINCT and INSERT ... ON CONFLICT
@ 2022-07-25 15:39  Tom Lane <tgl@sss.pgh.pa.us>
  parent: PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 5+ messages in thread

From: Tom Lane @ 2022-07-25 15:39 UTC (permalink / raw)
  To: postgresql@middaysomewhere.com; +Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>; pgsql-bugs@lists.postgresql.org

PG Bug reporting form <noreply@postgresql.org> writes:
> In PostgreSQL 15 beta, when I try to insert a NULL value which violates a
> "UNIQUE NULLS NOT DISTINCT" constraint, PostgreSQL will get stuck in an
> endless loop if the command includes an "ON CONFLICT" clause. 

> CREATE TABLE test (val TEXT UNIQUE NULLS NOT DISTINCT);
> INSERT INTO test (val) VALUES ('a') ON CONFLICT DO NOTHING; -- inserts 'a'
> INSERT INTO test (val) VALUES ('a') ON CONFLICT DO NOTHING; -- does
> nothing
> INSERT INTO test (val) VALUES (NULL) ON CONFLICT DO NOTHING; -- inserts
> NULL
> INSERT INTO test (val) VALUES (NULL) ON CONFLICT DO NOTHING; --
> unresponsive

Yup, still a problem in HEAD.  It correctly reports an error if
you just "INSERT INTO test (val) VALUES (NULL)", but something
in the ON CONFLICT code path seems not to be on board with this.
Peter?

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #17558: 15beta2: Endless loop with UNIQUE NULLS NOT DISTINCT and INSERT ... ON CONFLICT
@ 2022-07-26 03:17  Richard Guo <guofenglinux@gmail.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 5+ messages in thread

From: Richard Guo @ 2022-07-26 03:17 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: postgresql@middaysomewhere.com, Peter Eisentraut <peter.eisentraut@enterprisedb.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>

On Mon, Jul 25, 2022 at 11:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> PG Bug reporting form <noreply@postgresql.org> writes:
> > In PostgreSQL 15 beta, when I try to insert a NULL value which violates a
> > "UNIQUE NULLS NOT DISTINCT" constraint, PostgreSQL will get stuck in an
> > endless loop if the command includes an "ON CONFLICT" clause.
>
> > CREATE TABLE test (val TEXT UNIQUE NULLS NOT DISTINCT);
> > INSERT INTO test (val) VALUES ('a') ON CONFLICT DO NOTHING; -- inserts
> 'a'
> > INSERT INTO test (val) VALUES ('a') ON CONFLICT DO NOTHING; -- does
> > nothing
> > INSERT INTO test (val) VALUES (NULL) ON CONFLICT DO NOTHING; -- inserts
> > NULL
> > INSERT INTO test (val) VALUES (NULL) ON CONFLICT DO NOTHING; --
> > unresponsive
>
> Yup, still a problem in HEAD.  It correctly reports an error if
> you just "INSERT INTO test (val) VALUES (NULL)", but something
> in the ON CONFLICT code path seems not to be on board with this.
> Peter?


Yeah, I can see the same problem. _bt_check_unique() can catch violation
of unique index constraint successfully, so insert without 'ON CONFLICT'
has no problem. But ExecCheckIndexConstraints() fails to tell the NULL
tuple violates unique constraint, and the logic below is quite
suspicious to me.

/*
 * If any of the input values are NULL, the constraint check is assumed to
 * pass (i.e., we assume the operators are strict).
 */
for (i = 0; i < indnkeyatts; i++)
{
    if (isnull[i])
        return true;
}

Thanks
Richard

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #17558: 15beta2: Endless loop with UNIQUE NULLS NOT DISTINCT and INSERT ... ON CONFLICT
@ 2022-08-02 10:19  Michael Paquier <michael@paquier.xyz>
  parent: Richard Guo <guofenglinux@gmail.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Michael Paquier @ 2022-08-02 10:19 UTC (permalink / raw)
  To: Richard Guo <guofenglinux@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; postgresql@middaysomewhere.com, Peter Eisentraut <peter.eisentraut@enterprisedb.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>

On Tue, Jul 26, 2022 at 11:17:45AM +0800, Richard Guo wrote:
> Yeah, I can see the same problem. _bt_check_unique() can catch violation
> of unique index constraint successfully, so insert without 'ON CONFLICT'
> has no problem. But ExecCheckIndexConstraints() fails to tell the NULL
> tuple violates unique constraint, and the logic below is quite
> suspicious to me.

This is still an open item assigned to Peter, and beta3 is planned for
the 11th of August.  Would a resolution by the beginning of next week
be doable?
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../Yuj6HRRD5cLgG1Os@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #17558: 15beta2: Endless loop with UNIQUE NULLS NOT DISTINCT and INSERT ... ON CONFLICT
@ 2022-08-04 17:17  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 5+ messages in thread

From: Tom Lane @ 2022-08-04 17:17 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Richard Guo <guofenglinux@gmail.com>; postgresql@middaysomewhere.com, Peter Eisentraut <peter.eisentraut@enterprisedb.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>

Michael Paquier <michael@paquier.xyz> writes:
> On Tue, Jul 26, 2022 at 11:17:45AM +0800, Richard Guo wrote:
>> Yeah, I can see the same problem. _bt_check_unique() can catch violation
>> of unique index constraint successfully, so insert without 'ON CONFLICT'
>> has no problem. But ExecCheckIndexConstraints() fails to tell the NULL
>> tuple violates unique constraint, and the logic below is quite
>> suspicious to me.

> This is still an open item assigned to Peter, and beta3 is planned for
> the 11th of August.  Would a resolution by the beginning of next week
> be doable?

Since nothing seems to be happening here, I looked into it.  I think
Richard has correctly fingered the buggy code: we need to teach
check_exclusion_or_unique_constraint() that a NULL isn't necessarily
an instant pass.  As attached.

Since the presented test case caused an uninterruptible loop, I also added
a CHECK_FOR_INTERRUPTS() that could stop it.  (I was initially pretty
surprised that we could get through an index scan without hitting any CFI,
but on reflection it's less surprising, because no index scan actually
happens: check_exclusion_or_unique_constraint() short-circuits things
before very much code has been reached.)  I'm kind of inclined to
back-patch the nodeModifyTable.c change all the way, just for safety.

			regards, tom lane

Attachments:

  [text/x-diff] fix-infinite-loop-in-ON-CONFLICT.patch (3.8K, ../../3566362.1659633468@sss.pgh.pa.us/2-fix-infinite-loop-in-ON-CONFLICT.patch)
  download | inline diff:
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index 0cb0b8f111..6a8735edf7 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -699,13 +699,19 @@ check_exclusion_or_unique_constraint(Relation heap, Relation index,
 	}
 
 	/*
-	 * If any of the input values are NULL, the constraint check is assumed to
-	 * pass (i.e., we assume the operators are strict).
+	 * If any of the input values are NULL, and the index uses the default
+	 * nulls-are-distinct mode, the constraint check is assumed to pass (i.e.,
+	 * we assume the operators are strict).  Otherwise, we interpret the
+	 * constraint as specifying IS NULL for each column whose input value is
+	 * NULL.
 	 */
-	for (i = 0; i < indnkeyatts; i++)
+	if (!indexInfo->ii_NullsNotDistinct)
 	{
-		if (isnull[i])
-			return true;
+		for (i = 0; i < indnkeyatts; i++)
+		{
+			if (isnull[i])
+				return true;
+		}
 	}
 
 	/*
@@ -717,7 +723,7 @@ check_exclusion_or_unique_constraint(Relation heap, Relation index,
 	for (i = 0; i < indnkeyatts; i++)
 	{
 		ScanKeyEntryInitialize(&scankeys[i],
-							   0,
+							   isnull[i] ? SK_ISNULL | SK_SEARCHNULL : 0,
 							   i + 1,
 							   constr_strats[i],
 							   InvalidOid,
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index a49c3da5b6..deda321502 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -956,9 +956,11 @@ ExecInsert(ModifyTableContext *context,
 			 *
 			 * We loop back here if we find a conflict below, either during
 			 * the pre-check, or when we re-check after inserting the tuple
-			 * speculatively.
+			 * speculatively.  Better allow interrupts in case some bug makes
+			 * this an infinite loop.
 			 */
 	vlock:
+			CHECK_FOR_INTERRUPTS();
 			specConflict = false;
 			if (!ExecCheckIndexConstraints(resultRelInfo, slot, estate,
 										   &conflictTid, arbiterIndexes))
diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out
index 36ccbb5f15..05b7244e4a 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -449,15 +449,16 @@ DROP TABLE UNIQUE_TBL;
 CREATE TABLE UNIQUE_TBL (i int UNIQUE NULLS NOT DISTINCT, t text);
 INSERT INTO UNIQUE_TBL VALUES (1, 'one');
 INSERT INTO UNIQUE_TBL VALUES (2, 'two');
-INSERT INTO UNIQUE_TBL VALUES (1, 'three');
+INSERT INTO UNIQUE_TBL VALUES (1, 'three');  -- fail
 ERROR:  duplicate key value violates unique constraint "unique_tbl_i_key"
 DETAIL:  Key (i)=(1) already exists.
 INSERT INTO UNIQUE_TBL VALUES (4, 'four');
 INSERT INTO UNIQUE_TBL VALUES (5, 'one');
 INSERT INTO UNIQUE_TBL (t) VALUES ('six');
-INSERT INTO UNIQUE_TBL (t) VALUES ('seven');
+INSERT INTO UNIQUE_TBL (t) VALUES ('seven');  -- fail
 ERROR:  duplicate key value violates unique constraint "unique_tbl_i_key"
 DETAIL:  Key (i)=(null) already exists.
+INSERT INTO UNIQUE_TBL (t) VALUES ('eight') ON CONFLICT DO NOTHING;  -- no-op
 SELECT * FROM UNIQUE_TBL;
  i |  t   
 ---+------
diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql
index 34de0c969a..833819a32e 100644
--- a/src/test/regress/sql/constraints.sql
+++ b/src/test/regress/sql/constraints.sql
@@ -310,11 +310,12 @@ CREATE TABLE UNIQUE_TBL (i int UNIQUE NULLS NOT DISTINCT, t text);
 
 INSERT INTO UNIQUE_TBL VALUES (1, 'one');
 INSERT INTO UNIQUE_TBL VALUES (2, 'two');
-INSERT INTO UNIQUE_TBL VALUES (1, 'three');
+INSERT INTO UNIQUE_TBL VALUES (1, 'three');  -- fail
 INSERT INTO UNIQUE_TBL VALUES (4, 'four');
 INSERT INTO UNIQUE_TBL VALUES (5, 'one');
 INSERT INTO UNIQUE_TBL (t) VALUES ('six');
-INSERT INTO UNIQUE_TBL (t) VALUES ('seven');
+INSERT INTO UNIQUE_TBL (t) VALUES ('seven');  -- fail
+INSERT INTO UNIQUE_TBL (t) VALUES ('eight') ON CONFLICT DO NOTHING;  -- no-op
 
 SELECT * FROM UNIQUE_TBL;
 =

^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2022-08-04 17:17 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-07-25 01:04 BUG #17558: 15beta2: Endless loop with UNIQUE NULLS NOT DISTINCT and INSERT ... ON CONFLICT PG Bug reporting form <noreply@postgresql.org>
2022-07-25 15:39 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-07-26 03:17   ` Richard Guo <guofenglinux@gmail.com>
2022-08-02 10:19     ` Michael Paquier <michael@paquier.xyz>
2022-08-04 17:17       ` Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox