Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf9f63ed

Browse files
committed
Repair bogus handling of multi-assignment Params in upper plan levels.
Our support for multiple-set-clauses in UPDATE assumes that the Paramsreferencing a MULTIEXPR_SUBLINK SubPlan will appear before that SubPlanin the targetlist of the plan node that calculates the updated row.(Yeah, it's a hack...) In some PG branches it's possible that a Resultnode gets inserted between the primary calculation of the update tlistand the ModifyTable node. setrefs.c did the wrong thing in this caseand left the upper-level Params as Params, causing a crash at runtime.What it should do is replace them with "outer" Vars referencing the childplan node's output. That's a result of careless ordering of operationsin fix_upper_expr_mutator, so we can fix it just by reordering the code.Fix fix_join_expr_mutator similarly for consistency, even though joinnodes could never appear in such a context. (In general, it seemslikely to be a bit cheaper to use Vars than Params in such situationsanyway, so this patch might offer a tiny performance improvement.)The hazard extends back to 9.5 where the MULTIEXPR_SUBLINK stuffwas introduced, so back-patch that far. However, this may be a livebug only in 9.6.x and 10.x, as the other branches don't seem to wantto calculate the final tlist below the Result node. (That plan shapechange between branches might be a mini-bug in itself, but I'm notreally interested in digging into the reasons for that right now.Still, add a regression test memorializing what we expect there,so we'll notice if it changes again.)Per bug report from Eduards Bezverhijs.Discussion:https://postgr.es/m/b6cd572a-3e44-8785-75e9-c512a5a17a73@tieto.com
1 parent0226cd7 commitf9f63ed

File tree

3 files changed

+54
-11
lines changed

3 files changed

+54
-11
lines changed

‎src/backend/optimizer/plan/setrefs.c

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2269,8 +2269,6 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
22692269
/* If not supplied by input plans, evaluate the contained expr */
22702270
returnfix_join_expr_mutator((Node*)phv->phexpr,context);
22712271
}
2272-
if (IsA(node,Param))
2273-
returnfix_param_node(context->root, (Param*)node);
22742272
/* Try matching more complex expressions too, if tlists have any */
22752273
if (context->outer_itlist&&context->outer_itlist->has_non_vars)
22762274
{
@@ -2288,6 +2286,9 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
22882286
if (newvar)
22892287
return (Node*)newvar;
22902288
}
2289+
/* Special cases (apply only AFTER failing to match to lower tlist) */
2290+
if (IsA(node,Param))
2291+
returnfix_param_node(context->root, (Param*)node);
22912292
fix_expr_common(context->root,node);
22922293
returnexpression_tree_mutator(node,
22932294
fix_join_expr_mutator,
@@ -2375,6 +2376,16 @@ fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context)
23752376
/* If not supplied by input plan, evaluate the contained expr */
23762377
returnfix_upper_expr_mutator((Node*)phv->phexpr,context);
23772378
}
2379+
/* Try matching more complex expressions too, if tlist has any */
2380+
if (context->subplan_itlist->has_non_vars)
2381+
{
2382+
newvar=search_indexed_tlist_for_non_var((Expr*)node,
2383+
context->subplan_itlist,
2384+
context->newvarno);
2385+
if (newvar)
2386+
return (Node*)newvar;
2387+
}
2388+
/* Special cases (apply only AFTER failing to match to lower tlist) */
23782389
if (IsA(node,Param))
23792390
returnfix_param_node(context->root, (Param*)node);
23802391
if (IsA(node,Aggref))
@@ -2399,15 +2410,6 @@ fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context)
23992410
}
24002411
/* If no match, just fall through to process it normally */
24012412
}
2402-
/* Try matching more complex expressions too, if tlist has any */
2403-
if (context->subplan_itlist->has_non_vars)
2404-
{
2405-
newvar=search_indexed_tlist_for_non_var((Expr*)node,
2406-
context->subplan_itlist,
2407-
context->newvarno);
2408-
if (newvar)
2409-
return (Node*)newvar;
2410-
}
24112413
fix_expr_common(context->root,node);
24122414
returnexpression_tree_mutator(node,
24132415
fix_upper_expr_mutator,

‎src/test/regress/expected/update.out

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,37 @@ SELECT a, b, char_length(c) FROM update_test;
167167
42 | 12 | 10000
168168
(4 rows)
169169

170+
-- Check multi-assignment with a Result node to handle a one-time filter.
171+
EXPLAIN (VERBOSE, COSTS OFF)
172+
UPDATE update_test t
173+
SET (a, b) = (SELECT b, a FROM update_test s WHERE s.a = t.a)
174+
WHERE CURRENT_USER = SESSION_USER;
175+
QUERY PLAN
176+
------------------------------------------------------------------------
177+
Update on public.update_test t
178+
-> Result
179+
Output: ($1), ($2), t.c, ((SubPlan 1 (returns $1,$2))), t.ctid
180+
One-Time Filter: (CURRENT_USER = SESSION_USER)
181+
-> Seq Scan on public.update_test t
182+
Output: $1, $2, t.c, (SubPlan 1 (returns $1,$2)), t.ctid
183+
SubPlan 1 (returns $1,$2)
184+
-> Seq Scan on public.update_test s
185+
Output: s.b, s.a
186+
Filter: (s.a = t.a)
187+
(10 rows)
188+
189+
UPDATE update_test t
190+
SET (a, b) = (SELECT b, a FROM update_test s WHERE s.a = t.a)
191+
WHERE CURRENT_USER = SESSION_USER;
192+
SELECT a, b, char_length(c) FROM update_test;
193+
a | b | char_length
194+
-----+----+-------------
195+
| |
196+
100 | 21 |
197+
12 | 41 | 10000
198+
12 | 42 | 10000
199+
(4 rows)
200+
170201
-- Test ON CONFLICT DO UPDATE
171202
INSERT INTO upsert_test VALUES(1, 'Boo');
172203
-- uncorrelated sub-select:

‎src/test/regress/sql/update.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,16 @@ UPDATE update_test AS t SET b = update_test.b + 10 WHERE t.a = 10;
8989
UPDATE update_testSET c= repeat('x',10000)WHERE c='car';
9090
SELECT a, b, char_length(c)FROM update_test;
9191

92+
-- Check multi-assignment with a Result node to handle a one-time filter.
93+
EXPLAIN (VERBOSE, COSTS OFF)
94+
UPDATE update_test t
95+
SET (a, b)= (SELECT b, aFROM update_test sWHEREs.a=t.a)
96+
WHERECURRENT_USER=SESSION_USER;
97+
UPDATE update_test t
98+
SET (a, b)= (SELECT b, aFROM update_test sWHEREs.a=t.a)
99+
WHERECURRENT_USER=SESSION_USER;
100+
SELECT a, b, char_length(c)FROM update_test;
101+
92102
-- Test ON CONFLICT DO UPDATE
93103
INSERT INTO upsert_testVALUES(1,'Boo');
94104
-- uncorrelated sub-select:

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp