Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 6ccb973

Browse files
deanrasheeddeanrasheed
committed
Further fixing for multi-row VALUES lists for updatable views.
Previously, rewriteTargetListIU() generated a list of attribute numbers from the targetlist, which were passed to rewriteValuesRTE(), which expected them to contain the same number of entries as there are columns in the VALUES RTE, and to be in the same order. That was fine when the target relation was a table, but for an updatable view it could be broken in at least three different ways --- rewriteTargetListIU() could insert additional targetlist entries for view columns with defaults, the view columns could be in a different order from the columns of the underlying base relation, and targetlist entries could be merged together when assigning to elements of an array or composite type. As a result, when recursing to the base relation, the list of attribute numbers generated from the rewritten targetlist could no longer be relied upon to match the columns of the VALUES RTE. We got away with that prior to 41531e4 because it used to always be the case that rewriteValuesRTE() did nothing for the underlying base relation, since all DEFAULTS had already been replaced when it was initially invoked for the view, but that was incorrect because it failed to apply defaults from the base relation. Fix this by examining the targetlist entries more carefully and picking out just those that are simple Vars referencing the VALUES RTE. That's sufficient for the purposes of rewriteValuesRTE(), which is only responsible for dealing with DEFAULT items in the VALUES RTE. Any DEFAULT item in the VALUES RTE that doesn't have a matching simple-Var-assignment in the targetlist is an error which we complain about, but in theory that ought to be impossible. Additionally, move this code into rewriteValuesRTE() to give a clearer separation of concerns between the 2 functions. There is no need for rewriteTargetListIU() to know about the details of the VALUES RTE. While at it, fix the comment for rewriteValuesRTE() which claimed that it doesn't support array element and field assignments --- that hasn't been true since a3c7a99 (9.6 and later). Back-patch to all supported versions, with minor differences for the pre-9.6 branches, which don't support array element and field assignments to the same column in multi-row VALUES lists. Reviewed by Amit Langote. Discussion: https://postgr.es/m/15623-5d67a46788ec8b7f@postgresql.org
1 parent 0bf7f56 commit 6ccb973

File tree

3 files changed

+106
-43
lines changed

3 files changed

