Bug #17759: GENERATED columns not computed during MERGE

From: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Bug #17759: GENERATED columns not computed during MERGE
Date: 2023-01-29 09:57:39
Message-ID: CAEZATCXb_ezoMCcL0tzKwRGA1x0oeE=awTaysRfTPq+3wNJn8g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I spent a little time investigating bug #17759 [1] in more detail.
Initially, I thought that it had been fixed by 3f7836ff65, but it
turns out that's not the case.

[1] https://www.postgresql.org/message-id/17759-e76d9bece1b5421c%40postgresql.org

The immediate cause of the bug was that, before 3f7836ff65, the set of
generated columns to be updated depended on extraUpdatedCols from the
target RTE, and for MERGE, this was not being populated. 3f7836ff65
appeared to fix that (it fixes the test case in the bug report) by no
longer relying on rte->extraUpdatedCols, but unfortunately there's a
little more to it than that.

Since 3f7836ff65, ExecInitModifyTable() calls
ExecInitStoredGenerated() if the command is an INSERT or an UPDATE,
but not if it's a MERGE. This means that the generated column info
doesn't get built until later (when a merge action actually executes
for the first time). If the first merge action to execute is an
UPDATE, and no updated columns require generated columns to be
recomputed, then ExecInitStoredGenerated() will skip those generated
columns and not generate ri_GeneratedExprs / ri_extraUpdatedCols info
for them. That's a problem, however, since the MERGE might also
contain an INSERT that gets executed later, for which it isn't safe to
skip any of the generated columns. Here's a simple reproducer:

CREATE TABLE t (
id int PRIMARY key,
val int,
str text,
upper_str text GENERATED ALWAYS AS (upper(str)) STORED
);

INSERT INTO t VALUES (1, 10, 'orig');

MERGE INTO t
USING (VALUES (1, 100), (2, 200)) v(id, val) ON t.id = v.id
WHEN MATCHED THEN UPDATE SET val = v.val
WHEN NOT MATCHED THEN INSERT VALUES (v.id, v.val, 'new');

SELECT * FROM t;

id | val | str | upper_str
----+-----+------+-----------
1 | 100 | orig | ORIG
2 | 200 | new |
(2 rows)

So we need to ensure that ExecInitModifyTable() calls
ExecInitStoredGenerated() for MERGE. Passing CMD_MERGE to
ExecInitStoredGenerated() is good enough, since anything other than
CMD_UPDATE causes it to not skip any generated columns. That could be
improved by examining the merge action list (it would be OK to skip
generated columns as long as the MERGE didn't contain an INSERT
action), but I don't think it's worth the extra effort / risk.

So I think we need the attached in HEAD and v15.

Regards,
Dean

Attachment Content-Type Size
fix-merge-vs-generated-cols.patch text/x-patch 2.6 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Mikhail Gribkov 2023-01-29 10:19:12 Re: [PATCH] psql: Add tab-complete for optional view parameters
Previous Message wangw.fnst@fujitsu.com 2023-01-29 07:41:07 RE: Logical replication timeout problem