inplace087-merge-DO-NOTHING-v8.patch
text/plain
Filename: inplace087-merge-DO-NOTHING-v8.patch
Type: text/plain
Part: 1
Message:
Re: race condition in pg_class
Patch
Same data as JSON:
GET /api/v1/attachments/:id/patch
the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes.
API reference →
Format: unified
Series: patch v8
| File | + | − |
|---|---|---|
| src/backend/executor/nodeModifyTable.c | 23 | 17 |
| src/test/regress/expected/updatable_views.out | 5 | 0 |
| src/test/regress/sql/updatable_views.sql | 5 | 0 |
Author: Noah Misch <noah@leadboat.com>
Commit: Noah Misch <noah@leadboat.com>
Fix new assertion for MERGE view_name ... DO NOTHING.
Such queries don't expand automatically updatable views, and ModifyTable
uses the wholerow attribute unconditionally. The user-visible behavior
is fine, so change to more-specific assertions. Commit
d5f788b41dc2cbdde6e7694c70dda54d829a5ed5 added the wrong assertion.
Back-patch to v17, where commit 5f2e179bd31e5f5803005101eb12a8d7bf8db8f3
introduced MERGE view_name.
Reviewed by FIXME. Reported by Alexander Lakhin.
Discussion: https://postgr.es/m/e4b40a88-c134-6926-3196-bc4501cb87a2@gmail.com
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index a2442b7..4913e49 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -24,12 +24,13 @@
* values plus row-locating info for UPDATE and MERGE cases, or just the
* row-locating info for DELETE cases.
*
- * The relation to modify can be an ordinary table, a view having an
- * INSTEAD OF trigger, or a foreign table. Earlier processing already
- * pointed ModifyTable to the underlying relations of any automatically
- * updatable view not using an INSTEAD OF trigger, so code here can
- * assume it won't have one as a modification target. This node does
- * process ri_WithCheckOptions, which may have expressions from those
+ * The relation to modify can be an ordinary table, a foreign table, or a
+ * view. If it's a view, either it has sufficient INSTEAD OF triggers or
+ * this node executes only MERGE ... DO NOTHING. If the original MERGE
+ * targeted a view not in one of those two categories, earlier processing
+ * already pointed the ModifyTable result relation to an underlying
+ * relation of that other view. This node does process
+ * ri_WithCheckOptions, which may have expressions from those other,
* automatically updatable views.
*
* MERGE runs a join between the source relation and the target table.
@@ -2726,10 +2727,10 @@ ExecMerge(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
/*-----
* If we are dealing with a WHEN MATCHED case, tupleid or oldtuple is
- * valid, depending on whether the result relation is a table or a view
- * having an INSTEAD OF trigger. We execute the first action for which
- * the additional WHEN MATCHED AND quals pass. If an action without quals
- * is found, that action is executed.
+ * valid, depending on whether the result relation is a table or a view.
+ * We execute the first action for which the additional WHEN MATCHED AND
+ * quals pass. If an action without quals is found, that action is
+ * executed.
*
* Similarly, in the WHEN NOT MATCHED BY SOURCE case, tupleid or oldtuple
* is valid, and we look at the given WHEN NOT MATCHED BY SOURCE actions
@@ -2820,8 +2821,8 @@ ExecMerge(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
* Check and execute the first qualifying MATCHED or NOT MATCHED BY SOURCE
* action, depending on whether the join quals are satisfied. If the target
* relation is a table, the current target tuple is identified by tupleid.
- * Otherwise, if the target relation is a view having an INSTEAD OF trigger,
- * oldtuple is the current target tuple from the view.
+ * Otherwise, if the target relation is a view, oldtuple is the current target
+ * tuple from the view.
*
* We start from the first WHEN MATCHED or WHEN NOT MATCHED BY SOURCE action
* and check if the WHEN quals pass, if any. If the WHEN quals for the first
@@ -2887,11 +2888,8 @@ ExecMergeMatched(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
*/
Assert(tupleid != NULL || oldtuple != NULL);
if (oldtuple != NULL)
- {
- Assert(resultRelInfo->ri_TrigDesc);
ExecForceStoreHeapTuple(oldtuple, resultRelInfo->ri_oldTupleSlot,
false);
- }
else if (!table_tuple_fetch_row_version(resultRelInfo->ri_RelationDesc,
tupleid,
SnapshotAny,
@@ -2983,6 +2981,9 @@ lmerge_matched:
}
else
{
+ /* called table_tuple_fetch_row_version() above */
+ Assert(oldtuple == NULL);
+
result = ExecUpdateAct(context, resultRelInfo, tupleid,
NULL, newslot, canSetTag,
&updateCxt);
@@ -3031,8 +3032,13 @@ lmerge_matched:
return NULL; /* "do nothing" */
}
else
+ {
+ /* called table_tuple_fetch_row_version() above */
+ Assert(oldtuple == NULL);
+
result = ExecDeleteAct(context, resultRelInfo, tupleid,
false);
+ }
if (result == TM_Ok)
{
@@ -4004,8 +4010,8 @@ ExecModifyTable(PlanState *pstate)
* know enough here to set t_tableOid. Quite separately from
* this, the FDW may fetch its own junk attrs to identify the row.
*
- * Other relevant relkinds, currently limited to views having
- * INSTEAD OF triggers, always have a wholerow attribute.
+ * Other relevant relkinds, currently limited to views, always
+ * have a wholerow attribute.
*/
else if (AttributeNumberIsValid(resultRelInfo->ri_RowIdAttNo))
{
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index 1d1f568..5a2da97 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -199,6 +199,9 @@ MERGE INTO ro_view13 AS t USING (VALUES (3, 'Row 3')) AS v(a,b) ON t.a = v.a
ERROR: cannot insert into view "ro_view13"
DETAIL: Views that do not select from a single table or view are not automatically updatable.
HINT: To enable inserting into the view using MERGE, provide an INSTEAD OF INSERT trigger.
+MERGE INTO ro_view13 AS t USING (VALUES (2, 'Row 2')) AS v(a,b) ON t.a = v.a
+ WHEN MATCHED THEN DO NOTHING
+ WHEN NOT MATCHED THEN DO NOTHING; -- should be OK to do nothing
MERGE INTO ro_view13 AS t USING (VALUES (3, 'Row 3')) AS v(a,b) ON t.a = v.a
WHEN MATCHED THEN DO NOTHING
WHEN NOT MATCHED THEN DO NOTHING; -- should be OK to do nothing
@@ -375,6 +378,8 @@ DELETE FROM ro_view18;
ERROR: cannot delete from view "ro_view18"
DETAIL: Views that do not select from a single table or view are not automatically updatable.
HINT: To enable deleting from the view, provide an INSTEAD OF DELETE trigger or an unconditional ON DELETE DO INSTEAD rule.
+MERGE INTO ro_view18 AS t USING (VALUES (1, 'Row 1')) AS v(a,b) ON t.a = v.a
+ WHEN MATCHED THEN DO NOTHING; -- should be OK to do nothing
UPDATE ro_view19 SET last_value=1000;
ERROR: cannot update view "ro_view19"
DETAIL: Views that do not select from a single table or view are not automatically updatable.
diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql
index e0ab923..abfa557 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -68,6 +68,9 @@ MERGE INTO ro_view13 AS t USING (VALUES (2, 'Row 2')) AS v(a,b) ON t.a = v.a
WHEN MATCHED THEN UPDATE SET b = v.b;
MERGE INTO ro_view13 AS t USING (VALUES (3, 'Row 3')) AS v(a,b) ON t.a = v.a
WHEN NOT MATCHED THEN INSERT VALUES (v.a, v.b);
+MERGE INTO ro_view13 AS t USING (VALUES (2, 'Row 2')) AS v(a,b) ON t.a = v.a
+ WHEN MATCHED THEN DO NOTHING
+ WHEN NOT MATCHED THEN DO NOTHING; -- should be OK to do nothing
MERGE INTO ro_view13 AS t USING (VALUES (3, 'Row 3')) AS v(a,b) ON t.a = v.a
WHEN MATCHED THEN DO NOTHING
WHEN NOT MATCHED THEN DO NOTHING; -- should be OK to do nothing
@@ -121,6 +124,8 @@ DELETE FROM rw_view16 WHERE a=-3; -- should be OK
-- Read-only views
INSERT INTO ro_view17 VALUES (3, 'ROW 3');
DELETE FROM ro_view18;
+MERGE INTO ro_view18 AS t USING (VALUES (1, 'Row 1')) AS v(a,b) ON t.a = v.a
+ WHEN MATCHED THEN DO NOTHING; -- should be OK to do nothing
UPDATE ro_view19 SET last_value=1000;
UPDATE ro_view20 SET b=upper(b);