MERGE bug report
Hello Hackers,
Reporting a bug with the new MERGE statement. Tested against 75edb919613ee835e7680e40137e494c7856bcf9.
psql output as follows:
...
psql:merge.sql:33: ERROR: variable not found in subplan target lists
ROLLBACK
[local] joe@joe=# \errverbose
ERROR: XX000: variable not found in subplan target lists
LOCATION: fix_join_expr_mutator, setrefs.c:2800
Stack trace:
fix_join_expr_mutator setrefs.c:2800
expression_tree_mutator nodeFuncs.c:3348
fix_join_expr_mutator setrefs.c:2853
expression_tree_mutator nodeFuncs.c:2992
fix_join_expr_mutator setrefs.c:2853
expression_tree_mutator nodeFuncs.c:3348
fix_join_expr_mutator setrefs.c:2853
fix_join_expr setrefs.c:2753
set_plan_refs setrefs.c:1085
set_plan_references setrefs.c:315
standard_planner planner.c:498
planner planner.c:277
pg_plan_query postgres.c:883
pg_plan_queries postgres.c:975
exec_simple_query postgres.c:1169
PostgresMain postgres.c:4520
BackendRun postmaster.c:4593
BackendStartup postmaster.c:4321
ServerLoop postmaster.c:1801
PostmasterMain postmaster.c:1473
main main.c:202
__libc_start_main 0x00007fc4ccc0b1e2
_start 0x000000000048804e
Reproducer script:
BEGIN;
DROP TABLE IF EXISTS item, incoming, source CASCADE;
CREATE TABLE item
(order_id INTEGER NOT NULL,
item_id INTEGER NOT NULL,
quantity INTEGER NOT NULL,
price NUMERIC NOT NULL,
CONSTRAINT pk_item PRIMARY KEY (order_id, item_id));
INSERT INTO item VALUES (100, 1, 4, 100.00), (100, 2, 9, 199.00);
CREATE TABLE incoming (order_id, item_id, quantity, price)
AS (VALUES (100, 1, 4, 100.00), (100, 3, 1, 200.00));
CREATE TABLE source (order_id, item_id, quantity, price) AS
(SELECT order_id, item_id, incoming.quantity, incoming.price
FROM item LEFT JOIN incoming USING (order_id, item_id));
MERGE INTO item a
USING source b
ON (a.order_id, a.item_id) =
(b.order_id, b.item_id)
WHEN NOT MATCHED
THEN INSERT (order_id, item_id, quantity, price)
VALUES (order_id, item_id, quantity, price)
WHEN MATCHED
AND a.* IS DISTINCT FROM b.*
THEN UPDATE SET (quantity, price) = (b.quantity, b.price)
WHEN MATCHED
AND (b.quantity IS NULL AND b.price IS NULL)
THEN DELETE;
COMMIT;
It seems related to the use of a.* and b.*
Sorry I can't be more specific. Error manifests when planning occurs and that is well outside of my code base knowledge.
Hope this helps.
Cheers,
-Joe
On Tue, Apr 5, 2022 at 3:18 PM Joe Wildish <joe@lateraljoin.com> wrote:
Hello Hackers,
Reporting a bug with the new MERGE statement. Tested against
75edb919613ee835e7680e40137e494c7856bcf9.psql output as follows:
...
psql:merge.sql:33: ERROR: variable not found in subplan target lists
ROLLBACK
[local] joe@joe=# \errverbose
ERROR: XX000: variable not found in subplan target lists
LOCATION: fix_join_expr_mutator, setrefs.c:2800Stack trace:
fix_join_expr_mutator setrefs.c:2800
expression_tree_mutator nodeFuncs.c:3348
fix_join_expr_mutator setrefs.c:2853
expression_tree_mutator nodeFuncs.c:2992
fix_join_expr_mutator setrefs.c:2853
expression_tree_mutator nodeFuncs.c:3348
fix_join_expr_mutator setrefs.c:2853
fix_join_expr setrefs.c:2753
set_plan_refs setrefs.c:1085
set_plan_references setrefs.c:315
standard_planner planner.c:498
planner planner.c:277
pg_plan_query postgres.c:883
pg_plan_queries postgres.c:975
exec_simple_query postgres.c:1169
PostgresMain postgres.c:4520
BackendRun postmaster.c:4593
BackendStartup postmaster.c:4321
ServerLoop postmaster.c:1801
PostmasterMain postmaster.c:1473
main main.c:202
__libc_start_main 0x00007fc4ccc0b1e2
_start 0x000000000048804eReproducer script:
BEGIN;
DROP TABLE IF EXISTS item, incoming, source CASCADE;CREATE TABLE item
(order_id INTEGER NOT NULL,
item_id INTEGER NOT NULL,
quantity INTEGER NOT NULL,
price NUMERIC NOT NULL,
CONSTRAINT pk_item PRIMARY KEY (order_id, item_id));INSERT INTO item VALUES (100, 1, 4, 100.00), (100, 2, 9, 199.00);
CREATE TABLE incoming (order_id, item_id, quantity, price)
AS (VALUES (100, 1, 4, 100.00), (100, 3, 1, 200.00));CREATE TABLE source (order_id, item_id, quantity, price) AS
(SELECT order_id, item_id, incoming.quantity, incoming.price
FROM item LEFT JOIN incoming USING (order_id, item_id));MERGE INTO item a
USING source b
ON (a.order_id, a.item_id) =
(b.order_id, b.item_id)
WHEN NOT MATCHED
THEN INSERT (order_id, item_id, quantity, price)
VALUES (order_id, item_id, quantity, price)
WHEN MATCHED
AND a.* IS DISTINCT FROM b.*
THEN UPDATE SET (quantity, price) = (b.quantity, b.price)
WHEN MATCHED
AND (b.quantity IS NULL AND b.price IS NULL)
THEN DELETE;
COMMIT;It seems related to the use of a.* and b.*
Sorry I can't be more specific. Error manifests when planning occurs and
that is well outside of my code base knowledge.Hope this helps.
Cheers,
-Joe
Hi,
It seems all the calls to fix_join_expr_mutator() are within setrefs.c
I haven't found where in nodeFuncs.c fix_join_expr_mutator is called.
I am on commit 75edb919613ee835e7680e40137e494c7856bcf9 .
On Tue, Apr 5, 2022 at 3:35 PM Zhihong Yu <zyu@yugabyte.com> wrote:
On Tue, Apr 5, 2022 at 3:18 PM Joe Wildish <joe@lateraljoin.com> wrote:
Hello Hackers,
Reporting a bug with the new MERGE statement. Tested against
75edb919613ee835e7680e40137e494c7856bcf9.psql output as follows:
...
psql:merge.sql:33: ERROR: variable not found in subplan target lists
ROLLBACK
[local] joe@joe=# \errverbose
ERROR: XX000: variable not found in subplan target lists
LOCATION: fix_join_expr_mutator, setrefs.c:2800Stack trace:
fix_join_expr_mutator setrefs.c:2800
expression_tree_mutator nodeFuncs.c:3348
fix_join_expr_mutator setrefs.c:2853
expression_tree_mutator nodeFuncs.c:2992
fix_join_expr_mutator setrefs.c:2853
expression_tree_mutator nodeFuncs.c:3348
fix_join_expr_mutator setrefs.c:2853
fix_join_expr setrefs.c:2753
set_plan_refs setrefs.c:1085
set_plan_references setrefs.c:315
standard_planner planner.c:498
planner planner.c:277
pg_plan_query postgres.c:883
pg_plan_queries postgres.c:975
exec_simple_query postgres.c:1169
PostgresMain postgres.c:4520
BackendRun postmaster.c:4593
BackendStartup postmaster.c:4321
ServerLoop postmaster.c:1801
PostmasterMain postmaster.c:1473
main main.c:202
__libc_start_main 0x00007fc4ccc0b1e2
_start 0x000000000048804eReproducer script:
BEGIN;
DROP TABLE IF EXISTS item, incoming, source CASCADE;CREATE TABLE item
(order_id INTEGER NOT NULL,
item_id INTEGER NOT NULL,
quantity INTEGER NOT NULL,
price NUMERIC NOT NULL,
CONSTRAINT pk_item PRIMARY KEY (order_id, item_id));INSERT INTO item VALUES (100, 1, 4, 100.00), (100, 2, 9, 199.00);
CREATE TABLE incoming (order_id, item_id, quantity, price)
AS (VALUES (100, 1, 4, 100.00), (100, 3, 1, 200.00));CREATE TABLE source (order_id, item_id, quantity, price) AS
(SELECT order_id, item_id, incoming.quantity, incoming.price
FROM item LEFT JOIN incoming USING (order_id, item_id));MERGE INTO item a
USING source b
ON (a.order_id, a.item_id) =
(b.order_id, b.item_id)
WHEN NOT MATCHED
THEN INSERT (order_id, item_id, quantity, price)
VALUES (order_id, item_id, quantity, price)
WHEN MATCHED
AND a.* IS DISTINCT FROM b.*
THEN UPDATE SET (quantity, price) = (b.quantity, b.price)
WHEN MATCHED
AND (b.quantity IS NULL AND b.price IS NULL)
THEN DELETE;
COMMIT;It seems related to the use of a.* and b.*
Sorry I can't be more specific. Error manifests when planning occurs and
that is well outside of my code base knowledge.Hope this helps.
Cheers,
-JoeHi,
It seems all the calls to fix_join_expr_mutator() are within setrefs.cI haven't found where in nodeFuncs.c fix_join_expr_mutator is called.
I am on commit 75edb919613ee835e7680e40137e494c7856bcf9 .
Pardon - I typed too fast:
The call to fix_join_expr_mutator() is on this line (3348):
resultlist = lappend(resultlist,
mutator((Node *) lfirst(temp),
context));
On Wed, Apr 6, 2022 at 6:18 AM Joe Wildish <joe@lateraljoin.com> wrote:
Hello Hackers,
Reporting a bug with the new MERGE statement. Tested against
75edb919613ee835e7680e40137e494c7856bcf9.psql output as follows:
...
psql:merge.sql:33: ERROR: variable not found in subplan target lists
ROLLBACK
[local] joe@joe=# \errverbose
ERROR: XX000: variable not found in subplan target lists
LOCATION: fix_join_expr_mutator, setrefs.c:2800Stack trace:
fix_join_expr_mutator setrefs.c:2800
expression_tree_mutator nodeFuncs.c:3348
fix_join_expr_mutator setrefs.c:2853
expression_tree_mutator nodeFuncs.c:2992
fix_join_expr_mutator setrefs.c:2853
expression_tree_mutator nodeFuncs.c:3348
fix_join_expr_mutator setrefs.c:2853
fix_join_expr setrefs.c:2753
set_plan_refs setrefs.c:1085
set_plan_references setrefs.c:315
standard_planner planner.c:498
planner planner.c:277
pg_plan_query postgres.c:883
pg_plan_queries postgres.c:975
exec_simple_query postgres.c:1169
PostgresMain postgres.c:4520
BackendRun postmaster.c:4593
BackendStartup postmaster.c:4321
ServerLoop postmaster.c:1801
PostmasterMain postmaster.c:1473
main main.c:202
__libc_start_main 0x00007fc4ccc0b1e2
_start 0x000000000048804eReproducer script:
BEGIN;
DROP TABLE IF EXISTS item, incoming, source CASCADE;CREATE TABLE item
(order_id INTEGER NOT NULL,
item_id INTEGER NOT NULL,
quantity INTEGER NOT NULL,
price NUMERIC NOT NULL,
CONSTRAINT pk_item PRIMARY KEY (order_id, item_id));INSERT INTO item VALUES (100, 1, 4, 100.00), (100, 2, 9, 199.00);
CREATE TABLE incoming (order_id, item_id, quantity, price)
AS (VALUES (100, 1, 4, 100.00), (100, 3, 1, 200.00));CREATE TABLE source (order_id, item_id, quantity, price) AS
(SELECT order_id, item_id, incoming.quantity, incoming.price
FROM item LEFT JOIN incoming USING (order_id, item_id));MERGE INTO item a
USING source b
ON (a.order_id, a.item_id) =
(b.order_id, b.item_id)
WHEN NOT MATCHED
THEN INSERT (order_id, item_id, quantity, price)
VALUES (order_id, item_id, quantity, price)
WHEN MATCHED
AND a.* IS DISTINCT FROM b.*
THEN UPDATE SET (quantity, price) = (b.quantity, b.price)
WHEN MATCHED
AND (b.quantity IS NULL AND b.price IS NULL)
THEN DELETE;
COMMIT;It seems related to the use of a.* and b.*
That's right. The varattno is set to zero for whole-row Var. And in this
case these whole-row Vars are not included in the targetlist.
Attached is an attempt for the fix.
Thanks
Richard
Attachments:
0001-Include-entries-in-WHEN-conditions-into-processed_tl.patchapplication/octet-stream; name=0001-Include-entries-in-WHEN-conditions-into-processed_tl.patchDownload
From 978831a89cc1f77a2fec7ab56773e6230c9698a4 Mon Sep 17 00:00:00 2001
From: pgsql-guo <richard.guo@openpie.com>
Date: Wed, 6 Apr 2022 07:29:18 +0000
Subject: [PATCH] Include entries in WHEN conditions into processed_tlist
---
src/backend/optimizer/prep/preptlist.c | 32 ++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 99ab3d7559..f805aed73e 100644
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -150,6 +150,8 @@ preprocess_targetlist(PlannerInfo *root)
*/
foreach(l, parse->mergeActionList)
{
+ List *vars;
+ ListCell *cell;
MergeAction *action = (MergeAction *) lfirst(l);
if (action->commandType == CMD_INSERT)
@@ -158,6 +160,36 @@ preprocess_targetlist(PlannerInfo *root)
else if (action->commandType == CMD_UPDATE)
action->updateColnos =
extract_update_targetlist_colnos(action->targetList);
+
+ /*
+ * Add resjunk entries for any Vars used in WHEN conditions if any
+ * that belong to other relations.
+ */
+ vars = pull_var_clause((Node *) action->qual,
+ PVC_RECURSE_AGGREGATES |
+ PVC_RECURSE_WINDOWFUNCS |
+ PVC_INCLUDE_PLACEHOLDERS);
+
+ foreach(cell, vars)
+ {
+ Var *var = (Var *) lfirst(cell);
+ TargetEntry *tle;
+
+ if (IsA(var, Var) &&
+ var->varno == result_relation)
+ continue; /* don't need it */
+
+ if (tlist_member((Expr *) var, tlist))
+ continue; /* already got it */
+
+ tle = makeTargetEntry((Expr *) var,
+ list_length(tlist) + 1,
+ NULL,
+ true);
+
+ tlist = lappend(tlist, tle);
+ }
+ list_free(vars);
}
}
--
2.25.1
On 2022-Apr-06, Richard Guo wrote:
That's right. The varattno is set to zero for whole-row Var. And in this
case these whole-row Vars are not included in the targetlist.Attached is an attempt for the fix.
Wow, this is very interesting. I was surprised that this patch was
necessary at all -- I mean, if wholerow refs don't work, then why do
references to any other columns work? The answer is that parse_merge.c
is already setting up the subplan's targetlist by expanding all vars of
the source relation. I then remembered than in Simon's (or Pavan's)
original coding, parse_merge.c had a hack to include a var with the
source's wholerow in that targetlist, which I had later removed ...
I eventually realized that there's no need for parse_merge.c to expand
the source rel at all, and indeed it's wasteful: we can just let
preprocess_targetlist include the vars that are referenced by either
quals or each action's targetlist instead. That led me to the attached
patch, which is not commit-quality yet but it should show what I have in
mind.
I added a test query to tickle this problematic case.
Another point, not completely connected to this bug but appearing in the
same function, is that we have some redundant code: we can just let the
stanza for UPDATE/DELETE do the identity columns dance. This saves a
few lines in the MERGE-specific stanza there, which was doing exactly
the same thing. (There's a difference in the "inh" test, but I think
that was just outdated.)
I also discovered that the comment for fix_join_expr needed an update,
since it doesn't mention MERGE, and it does mention all other situations
in which it is used. Added that too.
This patch is a comment about "aggregates, window functions and
placeholder vars". This was relevant and correct when only the qual of
each action was being handled (i.e., Richard's patch). Now that we're
also handling the action's targetlist, I think I need to put the PVC
flags back. But no tests broke, which probably means we also need some
additional tests cases.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Attachments:
fix-merge-targetlist.patchtext/x-diff; charset=utf-8Download
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 6ea3505646..fa5969bbd5 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -2763,6 +2763,10 @@ search_indexed_tlist_for_sortgroupref(Expr *node,
* to-be-updated relation) alone. Correspondingly inner_itlist is to be
* EXCLUDED elements, outer_itlist = NULL and acceptable_rel the target
* relation.
+ * 4) MERGE. In this case, references to the source relation are to be
+ * replaced with INNER_VAR references, and target Vars (the to-be-
+ * modified relation) are left alone. So inner_itlist is to be
+ * the source relation and acceptable_rel the target relatin.
*
* 'clauses' is the targetlist or list of join clauses
* 'outer_itlist' is the indexed target list of the outer join relation,
diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 99ab3d7559..c1c3067365 100644
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -107,14 +107,15 @@ preprocess_targetlist(PlannerInfo *root)
root->update_colnos = extract_update_targetlist_colnos(tlist);
/*
- * For non-inherited UPDATE/DELETE, register any junk column(s) needed to
- * allow the executor to identify the rows to be updated or deleted. In
- * the inheritance case, we do nothing now, leaving this to be dealt with
- * when expand_inherited_rtentry() makes the leaf target relations. (But
- * there might not be any leaf target relations, in which case we must do
- * this in distribute_row_identity_vars().)
+ * For non-inherited UPDATE/DELETE/MERGE, register any junk column(s)
+ * needed to allow the executor to identify the rows to be updated or
+ * deleted. In the inheritance case, we do nothing now, leaving this to
+ * be dealt with when expand_inherited_rtentry() makes the leaf target
+ * relations. (But there might not be any leaf target relations, in which
+ * case we must do this in distribute_row_identity_vars().)
*/
- if ((command_type == CMD_UPDATE || command_type == CMD_DELETE) &&
+ if ((command_type == CMD_UPDATE || command_type == CMD_DELETE ||
+ command_type == CMD_MERGE) &&
!target_rte->inh)
{
/* row-identity logic expects to add stuff to processed_tlist */
@@ -125,23 +126,15 @@ preprocess_targetlist(PlannerInfo *root)
}
/*
- * For MERGE we need to handle the target list for the target relation,
- * and also target list for each action (only INSERT/UPDATE matter).
+ * For MERGE we also need to handle the target list for each INSERT and
+ * UPDATE action separately. In addition, we examine the qual of each
+ * action and add any Vars there (other than those of the target rel) to
+ * the subplan targetlist.
*/
if (command_type == CMD_MERGE)
{
ListCell *l;
- /*
- * For MERGE, add any junk column(s) needed to allow the executor to
- * identify the rows to be inserted or updated.
- */
- root->processed_tlist = tlist;
- add_row_identity_columns(root, result_relation,
- target_rte, target_relation);
-
- tlist = root->processed_tlist;
-
/*
* For MERGE, handle targetlist of each MergeAction separately. Give
* the same treatment to MergeAction->targetList as we would have
@@ -151,6 +144,8 @@ preprocess_targetlist(PlannerInfo *root)
foreach(l, parse->mergeActionList)
{
MergeAction *action = (MergeAction *) lfirst(l);
+ List *vars;
+ ListCell *l2;
if (action->commandType == CMD_INSERT)
action->targetList = expand_insert_targetlist(action->targetList,
@@ -158,6 +153,33 @@ preprocess_targetlist(PlannerInfo *root)
else if (action->commandType == CMD_UPDATE)
action->updateColnos =
extract_update_targetlist_colnos(action->targetList);
+
+ /*
+ * Add resjunk entries for any Vars used in WHEN conditions that
+ * belong to relations other than target. (Note that aggregates,
+ * window functions and placeholder vars are not possible in MERGE
+ * WHEN clauses.)
+ */
+ vars = pull_var_clause((Node *) list_concat(list_copy((List *) action->qual),
+ action->targetList),
+ 0);
+ foreach(l2, vars)
+ {
+ Var *var = (Var *) lfirst(l2);
+ TargetEntry *tle;
+
+ if (IsA(var, Var) && var->varno == result_relation)
+ continue; /* don't need it */
+
+ if (tlist_member((Expr *) var, tlist))
+ continue; /* already got it */
+
+ tle = makeTargetEntry((Expr *) var,
+ list_length(tlist) + 1,
+ NULL, true);
+ tlist = lappend(tlist, tle);
+ }
+ list_free(vars);
}
}
diff --git a/src/backend/parser/parse_merge.c b/src/backend/parser/parse_merge.c
index 5d0035a12b..9f2fe527c4 100644
--- a/src/backend/parser/parse_merge.c
+++ b/src/backend/parser/parse_merge.c
@@ -205,9 +205,11 @@ transformMergeStmt(ParseState *pstate, MergeStmt *stmt)
pstate->p_target_nsitem->p_names->aliasname),
errdetail("The name is used both as MERGE target table and data source."));
- qry->targetList = expandNSItemAttrs(pstate, nsitem, 0, false,
- exprLocation(stmt->sourceRelation));
-
+ /*
+ * There's no need for a targetlist here; it'll be set up by
+ * preprocess_targetlist later.
+ */
+ qry->targetList = NIL;
qry->rtable = pstate->p_rtable;
/*
diff --git a/src/test/regress/expected/merge.out b/src/test/regress/expected/merge.out
index 5954f10b8f..0fd037b45a 100644
--- a/src/test/regress/expected/merge.out
+++ b/src/test/regress/expected/merge.out
@@ -791,6 +791,19 @@ SELECT * FROM wq_target;
1 | 299
(1 row)
+-- check source-side whole-row references
+BEGIN;
+MERGE INTO wq_target t
+USING wq_source s ON (t.tid = s.sid)
+WHEN matched and t = s or t.tid = s.sid THEN
+ UPDATE SET balance = t.balance + s.balance;
+SELECT * FROM wq_target;
+ tid | balance
+-----+---------
+ 1 | 399
+(1 row)
+
+ROLLBACK;
-- check if subqueries work in the conditions?
MERGE INTO wq_target t
USING wq_source s ON t.tid = s.sid
diff --git a/src/test/regress/sql/merge.sql b/src/test/regress/sql/merge.sql
index 6d05a2f39c..8815e0cc49 100644
--- a/src/test/regress/sql/merge.sql
+++ b/src/test/regress/sql/merge.sql
@@ -527,6 +527,15 @@ WHEN MATCHED AND t.balance = 199 OR s.balance > 100 THEN
UPDATE SET balance = t.balance + s.balance;
SELECT * FROM wq_target;
+-- check source-side whole-row references
+BEGIN;
+MERGE INTO wq_target t
+USING wq_source s ON (t.tid = s.sid)
+WHEN matched and t = s or t.tid = s.sid THEN
+ UPDATE SET balance = t.balance + s.balance;
+SELECT * FROM wq_target;
+ROLLBACK;
+
-- check if subqueries work in the conditions?
MERGE INTO wq_target t
USING wq_source s ON t.tid = s.sid
On Sat, Apr 9, 2022 at 5:26 AM Alvaro Herrera <alvherre@alvh.no-ip.org>
wrote:
On 2022-Apr-06, Richard Guo wrote:
That's right. The varattno is set to zero for whole-row Var. And in this
case these whole-row Vars are not included in the targetlist.Attached is an attempt for the fix.
Wow, this is very interesting. I was surprised that this patch was
necessary at all -- I mean, if wholerow refs don't work, then why do
references to any other columns work? The answer is that parse_merge.c
is already setting up the subplan's targetlist by expanding all vars of
the source relation. I then remembered than in Simon's (or Pavan's)
original coding, parse_merge.c had a hack to include a var with the
source's wholerow in that targetlist, which I had later removed ...
At first I was wondering whether we need to also include vars used in
each action's targetlist, just as what we did for each action's qual.
Then later I realized parse_merge.c already did that. But now it looks
much better to process them two in preprocess_targetlist.
I eventually realized that there's no need for parse_merge.c to expand
the source rel at all, and indeed it's wasteful: we can just let
preprocess_targetlist include the vars that are referenced by either
quals or each action's targetlist instead. That led me to the attached
patch, which is not commit-quality yet but it should show what I have in
mind.
This patch looks in a good shape to me.
A minor comment is that we can use list_concat_copy(list1, list2)
instead of list_concat(list_copy(list1), list2) for better efficiency.
Thanks
Richard
On 2022-Apr-11, Richard Guo wrote:
At first I was wondering whether we need to also include vars used in
each action's targetlist, just as what we did for each action's qual.
Then later I realized parse_merge.c already did that. But now it looks
much better to process them two in preprocess_targetlist.
Yeah. I pushed that.
However, now EXPLAIN VERBOSE doesn't show the columns from the source
relation in the Output line --- I think only those that are used as join
quals are shown, thanks to distribute_quals_to_rels. I think it would
be better to fix this. Maybe expanding the source target list earlier
is called for, after all. I looked at transformUpdateStmt and siblings
for inspiration, but came out blank.
A minor comment is that we can use list_concat_copy(list1, list2)
instead of list_concat(list_copy(list1), list2) for better efficiency.
Thanks for that tip.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"La vida es para el que se aventura"