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

Commit58d8024

Browse files
committed
Fix assorted missing infrastructure for ON CONFLICT.
subquery_planner() failed to apply expression preprocessing to thearbiterElems and arbiterWhere fields of an OnConflictExpr. No doubt thetheory was that this wasn't necessary because we don't actually try toexecute those expressions; but that's wrong, because it results in failureto match to index expressions or index predicates that are changed at allby preprocessing. Per bug #14132 from Reynold Smith.Also add pullup_replace_vars processing for onConflictWhere. Perhapsit's impossible to have a subquery reference there, but I'm not exactlyconvinced; and even if true today it's a failure waiting to happen.Also add some comments to other places where one or another field ofOnConflictExpr is intentionally ignored, with explanation as to why it'sokay to do so.Also, catalog/dependency.c failed to record any dependency on the namedconstraint in ON CONFLICT ON CONSTRAINT, allowing such a constraint tobe dropped while rules exist that depend on it, and allowing pg_dump todump such a rule before the constraint it refers to. The normal executionpath managed to error out reasonably for a dangling constraint reference,but ruleutils.c dumped core; so in addition to fixing the omission, adda protective check in ruleutils.c, since we can't retroactively add adependency in existing databases.Back-patch to 9.5 where this code was introduced.Report: <20160510190350.2608.48667@wrigleys.postgresql.org>
1 parent7516cdb commit58d8024

File tree

8 files changed

+94
-12
lines changed

8 files changed

+94
-12
lines changed

