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

Commita3c7a99

Browse files
committed
Make INSERT-from-multiple-VALUES-rows handle targetlist indirection better.
Previously, if an INSERT with multiple rows of VALUES had indirection(array subscripting or field selection) in its target-columns list, theparser handled that by applying transformAssignedExpr() to each elementof each VALUES row independently. This led to having ArrayRef assignmentnodes or FieldStore nodes in each row of the VALUES RTE. That works forsimple cases, but in bug #14265 Nuri Boardman points out that it failsif there are multiple assignments to elements/fields of the same targetcolumn. For such cases to work, rewriteTargetListIU() has to nest theArrayRefs or FieldStores together to produce a single expression to beassigned to the column. But it failed to find them in the top-leveltargetlist and issued an error about "multiple assignments to same column".We could possibly fix this by teaching the rewriter to applyrewriteTargetListIU to each VALUES row separately, but that would be messy(it would change the output rowtype of the VALUES RTE, for example) andinefficient. Instead, let's fix the parser so that the VALUES RTE outputsare just the user-specified values, cast to the right type if necessary,and then the ArrayRefs or FieldStores are applied in the top-leveltargetlist to Vars representing the RTE's outputs. This is the sameparsetree representation already used for similar cases with INSERT/SELECTsyntax, so it allows simplifications in ruleutils.c, which no longer needsto treat INSERT-from-multiple-VALUES as its own special case.This implementation works by applying transformAssignedExpr to the VALUESentries as before, and then stripping off any ArrayRefs or FieldStores itadds. With lots of VALUES rows it would be noticeably more efficient tonot add those nodes in the first place. But that's just an optimizationnot a bug fix, and there doesn't seem to be any good way to do it withoutsignificant refactoring. (A non-invasive answer would be to applytransformAssignedExpr + stripping to just the first VALUES row, and thenjust forcibly cast remaining rows to the same data types exposed in thefirst row. But this way would lead to different, not-INSERT-specificerrors being reported in casting failure cases, so it doesn't seem verynice.) So leave that for later; this patch at least isn't making theper-row parsing work worse, and it does make the finished parsetreesmaller, saving rewriter and planner work.Catversion bump because stored rules containing such INSERTs would needto change. Because of that, no back-patch, even though this is a verylong-standing bug.Report: <20160727005725.7438.26021@wrigleys.postgresql.org>Discussion: <9578.1469645245@sss.pgh.pa.us>
1 parentef1b5af commita3c7a99

File tree

5 files changed

+205
-49
lines changed

5 files changed

+205
-49
lines changed

‎src/backend/parser/analyze.c

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ post_parse_analyze_hook_type post_parse_analyze_hook = NULL;
5151
staticQuery*transformDeleteStmt(ParseState*pstate,DeleteStmt*stmt);
5252
staticQuery*transformInsertStmt(ParseState*pstate,InsertStmt*stmt);
5353
staticList*transformInsertRow(ParseState*pstate,List*exprlist,
54-
List*stmtcols,List*icolumns,List*attrnos);
54+
List*stmtcols,List*icolumns,List*attrnos,
55+
boolstrip_indirection);
5556
staticOnConflictExpr*transformOnConflictClause(ParseState*pstate,
5657
OnConflictClause*onConflictClause);
5758
staticintcount_rowexpr_columns(ParseState*pstate,Node*expr);
@@ -619,7 +620,8 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
619620
/* Prepare row for assignment to target table */
620621
exprList=transformInsertRow(pstate,exprList,
621622
stmt->cols,
622-
icolumns,attrnos);
623+
icolumns,attrnos,
624+
false);
623625
}
624626
elseif (list_length(selectStmt->valuesLists)>1)
625627
{
@@ -663,10 +665,20 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
663665
exprLocation((Node*)sublist))));
664666
}
665667

666-
/* Prepare row for assignment to target table */
668+
/*
669+
* Prepare row for assignment to target table. We process any
670+
* indirection on the target column specs normally but then strip
671+
* off the resulting field/array assignment nodes, since we don't
672+
* want the parsed statement to contain copies of those in each
673+
* VALUES row. (It's annoying to have to transform the
674+
* indirection specs over and over like this, but avoiding it
675+
* would take some really messy refactoring of
676+
* transformAssignmentIndirection.)
677+
*/
667678
sublist=transformInsertRow(pstate,sublist,
668679
stmt->cols,
669-
icolumns,attrnos);
680+
icolumns,attrnos,
681+
true);
670682

