[YSQL] INSERT ON CONFLICT read batching stale map can give wrong results

[jasonyb]

Jira Link: DB-15586

Description

When INSERT ON CONFLICT read batching is used in conjunction with other DMLs within the same statement, the results can be distorted.

Compare the output of the alternate expectfiles of yb.orig.insert_on_conflict_with.out:

• yugabyte-db/src/postgres/src/test/regress/expected/yb.orig.insert_on_conflict_with_1.out

  Lines 70 to 92 in 95e3c5f

  	WITH w(i) AS (
  	INSERT INTO with_a VALUES (11) RETURNING i
  	) INSERT INTO with_a VALUES (generate_series(10, 15)) ON CONFLICT (i) DO UPDATE SET i = EXCLUDED.i + (SELECT i FROM w);
  	TABLE with_a;
  	i
  	----
  	22
  	21
  	15
  	14
  	13
  	12
  	9
  	8
  	7
  	6
  	5
  	4
  	3
  	2
  	1
  	(15 rows)

• yugabyte-db/src/postgres/src/test/regress/expected/yb.orig.insert_on_conflict_with_2.out

  Lines 70 to 73 in 95e3c5f

  	WITH w(i) AS (
  	INSERT INTO with_a VALUES (11) RETURNING i
  	) INSERT INTO with_a VALUES (generate_series(10, 15)) ON CONFLICT (i) DO UPDATE SET i = EXCLUDED.i + (SELECT i FROM w);
  	ERROR: duplicate key value violates unique constraint "with_a_pkey"

and

• yugabyte-db/src/postgres/src/test/regress/expected/yb.orig.insert_on_conflict_with_1.out

  Lines 95 to 116 in 95e3c5f

  	WITH w(i) AS (
  	DELETE FROM with_a WHERE i = 10 RETURNING i
  	) INSERT INTO with_a VALUES (generate_series(9, 15)) ON CONFLICT (i) DO UPDATE SET i = EXCLUDED.i + (SELECT i FROM w);
  	TABLE with_a;
  	i
  	----
  	19
  	15
  	14
  	13
  	12
  	11
  	10
  	8
  	7
  	6
  	5
  	4
  	3
  	2
  	1
  	(15 rows)

• yugabyte-db/src/postgres/src/test/regress/expected/yb.orig.insert_on_conflict_with_2.out

  Lines 78 to 99 in 95e3c5f

  	WITH w(i) AS (
  	DELETE FROM with_a WHERE i = 10 RETURNING i
  	) INSERT INTO with_a VALUES (generate_series(9, 15)) ON CONFLICT (i) DO UPDATE SET i = EXCLUDED.i + (SELECT i FROM w);
  	TABLE with_a;
  	i
  	----
  	20
  	19
  	15
  	14
  	13
  	12
  	11
  	8
  	7
  	6
  	5
  	4
  	3
  	2
  	1
  	(15 rows)

At it's core, it's a caching issue. Any statements that don't use INSERT ON CONFLICT read batching will not update the INSERT ON CONFLICT maps, both the global intents map (keys that are believed to have been added this executor node) and the batch-read map (keys/slots found existing in the index from this batch's batch read). The above test has simple cases of that using a WITH clause that manipulates the table but does not involve INSERT ON CONFLICT. All DMLs that manipulate the table should also update these maps. Alternatively, the maps should be discarded on such external manipulation, and batching should be restarted with fresh reads.

Commit message a120c25 discusses this issue and a potential solution:

Change the conflict map to be per-plan instead of global. This is
necessary in case we get context-switched out during flushing phase
and interact with the conflict map of a different plan. Keep the
intent set global as we want to detect whether the same row is
inserted twice across plans. Truthfully, such detection should not
be fully global as it should not mix into plans created by triggers,
but that can be loosened in the future. Along the same lines, there
are other deficiencies with the map design such as how it is not
updated when a plan does INSERT, UPDATE, or DELETE without involving
INSERT ON CONFLICT as the map/set alteration only happens within
INSERT ON CONFLICT code. Maybe it is better to go back to design
closer to before 3f2ac8c where the
map/set updates happen in the index INSERT/DELETE/UPDATE paths to
capture all the situations. Push these finer details to be future
work.

Issue Type

kind/bug

Warning: Please confirm that this issue does not contain any sensitive information

• I confirm this issue does not contain any sensitive information.