[YSQL] Updating a row more than once in different parts of a wCTE fails

[deeps1991]

Jira Link: DB-1751
While enabling Postgres regression tests for with clause, the following test fails.

Pre-test

CREATE TABLE withz AS SELECT i AS k, (i || ' v')::text v FROM generate_series(1, 16, 3) i;
ALTER TABLE withz ADD UNIQUE (k);

WITH t AS (
    INSERT INTO withz SELECT i, 'insert'
    FROM generate_series(0, 16) i
    ON CONFLICT (k) DO UPDATE SET v = withz.v || ', now update'
    RETURNING *
)
SELECT * FROM t;

-- Test EXCLUDED.* reference within CTE
WITH aa AS (
    INSERT INTO withz VALUES(1, 5) ON CONFLICT (k) DO UPDATE SET v = EXCLUDED.v
    WHERE withz.k != EXCLUDED.k
    RETURNING *
)
SELECT * FROM aa;

Failing test:

-- Update a row more than once, in different parts of a wCTE. That is
-- an allowed, presumably very rare, edge case, but since it was
-- broken in the past, having a test seems worthwhile.
WITH simpletup AS (
  SELECT 2 k, 'Green' v),
upsert_cte AS (
  INSERT INTO withz VALUES(2, 'Blue') ON CONFLICT (k) DO
    UPDATE SET (k, v) = (SELECT k, v FROM simpletup WHERE simpletup.k = withz.k)
    RETURNING k, v)
INSERT INTO withz VALUES(2, 'Red') ON CONFLICT (k) DO
UPDATE SET (k, v) = (SELECT k, v FROM upsert_cte WHERE upsert_cte.k = withz.k)
RETURNING k, v;

The above should return 0 rows, but in YDB cluster, see the following output:

WARNING:  TupleDesc reference leak: TupleDesc 0x123ce9eb8 (16541,-1) still referenced
 k |   v
---+-------
 2 | Green
(1 row)

[jasonyb]

Found this existing issue after already doing a separate analysis:

When studying fields in EState, yb_conflict_slot looked suspicious. Came up with a multi-conflict plan that causes reference leak:

WARNING:  TupleDesc reference leak: TupleDesc 0x3653fe182208 (16399,-1) still referenced

Repro A:

drop table if exists a;
drop table if exists b;
create table a (i int unique);
create table b (i int unique);
insert into a values (1);
insert into b values (2);

EXPLAIN
with w(i) as (
    insert into a values (1) on conflict on constraint a_i_key do update set i = 10 returning i
) insert into b values (2) on conflict on constraint b_i_key do update set i = (select 20 from w);
with w(i) as (
    insert into a values (1) on conflict on constraint a_i_key do update set i = 10 returning i
) insert into b values (2) on conflict on constraint b_i_key do update set i = (select 20 from w);

Repro B:

drop table if exists a;
create table a (i int unique);
insert into a values (1), (2);

EXPLAIN
with w(i) as (
    insert into a values (1) on conflict on constraint a_i_key do update set i = 10 returning i
) insert into a values (2) on conflict on constraint a_i_key do update set i = (select 20 from w);
with w(i) as (
    insert into a values (1) on conflict on constraint a_i_key do update set i = 10 returning i
) insert into a values (2) on conflict on constraint a_i_key do update set i = (select 20 from w);

Repro C:

drop table if exists a;
create table a (i int unique);
insert into a values (1), (2), (3);

EXPLAIN
with w(i) as (
    insert into a values (1) on conflict on constraint a_i_key do update set i = 10 returning i
), x(i) as (
    insert into a values (2) on conflict on constraint a_i_key do update set i = 20 returning i
) insert into a values (3) on conflict on constraint a_i_key do update set i = (select 30 from w);
with w(i) as (
    insert into a values (1) on conflict on constraint a_i_key do update set i = 10 returning i
), x(i) as (
    insert into a values (2) on conflict on constraint a_i_key do update set i = 20 returning i
) insert into a values (3) on conflict on constraint a_i_key do update set i = (select 30 from w);

---

With repro A, I found the tuple it is complaining about is table b column i.

(gdb) p *res->attrs
$19 = {attrelid = 16397, attname = {data = "i", '\000' <repeats 62 times>}, atttypid = 23, attstattarget = -1, attlen = 4, attnum = 1, attndims = 0, attcacheoff = 0, 
  atttypmod = -1, attbyval = true, attstorage = 112 'p', attalign = 105 'i', attnotnull = false, atthasdef = false, atthasmissing = false, attidentity = 0 '\000', 
  attisdropped = false, attislocal = true, attinhcount = 0, attcollation = 0}

select 16399::regtype; and select 16397::regclass; give b.

And the explain is

                       QUERY PLAN                        
---------------------------------------------------------
 Insert on b  (cost=0.03..0.04 rows=1 width=4)
   Conflict Resolution: UPDATE
   Conflict Arbiter Indexes: b_i_key
   CTE w
     ->  Insert on a  (cost=0.00..0.01 rows=1 width=4)
           Conflict Resolution: UPDATE
           Conflict Arbiter Indexes: a_i_key
           ->  Result  (cost=0.00..0.01 rows=1 width=4)
   InitPlan 2 (returns $2)
     ->  CTE Scan on w  (cost=0.00..0.02 rows=1 width=4)
   ->  Result  (cost=0.00..0.01 rows=1 width=4)
(11 rows)

The logic is to first ExecInsert on b. The top of that function clears yb_conflict_slot. This ends up calling ExecOnConflictUpdate

• ExecInsert on b
  • estate->yb_conflict_slot = NULL
  • ExecCheckIndexConstraints
    • estate->yb_conflict_slot = existing_slot -- save b's slot
  • ExecOnConflictUpdate
    • use yb_conflict_slot
    • ExecProject
      • ...
        • ExecInsert on a
          • estate->yb_conflict_slot = NULL -- this overwrites b's slot
          • ExecCheckIndexConstraints
            • estate->yb_conflict_slot = existing_slot -- save a's slot
          • ExecOnConflictUpdate
            • use yb_conflict_slot
            • ExecProject
            • ExecUpdate
          • ExecDropSingleTupleTableSlot(estate->yb_conflict_slot)
    • ExecUpdate
  • ExecDropSingleTupleTableSlot(estate->yb_conflict_slot)

It did not look like there is an issue of cross-contamination between on-conflict-do-update plans since the slot stored into yb_conflict_slot is used soon after, but the original issue comment has a repro for wrong results. I haven't looked into it much further.

Perhaps yb_conflict_slot does not belong in EState but in a more local state.

[jasonyb]

This issue is now only about the incorrect (as in different from vanilla PG) output

 k |   v
---+-------
 2 | Green
(1 row)

TupleDesc reference leak is tracked in #23429.