671683
/*
672684
* We must assign collations now because assign_query_collations
@@ -717,6 +729,14 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
717729
* Generate list of Vars referencing the RTE
718730
*/
719731
expandRTE(rte,rtr->rtindex,0,-1, false,NULL,&exprList);
732+
733+
/*
734+
* Re-apply any indirection on the target column specs to the Vars
735+
*/
736+
exprList=transformInsertRow(pstate,exprList,
737+
stmt->cols,
738+
icolumns,attrnos,
739+
false);
720740
}
721741
else
722742
{
@@ -739,7 +759,8 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
739759
/* Prepare row for assignment to target table */
740760
exprList=transformInsertRow(pstate,exprList,
741761
stmt->cols,
742-
icolumns,attrnos);
762+
icolumns,attrnos,
763+
false);
743764
}
744765

745766
/*
@@ -808,12 +829,17 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
808829
/*
809830
* Prepare an INSERT row for assignment to the target table.
810831
*
811-
* The row might be either a VALUES row, or variables referencing a
812-
* sub-SELECT output.
832+
* exprlist: transformed expressions for source values; these might come from
833+
* a VALUES row, or be Vars referencing a sub-SELECT or VALUES RTE output.
834+
* stmtcols: original target-columns spec for INSERT (we just test for NIL)
835+
* icolumns: effective target-columns spec (list of ResTarget)
836+
* attrnos: integer column numbers (must be same length as icolumns)
837+
* strip_indirection: if true, remove any field/array assignment nodes
813838
*/
814839
staticList*
815840
transformInsertRow(ParseState*pstate,List*exprlist,
816-
List*stmtcols,List*icolumns,List*attrnos)
841+
List*stmtcols,List*icolumns,List*attrnos,
842+
boolstrip_indirection)
817843
{
818844
List*result;
819845
ListCell*lc;
@@ -879,6 +905,29 @@ transformInsertRow(ParseState *pstate, List *exprlist,
879905
col->indirection,
880906
col->location);
881907

908+
if (strip_indirection)
909+
{
910+
while (expr)
911+
{
912+
if (IsA(expr,FieldStore))
913+
{
914+
FieldStore*fstore= (FieldStore*)expr;
915+
916+
expr= (Expr*)linitial(fstore->newvals);
917+
}
918+
elseif (IsA(expr,ArrayRef))
919+
{
920+
ArrayRef*aref= (ArrayRef*)expr;
921+
922+
if (aref->refassgnexpr==NULL)
923+
break;
924+
expr=aref->refassgnexpr;
925+
}
926+
else
927+
break;
928+
}
929+
}
930+
882931
result=lappend(result,expr);
883932

884933
icols=lnext(icols);

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

Lines changed: 20 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -438,8 +438,7 @@ static void get_tablesample_def(TableSampleClause *tablesample,
438438
deparse_context*context);
439439
staticvoidget_opclass_name(Oidopclass,Oidactual_datatype,
440440
StringInfobuf);
441-
staticNode*processIndirection(Node*node,deparse_context*context,
442-
boolprintit);
441+
staticNode*processIndirection(Node*node,deparse_context*context);
443442
staticvoidprintSubscripts(ArrayRef*aref,deparse_context*context);
444443
staticchar*get_relation_name(Oidrelid);
445444
staticchar*generate_relation_name(Oidrelid,List*namespaces);
@@ -4551,11 +4550,9 @@ get_values_def(List *values_lists, deparse_context *context)
45514550
appendStringInfoChar(buf,',');
45524551

45534552
/*
4554-
* Strip any top-level nodes representing indirection assignments,
4555-
* then print the result. Whole-row Vars need special treatment.
4553+
* Print the value. Whole-row Vars need special treatment.
45564554
*/
4557-
get_rule_expr_toplevel(processIndirection(col,context, false),
4558-
context, false);
4555+
get_rule_expr_toplevel(col,context, false);
45594556
}
45604557
appendStringInfoChar(buf,')');
45614558
}
@@ -5512,7 +5509,6 @@ get_insert_query_def(Query *query, deparse_context *context)
55125509
RangeTblEntry*values_rte=NULL;
55135510
RangeTblEntry*rte;
55145511
char*sep;
5515-
ListCell*values_cell;
55165512
ListCell*l;
55175513
List*strippedexprs;
55185514