+106
-43
lines changed

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 59 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,13 @@ static List *rewriteTargetListIU(List *targetList,
6565
CmdType commandType,
6666
OverridingKind override,
6767
Relation target_relation,
68-
int result_rti,
69-
List **attrno_list);
68+
int result_rti);
7069
static TargetEntry *process_matched_tle(TargetEntry *src_tle,
7170
TargetEntry *prior_tle,
7271
const char *attrName);
7372
static Node *get_assignment_input(Node *node);
74-
static bool rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte,
75-
Relation target_relation, List *attrnos, bool force_nulls);
73+
static bool rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
74+
Relation target_relation, bool force_nulls);
7675
static void markQueryForLocking(Query *qry, Node *jtnode,
7776
LockClauseStrength strength, LockWaitPolicy waitPolicy,
7877
bool pushedDown);
@@ -701,11 +700,6 @@ adjustJoinTreeList(Query *parsetree, bool removert, int rt_index)
701700
* is not needed for rewriting, but will be needed by the planner, and we
702701
* can do it essentially for free while handling the other items.
703702
*
704-
* If attrno_list isn't NULL, we return an additional output besides the
705-
* rewritten targetlist: an integer list of the assigned-to attnums, in
706-
* order of the original tlist's non-junk entries. This is needed for
707-
* processing VALUES RTEs.
708-
*
709703
* Note that for an inheritable UPDATE, this processing is only done once,
710704
* using the parent relation as reference. It must not do anything that
711705
* will not be correct when transposed to the child relation(s). (Step 4
@@ -718,8 +712,7 @@ rewriteTargetListIU(List *targetList,
718712
CmdType commandType,
719713
OverridingKind override,
720714
Relation target_relation,
721-
int result_rti,
722-
List **attrno_list)
715+
int result_rti)
723716
{
724717
TargetEntry **new_tles;
725718
List *new_tlist = NIL;
@@ -730,9 +723,6 @@ rewriteTargetListIU(List *targetList,
730723
numattrs;
731724
ListCell *temp;
732725

733-
if (attrno_list) /* initialize optional result list */
734-
*attrno_list = NIL;
735-
736726
/*
737727
* We process the normal (non-junk) attributes by scanning the input tlist
738728
* once and transferring TLEs into an array, then scanning the array to
@@ -758,10 +748,6 @@ rewriteTargetListIU(List *targetList,
758748
elog(ERROR, "bogus resno %d in targetlist", attrno);
759749
att_tup = TupleDescAttr(target_relation->rd_att, attrno - 1);
760750

761-
/* put attrno into attrno_list even if it's dropped */
762-
if (attrno_list)
763-
*attrno_list = lappend_int(*attrno_list, attrno);
764-
765751
/* We can (and must) ignore deleted attributes */
766752
if (att_tup->attisdropped)
767753
continue;
@@ -1234,22 +1220,26 @@ searchForDefault(RangeTblEntry *rte)
12341220
* an insert into an auto-updatable view, and the product queries are inserts
12351221
* into a rule-updatable view.
12361222
*
1237-
* Note that we currently can't support subscripted or field assignment
1238-
* in the multi-VALUES case. The targetlist will contain simple Vars
1239-
* referencing the VALUES RTE, and therefore process_matched_tle() will
1240-
* reject any such attempt with "multiple assignments to same column".
1223+
* Note that we may have subscripted or field assignment targetlist entries,
1224+
* as well as more complex expressions from already-replaced DEFAULT items if
1225+
* we have recursed to here for an auto-updatable view. However, it ought to
1226+
* be impossible for such entries to have DEFAULTs assigned to them --- we
1227+
* should only have to replace DEFAULT items for targetlist entries that
1228+
* contain simple Vars referencing the VALUES RTE.
12411229
*
12421230
* Returns true if all DEFAULT items were replaced, and false if some were
12431231
* left untouched.
12441232
*/
12451233
static bool
1246-
rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte,
1247-
Relation target_relation, List *attrnos, bool force_nulls)
1234+
rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
1235+
Relation target_relation, bool force_nulls)
12481236
{
12491237
List *newValues;
12501238
ListCell *lc;
12511239
bool isAutoUpdatableView;
12521240
bool allReplaced;
1241+
int numattrs;
1242+
int *attrnos;
12531243

12541244
/*
12551245
* Rebuilding all the lists is a pretty expensive proposition in a big
@@ -1262,8 +1252,33 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte,
12621252
if (!force_nulls && !searchForDefault(rte))
12631253
return true; /* nothing to do */
12641254

1265-
/* Check list lengths (we can assume all the VALUES sublists are alike) */
1266-
Assert(list_length(attrnos) == list_length(linitial(rte->values_lists)));
1255+
/*
1256+
* Scan the targetlist for entries referring to the VALUES RTE, and note
1257+
* the target attributes. As noted above, we should only need to do this
1258+
* for targetlist entries containing simple Vars --- nothing else in the
1259+
* VALUES RTE should contain DEFAULT items, and we complain if such a
1260+
* thing does occur.
1261+
*/
1262+
numattrs = list_length(linitial(rte->values_lists));
1263+
attrnos = (int *) palloc0(numattrs * sizeof(int));
1264+
1265+
foreach(lc, parsetree->targetList)
1266+
{
1267+
TargetEntry *tle = (TargetEntry *) lfirst(lc);
1268+
1269+
if (IsA(tle->expr, Var))
1270+
{
1271+
Var *var = (Var *) tle->expr;
1272+
1273+
if (var->varno == rti)
1274+
{
1275+
int attrno = var->varattno;
1276+
1277+
Assert(attrno >= 1 && attrno <= numattrs);
1278+
attrnos[attrno - 1] = tle->resno;
1279+
}
1280+
}
1281+
}
12671282

12681283
/*
12691284
* Check if the target relation is an auto-updatable view, in which case
@@ -1314,18 +1329,23 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte,
13141329
List *sublist = (List *) lfirst(lc);
13151330
List *newList = NIL;
13161331
ListCell *lc2;
1317-
ListCell *lc3;
1332+
int i;
1333+
1334+
Assert(list_length(sublist) == numattrs);
13181335

1319-
forboth(lc2, sublist, lc3, attrnos)
1336+
i = 0;
1337+
foreach(lc2, sublist)
13201338
{
13211339
Node *col = (Node *) lfirst(lc2);
1322-
int attrno = lfirst_int(lc3);
1340+
int attrno = attrnos[i++];
13231341

13241342
if (IsA(col, SetToDefault))
13251343
{
13261344
Form_pg_attribute att_tup;
13271345
Node *new_expr;
13281346

1347+
if (attrno == 0)
1348+
elog(ERROR, "cannot set value in column %d to DEFAULT", i);
13291349
att_tup = TupleDescAttr(target_relation->rd_att, attrno - 1);
13301350

13311351
if (!force_nulls && !att_tup->attisdropped)
@@ -1373,6 +1393,8 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte,
13731393
}
13741394
rte->values_lists = newValues;
13751395

1396+
pfree(attrnos);
1397+
13761398
return allReplaced;
13771399
}
13781400

@@ -3451,7 +3473,6 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
34513473
List *locks;
34523474
List *product_queries;
34533475
bool hasUpdate = false;
3454-
List *attrnos = NIL;
34553476
int values_rte_index = 0;
34563477
bool defaults_remaining = false;
34573478

@@ -3501,11 +3522,10 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
35013522
parsetree->commandType,
35023523
parsetree->override,
35033524
rt_entry_relation,
3504-
parsetree->resultRelation,
3505-
&attrnos);
3525+
parsetree->resultRelation);
35063526
/* ... and the VALUES expression lists */
3507-
if (!rewriteValuesRTE(parsetree, values_rte,
3508-
rt_entry_relation, attrnos, false))
3527+
if (!rewriteValuesRTE(parsetree, values_rte, values_rte_index,
3528+
rt_entry_relation, false))
35093529
defaults_remaining = true;
35103530
}
35113531
else
@@ -3516,7 +3536,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
35163536
parsetree->commandType,
35173537
parsetree->override,
35183538
rt_entry_relation,
3519-
parsetree->resultRelation, NULL);
3539+
parsetree->resultRelation);
35203540
}
35213541