‎src/backend/catalog/dependency.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1777,6 +1777,15 @@ find_expr_references_walker(Node *node,
17771777
add_object_address(OCLASS_TYPE,cd->resulttype,0,
17781778
context->addrs);
17791779
}
1780+
elseif (IsA(node,OnConflictExpr))
1781+
{
1782+
OnConflictExpr*onconflict= (OnConflictExpr*)node;
1783+
1784+
if (OidIsValid(onconflict->constraint))
1785+
add_object_address(OCLASS_CONSTRAINT,onconflict->constraint,0,
1786+
context->addrs);
1787+
/* fall through to examine arguments */
1788+
}
17801789
elseif (IsA(node,SortGroupClause))
17811790
{
17821791
SortGroupClause*sgc= (SortGroupClause*)node;

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ planner_hook_type planner_hook = NULL;
6565
#defineEXPRKIND_APPINFO7
6666
#defineEXPRKIND_PHV8
6767
#defineEXPRKIND_TABLESAMPLE9
68+
#defineEXPRKIND_ARBITER_ELEM10
6869

6970
/* Passthrough data for standard_qp_callback */
7071
typedefstruct
@@ -483,13 +484,23 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
483484

484485
if (parse->onConflict)
485486
{
487+
parse->onConflict->arbiterElems= (List*)
488+
preprocess_expression(root,
489+
(Node*)parse->onConflict->arbiterElems,
490+
EXPRKIND_ARBITER_ELEM);
491+
parse->onConflict->arbiterWhere=
492+
preprocess_expression(root,
493+
parse->onConflict->arbiterWhere,
494+
EXPRKIND_QUAL);
486495
parse->onConflict->onConflictSet= (List*)
487-
preprocess_expression(root, (Node*)parse->onConflict->onConflictSet,
496+
preprocess_expression(root,
497+
(Node*)parse->onConflict->onConflictSet,
488498
EXPRKIND_TARGET);
489-
490499
parse->onConflict->onConflictWhere=
491-
preprocess_expression(root, (Node*)parse->onConflict->onConflictWhere,
500+
preprocess_expression(root,
501+
parse->onConflict->onConflictWhere,
492502
EXPRKIND_QUAL);
503+
/* exclRelTlist contains only Vars, so no preprocessing needed */
493504
}
494505

495506
root->append_rel_list= (List*)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2426,6 +2426,7 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
24262426
&context);
24272427
finalize_primnode((Node*)mtplan->onConflictWhere,
24282428
&context);
2429+
/* exclRelTlist contains only Vars, doesn't need examination */
24292430
foreach(l,mtplan->plans)
24302431
{
24312432
context.paramids=

‎src/backend/optimizer/prep/prepjointree.c

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,8 +1031,19 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
10311031
parse->returningList= (List*)
10321032
pullup_replace_vars((Node*)parse->returningList,&rvcontext);
10331033
if (parse->onConflict)
1034+
{
10341035
parse->onConflict->onConflictSet= (List*)
1035-
pullup_replace_vars((Node*)parse->onConflict->onConflictSet,&rvcontext);
1036+
pullup_replace_vars((Node*)parse->onConflict->onConflictSet,
1037+
&rvcontext);
1038+
parse->onConflict->onConflictWhere=
1039+
pullup_replace_vars(parse->onConflict->onConflictWhere,
1040+
&rvcontext);
1041+
1042+
/*
1043+
* We assume ON CONFLICT's arbiterElems, arbiterWhere, exclRelTlist
1044+
* can't contain any references to a subquery
1045+
*/
1046+
}
10361047
replace_vars_in_jointree((Node*)parse->jointree,&rvcontext,
10371048
lowest_nulling_outer_join);
10381049
Assert(parse->setOperations==NULL);
@@ -1625,8 +1636,19 @@ pull_up_simple_values(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte)
16251636
parse->returningList= (List*)
16261637
pullup_replace_vars((Node*)parse->returningList,&rvcontext);
16271638
if (parse->onConflict)
1639+
{
16281640
parse->onConflict->onConflictSet= (List*)
1629-
pullup_replace_vars((Node*)parse->onConflict->onConflictSet,&rvcontext);
1641+
pullup_replace_vars((Node*)parse->onConflict->onConflictSet,
1642+
&rvcontext);
1643+
parse->onConflict->onConflictWhere=
1644+
pullup_replace_vars(parse->onConflict->onConflictWhere,
1645+
&rvcontext);
1646+
1647+
/*
1648+
* We assume ON CONFLICT's arbiterElems, arbiterWhere, exclRelTlist
1649+
* can't contain any references to a subquery
1650+
*/
1651+
}
16301652
replace_vars_in_jointree((Node*)parse->jointree,&rvcontext,NULL);
16311653
Assert(parse->setOperations==NULL);
16321654
parse->havingQual=pullup_replace_vars(parse->havingQual,&rvcontext);

‎src/backend/optimizer/util/plancat.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,6 @@ infer_arbiter_indexes(PlannerInfo *root)
530530
Bitmapset*indexedAttrs=NULL;
531531
List*idxExprs;
532532
List*predExprs;
533-
List*whereExplicit;
534533
AttrNumbernatt;
535534
ListCell*el;
536535

@@ -592,6 +591,7 @@ infer_arbiter_indexes(PlannerInfo *root)
592591
{
593592
intattno=idxRel->rd_index->indkey.values[natt];
594593

594+
/* XXX broken */
595595
if (attno<0)
596596
elog(ERROR,"system column in index");
597597

@@ -652,13 +652,12 @@ infer_arbiter_indexes(PlannerInfo *root)
652652
gotonext;
653653

654654
/*
655-
*Any user-supplied ON CONFLICT uniqueindex inference WHERE clause
656-
*need only be implied by the cataloged index definitions predicate.
655+
*If it's a partialindex, its predicate must be implied by the ON
656+
*CONFLICT's WHERE clause.
657657
*/
658658
predExprs=RelationGetIndexPredicate(idxRel);
659-
whereExplicit=make_ands_implicit((Expr*)onconflict->arbiterWhere);
660659

661-
if (!predicate_implied_by(predExprs,whereExplicit))
660+
if (!predicate_implied_by(predExprs,(List*)onconflict->arbiterWhere))
662661
gotonext;
663662

664663
results=lappend_oid(results,idxForm->indexrelid);

‎src/backend/utils/adt/ruleutils.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5543,12 +5543,15 @@ get_insert_query_def(Query *query, deparse_context *context)
55435543
context->varprefix=save_varprefix;
55445544
}
55455545
}
5546-
elseif (confl->constraint!=InvalidOid)
5546+
elseif (OidIsValid(confl->constraint))
55475547
{
55485548
char*constraint=get_constraint_name(confl->constraint);
55495549

5550+
if (!constraint)
5551+
elog(ERROR,"cache lookup failed for constraint %u",
5552+
confl->constraint);
55505553
appendStringInfo(buf," ON CONSTRAINT %s",
5551-
quote_qualified_identifier(NULL,constraint));
5554+
quote_identifier(constraint));
55525555
}
55535556

55545557
if (confl->action==ONCONFLICT_NOTHING)

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,21 @@ ERROR: column excluded.oid does not exist
454454
LINE 1: ...values (1) on conflict (key) do update set data = excluded.o...
455455
^
456456
drop table syscolconflicttest;
457+
--
458+
-- Previous tests all managed to not test any expressions requiring
459+
-- planner preprocessing ...
460+
--
461+
create table insertconflict (a bigint, b bigint);
462+
create unique index insertconflicti1 on insertconflict(coalesce(a, 0));
463+
create unique index insertconflicti2 on insertconflict(b)
464+
where coalesce(a, 1) > 0;
465+
insert into insertconflict values (1, 2)
466+
on conflict (coalesce(a, 0)) do nothing;
467+
insert into insertconflict values (1, 2)
468+
on conflict (b) where coalesce(a, 1) > 0 do nothing;
469+
insert into insertconflict values (1, 2)
470+
on conflict (b) where coalesce(a, 1) > 1 do nothing;
471+
drop table insertconflict;
457472
-- ******************************************************************
458473
-- * *
459474
-- * Test inheritance (example taken from tutorial) *

‎src/test/regress/sql/insert_conflict.sql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,28 @@ insert into syscolconflicttest values (1) on conflict (key) do update set data =
261261
insert into syscolconflicttestvalues (1)on conflict (key) doupdateset data=excluded.oid::text;
262262
droptable syscolconflicttest;
263263

264+
--
265+
-- Previous tests all managed to not test any expressions requiring
266+
-- planner preprocessing ...
267+
--
268+
createtableinsertconflict (abigint, bbigint);
269+
270+
createunique indexinsertconflicti1on insertconflict(coalesce(a,0));
271+
272+
createunique indexinsertconflicti2on insertconflict(b)
273+
where coalesce(a,1)>0;
274+
275+
insert into insertconflictvalues (1,2)
276+
on conflict (coalesce(a,0)) do nothing;
277+
278+
insert into insertconflictvalues (1,2)
279+
on conflict (b)where coalesce(a,1)>0 do nothing;
280+
281+
insert into insertconflictvalues (1,2)
282+
on conflict (b)where coalesce(a,1)>1 do nothing;
283+
284+
droptable insertconflict;
285+
264286

265287
-- ******************************************************************
266288
-- * *

0 commit comments

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