@@ -5563,17 +5559,9 @@ get_insert_query_def(Query *query, deparse_context *context)
55635559
quote_identifier(rte->alias->aliasname));
55645560

55655561
/*
5566-
* Add the insert-column-names list. To handle indirection properly, we
5567-
* need to look for indirection nodes in the top targetlist (if it's
5568-
* INSERT ... SELECT or INSERT ... single VALUES), or in the first
5569-
* expression list of the VALUES RTE (if it's INSERT ... multi VALUES). We
5570-
* assume that all the expression lists will have similar indirection in
5571-
* the latter case.
5562+
* Add the insert-column-names list. Any indirection decoration needed on
5563+
* the column names can be inferred from the top targetlist.
55725564
*/
5573-
if (values_rte)
5574-
values_cell=list_head((List*)linitial(values_rte->values_lists));
5575-
else
5576-
values_cell=NULL;
55775565
strippedexprs=NIL;
55785566
sep="";
55795567
if (query->targetList)
@@ -5599,20 +5587,14 @@ get_insert_query_def(Query *query, deparse_context *context)
55995587
/*
56005588
* Print any indirection needed (subfields or subscripts), and strip
56015589
* off the top-level nodes representing the indirection assignments.
5590+
* Add the stripped expressions to strippedexprs. (If it's a
5591+
* single-VALUES statement, the stripped expressions are the VALUES to
5592+
* print below. Otherwise they're just Vars and not really
5593+
* interesting.)
56025594
*/
5603-
if (values_cell)
5604-
{
5605-
/* we discard the stripped expression in this case */
5606-
processIndirection((Node*)lfirst(values_cell),context, true);
5607-
values_cell=lnext(values_cell);
5608-
}
5609-
else
5610-
{
5611-
/* we keep a list of the stripped expressions in this case */
5612-
strippedexprs=lappend(strippedexprs,
5613-
processIndirection((Node*)tle->expr,
5614-
context, true));
5615-
}
5595+
strippedexprs=lappend(strippedexprs,
5596+
processIndirection((Node*)tle->expr,
5597+
context));
56165598
}
56175599
if (query->targetList)
56185600
appendStringInfoString(buf,") ");
@@ -5891,7 +5873,7 @@ get_update_query_targetlist_def(Query *query, List *targetList,
58915873
* Print any indirection needed (subfields or subscripts), and strip
58925874
* off the top-level nodes representing the indirection assignments.
58935875
*/
5894-
expr=processIndirection((Node*)tle->expr,context, true);
5876+
expr=processIndirection((Node*)tle->expr,context);
58955877

58965878
/*
58975879
* If we're in a multiassignment, skip printing anything more, unless
@@ -7296,7 +7278,7 @@ get_rule_expr(Node *node, deparse_context *context,
72967278
* subscripting in immediate descendants. It returns the
72977279
* RHS expr that is actually being "assigned".
72987280
*/
7299-
refassgnexpr=processIndirection(node,context, true);
7281+
refassgnexpr=processIndirection(node,context);
73007282
appendStringInfoString(buf," := ");
73017283
get_rule_expr(refassgnexpr,context,showimplicit);
73027284
}
@@ -9561,12 +9543,12 @@ get_opclass_name(Oid opclass, Oid actual_datatype,
95619543
* processIndirection - take care of array and subfield assignment
95629544
*
95639545
* We strip any top-level FieldStore or assignment ArrayRef nodes that
9564-
* appear in the input,and return the subexpression that's to be assigned.
9565-
*If printit is true,wealso print out the appropriate decoration for the
9566-
*base column name (that the caller just printed).
9546+
* appear in the input,printing them as decoration for the base column
9547+
*name (whichweassume the caller just printed). Return the subexpression
9548+
* that's to be assigned.
95679549
*/
95689550
staticNode*
9569-
processIndirection(Node*node,deparse_context*context,boolprintit)
9551+
processIndirection(Node*node,deparse_context*context)
95709552
{
95719553
StringInfobuf=context->buf;
95729554

@@ -9594,8 +9576,7 @@ processIndirection(Node *node, deparse_context *context, bool printit)
95949576
Assert(list_length(fstore->fieldnums)==1);
95959577
fieldname=get_relid_attribute_name(typrelid,
95969578
linitial_int(fstore->fieldnums));
9597-
if (printit)
9598-
appendStringInfo(buf,".%s",quote_identifier(fieldname));
9579+
appendStringInfo(buf,".%s",quote_identifier(fieldname));
95999580

96009581
/*
96019582
* We ignore arg since it should be an uninteresting reference to
@@ -9609,8 +9590,7 @@ processIndirection(Node *node, deparse_context *context, bool printit)
96099590

96109591
if (aref->refassgnexpr==NULL)
96119592
break;
9612-
if (printit)
9613-
printSubscripts(aref,context);
9593+
printSubscripts(aref,context);
96149594

96159595
/*
96169596
* We ignore refexpr since it should be an uninteresting reference

‎src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/*yyyymmddN */
56-
#defineCATALOG_VERSION_NO201608021
56+
#defineCATALOG_VERSION_NO201608031
5757

5858
#endif

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,82 @@ select col1, col2, char_length(col3) from inserttest;
8181
(8 rows)
8282

8383
drop table inserttest;
84+
--
85+
-- check indirection (field/array assignment), cf bug #14265
86+
--
87+
-- these tests are aware that transformInsertStmt has 3 separate code paths
88+
--
89+
create type insert_test_type as (if1 int, if2 text[]);
90+
create table inserttest (f1 int, f2 int[],
91+
f3 insert_test_type, f4 insert_test_type[]);
92+
insert into inserttest (f2[1], f2[2]) values (1,2);
93+
insert into inserttest (f2[1], f2[2]) values (3,4), (5,6);
94+
insert into inserttest (f2[1], f2[2]) select 7,8;
95+
insert into inserttest (f2[1], f2[2]) values (1,default); -- not supported
96+
ERROR: cannot set an array element to DEFAULT
97+
LINE 1: insert into inserttest (f2[1], f2[2]) values (1,default);
98+
^
99+
insert into inserttest (f3.if1, f3.if2) values (1,array['foo']);
100+
insert into inserttest (f3.if1, f3.if2) values (1,'{foo}'), (2,'{bar}');
101+
insert into inserttest (f3.if1, f3.if2) select 3, '{baz,quux}';
102+
insert into inserttest (f3.if1, f3.if2) values (1,default); -- not supported
103+
ERROR: cannot set a subfield to DEFAULT
104+
LINE 1: insert into inserttest (f3.if1, f3.if2) values (1,default);
105+
^
106+
insert into inserttest (f3.if2[1], f3.if2[2]) values ('foo', 'bar');
107+
insert into inserttest (f3.if2[1], f3.if2[2]) values ('foo', 'bar'), ('baz', 'quux');
108+
insert into inserttest (f3.if2[1], f3.if2[2]) select 'bear', 'beer';
109+
insert into inserttest (f4[1].if2[1], f4[1].if2[2]) values ('foo', 'bar');
110+
insert into inserttest (f4[1].if2[1], f4[1].if2[2]) values ('foo', 'bar'), ('baz', 'quux');
111+
insert into inserttest (f4[1].if2[1], f4[1].if2[2]) select 'bear', 'beer';
112+
select * from inserttest;
113+
f1 | f2 | f3 | f4
114+
----+-------+------------------+------------------------
115+
| {1,2} | |
116+
| {3,4} | |
117+
| {5,6} | |
118+
| {7,8} | |
119+
| | (1,{foo}) |
120+
| | (1,{foo}) |
121+
| | (2,{bar}) |
122+
| | (3,"{baz,quux}") |
123+
| | (,"{foo,bar}") |
124+
| | (,"{foo,bar}") |
125+
| | (,"{baz,quux}") |
126+
| | (,"{bear,beer}") |
127+
| | | {"(,\"{foo,bar}\")"}
128+
| | | {"(,\"{foo,bar}\")"}
129+
| | | {"(,\"{baz,quux}\")"}
130+
| | | {"(,\"{bear,beer}\")"}
131+
(16 rows)
132+
133+
-- also check reverse-listing
134+
create table inserttest2 (f1 bigint, f2 text);
135+
create rule irule1 as on insert to inserttest2 do also
136+
insert into inserttest (f3.if2[1], f3.if2[2])
137+
values (new.f1,new.f2);
138+
create rule irule2 as on insert to inserttest2 do also
139+
insert into inserttest (f4[1].if1, f4[1].if2[2])
140+
values (1,'fool'),(new.f1,new.f2);
141+
create rule irule3 as on insert to inserttest2 do also
142+
insert into inserttest (f4[1].if1, f4[1].if2[2])
143+
select new.f1, new.f2;
144+
\d+ inserttest2
145+
Table "public.inserttest2"
146+
Column | Type | Modifiers | Storage | Stats target | Description
147+
--------+--------+-----------+----------+--------------+-------------
148+
f1 | bigint | | plain | |
149+
f2 | text | | extended | |
150+
Rules:
151+
irule1 AS
152+
ON INSERT TO inserttest2 DO INSERT INTO inserttest (f3.if2[1], f3.if2[2])
153+
VALUES (new.f1, new.f2)
154+
irule2 AS
155+
ON INSERT TO inserttest2 DO INSERT INTO inserttest (f4[1].if1, f4[1].if2[2]) VALUES (1,'fool'::text), (new.f1,new.f2)
156+
irule3 AS
157+
ON INSERT TO inserttest2 DO INSERT INTO inserttest (f4[1].if1, f4[1].if2[2]) SELECT new.f1,
158+
new.f2
159+
160+
drop table inserttest2;
161+
drop table inserttest;
162+
drop type insert_test_type;

‎src/test/regress/sql/insert.sql

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,51 @@ insert into inserttest values(30, 50, repeat('x', 10000));
3636
select col1, col2, char_length(col3)from inserttest;
3737

3838
droptable inserttest;
39+
40+
--
41+
-- check indirection (field/array assignment), cf bug #14265
42+
--
43+
-- these tests are aware that transformInsertStmt has 3 separate code paths
44+
--
45+
46+
createtypeinsert_test_typeas (if1int, if2text[]);
47+
48+
createtableinserttest (f1int, f2int[],
49+
f3 insert_test_type, f4 insert_test_type[]);
50+
51+
insert into inserttest (f2[1], f2[2])values (1,2);
52+
insert into inserttest (f2[1], f2[2])values (3,4), (5,6);
53+
insert into inserttest (f2[1], f2[2])select7,8;
54+
insert into inserttest (f2[1], f2[2])values (1,default);-- not supported
55+
56+
insert into inserttest (f3.if1,f3.if2)values (1,array['foo']);
57+
insert into inserttest (f3.if1,f3.if2)values (1,'{foo}'), (2,'{bar}');
58+
insert into inserttest (f3.if1,f3.if2)select3,'{baz,quux}';
59+
insert into inserttest (f3.if1,f3.if2)values (1,default);-- not supported
60+
61+
insert into inserttest (f3.if2[1],f3.if2[2])values ('foo','bar');
62+
insert into inserttest (f3.if2[1],f3.if2[2])values ('foo','bar'), ('baz','quux');
63+
insert into inserttest (f3.if2[1],f3.if2[2])select'bear','beer';
64+
65+
insert into inserttest (f4[1].if2[1], f4[1].if2[2])values ('foo','bar');
66+
insert into inserttest (f4[1].if2[1], f4[1].if2[2])values ('foo','bar'), ('baz','quux');
67+
insert into inserttest (f4[1].if2[1], f4[1].if2[2])select'bear','beer';
68+
69+
select*from inserttest;
70+
71+
-- also check reverse-listing
72+
createtableinserttest2 (f1bigint, f2text);
73+
createruleirule1ason insert to inserttest2 do also
74+
insert into inserttest (f3.if2[1],f3.if2[2])
75+
values (new.f1,new.f2);
76+
createruleirule2ason insert to inserttest2 do also
77+
insert into inserttest (f4[1].if1, f4[1].if2[2])
78+
values (1,'fool'),(new.f1,new.f2);
79+
createruleirule3ason insert to inserttest2 do also
80+
insert into inserttest (f4[1].if1, f4[1].if2[2])
81+
selectnew.f1,new.f2;
82+
\d+ inserttest2
83+
84+
droptable inserttest2;
85+
droptable inserttest;
86+
droptype insert_test_type;

0 commit comments

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