35223542
if (parsetree->onConflict &&
@@ -3527,8 +3547,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
35273547
CMD_UPDATE,
35283548
parsetree->override,
35293549
rt_entry_relation,
3530-
parsetree->resultRelation,
3531-
NULL);
3550+
parsetree->resultRelation);
35323551
}
35333552
}
35343553
else if (event == CMD_UPDATE)
@@ -3538,7 +3557,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
35383557
parsetree->commandType,
35393558
parsetree->override,
35403559
rt_entry_relation,
3541-
parsetree->resultRelation, NULL);
3560+
parsetree->resultRelation);
35423561
}
35433562
else if (event == CMD_DELETE)
35443563
{
@@ -3583,7 +3602,8 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
35833602
RangeTblEntry *values_rte = rt_fetch(values_rte_index,
35843603
pt->rtable);
35853604

3586-
rewriteValuesRTE(pt, values_rte, rt_entry_relation, attrnos,
3605+
rewriteValuesRTE(pt, values_rte, values_rte_index,
3606+
rt_entry_relation,
35873607
true); /* Force remaining defaults to NULL */
35883608
}
35893609
}

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

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,6 +2789,7 @@ insert into base_tab_def_view values (12), (13);
27892789
insert into base_tab_def_view values (14, default, default, default, default);
27902790
insert into base_tab_def_view values (15, default, default, default, default),
27912791
(16, default, default, default, default);
2792+
insert into base_tab_def_view values (17), (default);
27922793
select * from base_tab_def order by a;
27932794
a | b | c | d | e
27942795
----+---------------+---------------+--------------+---
@@ -2804,7 +2805,9 @@ select * from base_tab_def order by a;
28042805
14 | View default | Table default | View default |
28052806
15 | View default | Table default | View default |
28062807
16 | View default | Table default | View default |
2807-
(12 rows)
2808+
17 | View default | Table default | View default |
2809+
| View default | Table default | View default |
2810+
(14 rows)
28082811

28092812
-- Adding an INSTEAD OF trigger should cause NULLs to be inserted instead of
28102813
-- table defaults, where there are no view defaults.
@@ -2830,6 +2833,7 @@ insert into base_tab_def_view values (12), (13);
28302833
insert into base_tab_def_view values (14, default, default, default, default);
28312834
insert into base_tab_def_view values (15, default, default, default, default),
28322835
(16, default, default, default, default);
2836+
insert into base_tab_def_view values (17), (default);
28332837
select * from base_tab_def order by a;
28342838
a | b | c | d | e
28352839
----+---------------+---------------+--------------+---
@@ -2845,7 +2849,9 @@ select * from base_tab_def order by a;
28452849
14 | View default | | View default |
28462850
15 | View default | | View default |
28472851
16 | View default | | View default |
2848-
(12 rows)
2852+
17 | View default | | View default |
2853+
| View default | | View default |
2854+
(14 rows)
28492855

28502856
-- Using an unconditional DO INSTEAD rule should also cause NULLs to be
28512857
-- inserted where there are no view defaults.
@@ -2864,6 +2870,7 @@ insert into base_tab_def_view values (12), (13);
28642870
insert into base_tab_def_view values (14, default, default, default, default);
28652871
insert into base_tab_def_view values (15, default, default, default, default),
28662872
(16, default, default, default, default);
2873+
insert into base_tab_def_view values (17), (default);
28672874
select * from base_tab_def order by a;
28682875
a | b | c | d | e
28692876
----+---------------+---------------+--------------+---
@@ -2879,7 +2886,9 @@ select * from base_tab_def order by a;
28792886
14 | View default | | View default |
28802887
15 | View default | | View default |
28812888
16 | View default | | View default |
2882-
(12 rows)
2889+
17 | View default | | View default |
2890+
| View default | | View default |
2891+
(14 rows)
28832892

28842893
-- A DO ALSO rule should cause each row to be inserted twice. The first
28852894
-- insert should behave the same as an auto-updatable view (using table
@@ -2900,6 +2909,7 @@ insert into base_tab_def_view values (12), (13);
29002909
insert into base_tab_def_view values (14, default, default, default, default);
29012910
insert into base_tab_def_view values (15, default, default, default, default),
29022911
(16, default, default, default, default);
2912+
insert into base_tab_def_view values (17), (default);
29032913
select * from base_tab_def order by a, c NULLS LAST;
29042914
a | b | c | d | e
29052915
----+---------------+---------------+--------------+---
@@ -2921,7 +2931,26 @@ select * from base_tab_def order by a, c NULLS LAST;
29212931
15 | View default | | View default |
29222932
16 | View default | Table default | View default |
29232933
16 | View default | | View default |
2924-
(18 rows)
2934+
17 | View default | Table default | View default |
2935+
17 | View default | | View default |
2936+
| View default | Table default | View default |
2937+
| View default | | View default |
2938+
(22 rows)
29252939

29262940
drop view base_tab_def_view;
29272941
drop table base_tab_def;
2942+
-- Test defaults with array assignments
2943+
create table base_tab (a serial, b int[], c text, d text default 'Table default');
2944+
create view base_tab_view as select c, a, b from base_tab;
2945+
alter view base_tab_view alter column c set default 'View default';
2946+
insert into base_tab_view (b[1], b[2], c, b[5], b[4], a, b[3])
2947+
values (1, 2, default, 5, 4, default, 3), (10, 11, 'C value', 14, 13, 100, 12);
2948+
select * from base_tab order by a;
2949+
a | b | c | d
2950+
-----+------------------+--------------+---------------
2951+
1 | {1,2,3,4,5} | View default | Table default
2952+
100 | {10,11,12,13,14} | C value | Table default
2953+
(2 rows)
2954+
2955+
drop view base_tab_view;
2956+
drop table base_tab;

‎src/test/regress/sql/updatable_views.sql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1397,6 +1397,7 @@ insert into base_tab_def_view values (12), (13);
13971397
insert into base_tab_def_view values (14, default, default, default, default);
13981398
insert into base_tab_def_view values (15, default, default, default, default),
13991399
(16, default, default, default, default);
1400+
insert into base_tab_def_view values (17), (default);
14001401
select * from base_tab_def order by a;
14011402

14021403
-- Adding an INSTEAD OF trigger should cause NULLs to be inserted instead of
@@ -1423,6 +1424,7 @@ insert into base_tab_def_view values (12), (13);
14231424
insert into base_tab_def_view values (14, default, default, default, default);
14241425
insert into base_tab_def_view values (15, default, default, default, default),
14251426
(16, default, default, default, default);
1427+
insert into base_tab_def_view values (17), (default);
14261428
select * from base_tab_def order by a;
14271429

14281430
-- Using an unconditional DO INSTEAD rule should also cause NULLs to be
@@ -1442,6 +1444,7 @@ insert into base_tab_def_view values (12), (13);
14421444
insert into base_tab_def_view values (14, default, default, default, default);
14431445
insert into base_tab_def_view values (15, default, default, default, default),
14441446
(16, default, default, default, default);
1447+
insert into base_tab_def_view values (17), (default);
14451448
select * from base_tab_def order by a;
14461449

14471450
-- A DO ALSO rule should cause each row to be inserted twice. The first
@@ -1463,7 +1466,18 @@ insert into base_tab_def_view values (12), (13);
14631466
insert into base_tab_def_view values (14, default, default, default, default);
14641467
insert into base_tab_def_view values (15, default, default, default, default),
14651468
(16, default, default, default, default);
1469+
insert into base_tab_def_view values (17), (default);
14661470
select * from base_tab_def order by a, c NULLS LAST;
14671471

14681472
drop view base_tab_def_view;
14691473
drop table base_tab_def;
1474+
1475+
-- Test defaults with array assignments
1476+
create table base_tab (a serial, b int[], c text, d text default 'Table default');
1477+
create view base_tab_view as select c, a, b from base_tab;
1478+
alter view base_tab_view alter column c set default 'View default';
1479+
insert into base_tab_view (b[1], b[2], c, b[5], b[4], a, b[3])
1480+
values (1, 2, default, 5, 4, default, 3), (10, 11, 'C value', 14, 13, 100, 12);
1481+
select * from base_tab order by a;
1482+
drop view base_tab_view;
1483+
drop table base_tab;

0 commit comments

Comments
 (0)