Skip to content

Commit 431471e

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 00382f3 commit 431471e

File tree

3 files changed

+105
-40
lines changed

3 files changed

+105
-40
lines changed

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 58 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,13 @@ static Query *rewriteRuleAction(Query *parsetree,
5151
CmdType event,
5252
bool *returning_flag);
5353
static List *adjustJoinTreeList(Query *parsetree, bool removert, int rt_index);
54-
static void rewriteTargetListIU(Query *parsetree, Relation target_relation,
55-
List **attrno_list);
54+
static void rewriteTargetListIU(Query *parsetree, Relation target_relation);
5655
static TargetEntry *process_matched_tle(TargetEntry *src_tle,
5756
TargetEntry *prior_tle,
5857
const char *attrName);
5958
static Node *get_assignment_input(Node *node);
60-
static bool rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte,
61-
Relation target_relation, List *attrnos, bool force_nulls);
59+
static bool rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
60+
Relation target_relation, bool force_nulls);
6261
static void rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte,
6362
Relation target_relation);
6463
static void markQueryForLocking(Query *qry, Node *jtnode,
@@ -671,15 +670,9 @@ adjustJoinTreeList(Query *parsetree, bool removert, int rt_index)
671670
* references to NEW.foo will produce wrong or incomplete results. Item 4
672671
* is not needed for rewriting, but will be needed by the planner, and we
673672
* can do it essentially for free while handling the other items.
674-
*
675-
* If attrno_list isn't NULL, we return an additional output besides the
676-
* rewritten targetlist: an integer list of the assigned-to attnums, in
677-
* order of the original tlist's non-junk entries. This is needed for
678-
* processing VALUES RTEs.
679673
*/
680674
static void
681-
rewriteTargetListIU(Query *parsetree, Relation target_relation,
682-
List **attrno_list)
675+
rewriteTargetListIU(Query *parsetree, Relation target_relation)
683676
{
684677
CmdType commandType = parsetree->commandType;
685678
TargetEntry **new_tles;
@@ -691,9 +684,6 @@ rewriteTargetListIU(Query *parsetree, Relation target_relation,
691684
numattrs;
692685
ListCell *temp;
693686

694-
if (attrno_list) /* initialize optional result list */
695-
*attrno_list = NIL;
696-
697687
/*
698688
* We process the normal (non-junk) attributes by scanning the input tlist
699689
* once and transferring TLEs into an array, then scanning the array to
@@ -719,10 +709,6 @@ rewriteTargetListIU(Query *parsetree, Relation target_relation,
719709
elog(ERROR, "bogus resno %d in targetlist", attrno);
720710
att_tup = target_relation->rd_att->attrs[attrno - 1];
721711

722-
/* put attrno into attrno_list even if it's dropped */
723-
if (attrno_list)
724-
*attrno_list = lappend_int(*attrno_list, attrno);
725-
726712
/* We can (and must) ignore deleted attributes */
727713
if (att_tup->attisdropped)
728714
continue;
@@ -1154,22 +1140,26 @@ searchForDefault(RangeTblEntry *rte)
11541140
* an insert into an auto-updatable view, and the product queries are inserts
11551141
* into a rule-updatable view.
11561142
*
1157-
* Note that we currently can't support subscripted or field assignment
1158-
* in the multi-VALUES case. The targetlist will contain simple Vars
1159-
* referencing the VALUES RTE, and therefore process_matched_tle() will
1160-
* reject any such attempt with "multiple assignments to same column".
1143+
* Note that we may have subscripted or field assignment targetlist entries,
1144+
* as well as more complex expressions from already-replaced DEFAULT items if
1145+
* we have recursed to here for an auto-updatable view. However, it ought to
1146+
* be impossible for such entries to have DEFAULTs assigned to them --- we
1147+
* should only have to replace DEFAULT items for targetlist entries that
1148+
* contain simple Vars referencing the VALUES RTE.
11611149
*
11621150
* Returns true if all DEFAULT items were replaced, and false if some were
11631151
* left untouched.
11641152
*/
11651153
static bool
1166-
rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte,
1167-
Relation target_relation, List *attrnos, bool force_nulls)
1154+
rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
1155+
Relation target_relation, bool force_nulls)
11681156
{
11691157
List *newValues;
11701158
ListCell *lc;
11711159
bool isAutoUpdatableView;
11721160
bool allReplaced;
1161+
int numattrs;
1162+
int *attrnos;
11731163

11741164
/*
11751165
* Rebuilding all the lists is a pretty expensive proposition in a big
@@ -1182,8 +1172,33 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte,
11821172
if (!force_nulls && !searchForDefault(rte))
11831173
return true; /* nothing to do */
11841174

1185-
/* Check list lengths (we can assume all the VALUES sublists are alike) */
1186-
Assert(list_length(attrnos) == list_length(linitial(rte->values_lists)));
1175+
/*
1176+
* Scan the targetlist for entries referring to the VALUES RTE, and note
1177+
* the target attributes. As noted above, we should only need to do this
1178+
* for targetlist entries containing simple Vars --- nothing else in the
1179+
* VALUES RTE should contain DEFAULT items, and we complain if such a
1180+
* thing does occur.
1181+
*/
1182+
numattrs = list_length(linitial(rte->values_lists));
1183+
attrnos = (int *) palloc0(numattrs * sizeof(int));
1184+
1185+
foreach(lc, parsetree->targetList)
1186+
{
1187+
TargetEntry *tle = (TargetEntry *) lfirst(lc);
1188+
1189+
if (IsA(tle->expr, Var))
1190+
{
1191+
Var *var = (Var *) tle->expr;
1192+
1193+
if (var->varno == rti)
1194+
{
1195+
int attrno = var->varattno;
1196+
1197+
Assert(attrno >= 1 && attrno <= numattrs);
1198+
attrnos[attrno - 1] = tle->resno;
1199+
}
1200+
}
1201+
}
11871202

11881203
/*
11891204
* Check if the target relation is an auto-updatable view, in which case
@@ -1233,18 +1248,23 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte,
12331248
List *sublist = (List *) lfirst(lc);
12341249
List *newList = NIL;
12351250
ListCell *lc2;
1236-
ListCell *lc3;
1251+
int i;
1252+
1253+
Assert(list_length(sublist) == numattrs);
12371254

1238-
forboth(lc2, sublist, lc3, attrnos)
1255+
i = 0;
1256+
foreach(lc2, sublist)
12391257
{
12401258
Node *col = (Node *) lfirst(lc2);
1241-
int attrno = lfirst_int(lc3);
1259+
int attrno = attrnos[i++];
12421260

12431261
if (IsA(col, SetToDefault))
12441262
{
12451263
Form_pg_attribute att_tup;
12461264
Node *new_expr;
12471265

1266+
if (attrno == 0)
1267+
elog(ERROR, "cannot set value in column %d to DEFAULT", i);
12481268
att_tup = target_relation->rd_att->attrs[attrno - 1];
12491269

12501270
if (!force_nulls && !att_tup->attisdropped)
@@ -1292,6 +1312,8 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte,
12921312
}
12931313
rte->values_lists = newValues;
12941314

1315+
pfree(attrnos);
1316+
12951317
return allReplaced;
12961318
}
12971319

@@ -3146,7 +3168,6 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
31463168
Relation rt_entry_relation;
31473169
List *locks;
31483170
List *product_queries;
3149-
List *attrnos = NIL;
31503171
int values_rte_index = 0;
31513172
bool defaults_remaining = false;
31523173

@@ -3192,21 +3213,21 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
31923213
if (values_rte)
31933214
{
31943215
/* Process the main targetlist ... */
3195-
rewriteTargetListIU(parsetree, rt_entry_relation, &attrnos);
3216+
rewriteTargetListIU(parsetree, rt_entry_relation);
31963217
/* ... and the VALUES expression lists */
3197-
if (!rewriteValuesRTE(parsetree, values_rte,
3198-
rt_entry_relation, attrnos, false))
3218+
if (!rewriteValuesRTE(parsetree, values_rte, values_rte_index,
3219+
rt_entry_relation, false))
31993220
defaults_remaining = true;
32003221
}
32013222
else
32023223
{
32033224
/* Process just the main targetlist */
3204-
rewriteTargetListIU(parsetree, rt_entry_relation, NULL);
3225+
rewriteTargetListIU(parsetree, rt_entry_relation);
32053226
}
32063227
}
32073228
else if (event == CMD_UPDATE)
32083229
{
3209-
rewriteTargetListIU(parsetree, rt_entry_relation, NULL);
3230+
rewriteTargetListIU(parsetree, rt_entry_relation);
32103231
rewriteTargetListUD(parsetree, rt_entry, rt_entry_relation);
32113232
}
32123233
else if (event == CMD_DELETE)
@@ -3252,7 +3273,8 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
32523273
RangeTblEntry *values_rte = rt_fetch(values_rte_index,
32533274
pt->rtable);
32543275

3255-
rewriteValuesRTE(pt, values_rte, rt_entry_relation, attrnos,
3276+
rewriteValuesRTE(pt, values_rte, values_rte_index,
3277+
rt_entry_relation,
32563278
true); /* Force remaining defaults to NULL */
32573279
}
32583280
}

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

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2420,6 +2420,7 @@ insert into base_tab_def_view values (12), (13);
24202420
insert into base_tab_def_view values (14, default, default, default, default);
24212421
insert into base_tab_def_view values (15, default, default, default, default),
24222422
(16, default, default, default, default);
2423+
insert into base_tab_def_view values (17), (default);
24232424
select * from base_tab_def order by a;
24242425
a | b | c | d | e
24252426
----+---------------+---------------+--------------+---
@@ -2435,7 +2436,9 @@ select * from base_tab_def order by a;
24352436
14 | View default | Table default | View default |
24362437
15 | View default | Table default | View default |
24372438
16 | View default | Table default | View default |
2438-
(12 rows)
2439+
17 | View default | Table default | View default |
2440+
| View default | Table default | View default |
2441+
(14 rows)
24392442

24402443
-- Adding an INSTEAD OF trigger should cause NULLs to be inserted instead of
24412444
-- table defaults, where there are no view defaults.
@@ -2461,6 +2464,7 @@ insert into base_tab_def_view values (12), (13);
24612464
insert into base_tab_def_view values (14, default, default, default, default);
24622465
insert into base_tab_def_view values (15, default, default, default, default),
24632466
(16, default, default, default, default);
2467+
insert into base_tab_def_view values (17), (default);
24642468
select * from base_tab_def order by a;
24652469
a | b | c | d | e
24662470
----+---------------+---------------+--------------+---
@@ -2476,7 +2480,9 @@ select * from base_tab_def order by a;
24762480
14 | View default | | View default |
24772481
15 | View default | | View default |
24782482
16 | View default | | View default |
2479-
(12 rows)
2483+
17 | View default | | View default |
2484+
| View default | | View default |
2485+
(14 rows)
24802486

24812487
-- Using an unconditional DO INSTEAD rule should also cause NULLs to be
24822488
-- inserted where there are no view defaults.
@@ -2495,6 +2501,7 @@ insert into base_tab_def_view values (12), (13);
24952501
insert into base_tab_def_view values (14, default, default, default, default);
24962502
insert into base_tab_def_view values (15, default, default, default, default),
24972503
(16, default, default, default, default);
2504+
insert into base_tab_def_view values (17), (default);
24982505
select * from base_tab_def order by a;
24992506
a | b | c | d | e
25002507
----+---------------+---------------+--------------+---
@@ -2510,7 +2517,9 @@ select * from base_tab_def order by a;
25102517
14 | View default | | View default |
25112518
15 | View default | | View default |
25122519
16 | View default | | View default |
2513-
(12 rows)
2520+
17 | View default | | View default |
2521+
| View default | | View default |
2522+
(14 rows)
25142523

25152524
-- A DO ALSO rule should cause each row to be inserted twice. The first
25162525
-- insert should behave the same as an auto-updatable view (using table
@@ -2531,6 +2540,7 @@ insert into base_tab_def_view values (12), (13);
25312540
insert into base_tab_def_view values (14, default, default, default, default);
25322541
insert into base_tab_def_view values (15, default, default, default, default),
25332542
(16, default, default, default, default);
2543+
insert into base_tab_def_view values (17), (default);
25342544
select * from base_tab_def order by a, c NULLS LAST;
25352545
a | b | c | d | e
25362546
----+---------------+---------------+--------------+---
@@ -2552,7 +2562,26 @@ select * from base_tab_def order by a, c NULLS LAST;
25522562
15 | View default | | View default |
25532563
16 | View default | Table default | View default |
25542564
16 | View default | | View default |
2555-
(18 rows)
2565+
17 | View default | Table default | View default |
2566+
17 | View default | | View default |
2567+
| View default | Table default | View default |
2568+
| View default | | View default |
2569+
(22 rows)
25562570

25572571
drop view base_tab_def_view;
25582572
drop table base_tab_def;
2573+
-- Test defaults with array assignments
2574+
create table base_tab (a serial, b int[], c text, d text default 'Table default');
2575+
create view base_tab_view as select c, a, b from base_tab;
2576+
alter view base_tab_view alter column c set default 'View default';
2577+
insert into base_tab_view (b[1], c, a)
2578+
values (1, default, default), (10, 'C value', 100);
2579+
select * from base_tab order by a;
2580+
a | b | c | d
2581+
-----+------+--------------+---------------
2582+
1 | {1} | View default | Table default
2583+
100 | {10} | C value | Table default
2584+
(2 rows)
2585+
2586+
drop view base_tab_view;
2587+
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
@@ -1121,6 +1121,7 @@ insert into base_tab_def_view values (12), (13);
11211121
insert into base_tab_def_view values (14, default, default, default, default);
11221122
insert into base_tab_def_view values (15, default, default, default, default),
11231123
(16, default, default, default, default);
1124+
insert into base_tab_def_view values (17), (default);
11241125
select * from base_tab_def order by a;
11251126

11261127
-- Adding an INSTEAD OF trigger should cause NULLs to be inserted instead of
@@ -1147,6 +1148,7 @@ insert into base_tab_def_view values (12), (13);
11471148
insert into base_tab_def_view values (14, default, default, default, default);
11481149
insert into base_tab_def_view values (15, default, default, default, default),
11491150
(16, default, default, default, default);
1151+
insert into base_tab_def_view values (17), (default);
11501152
select * from base_tab_def order by a;
11511153

11521154
-- Using an unconditional DO INSTEAD rule should also cause NULLs to be
@@ -1166,6 +1168,7 @@ insert into base_tab_def_view values (12), (13);
11661168
insert into base_tab_def_view values (14, default, default, default, default);
11671169
insert into base_tab_def_view values (15, default, default, default, default),
11681170
(16, default, default, default, default);
1171+
insert into base_tab_def_view values (17), (default);
11691172
select * from base_tab_def order by a;
11701173

11711174
-- A DO ALSO rule should cause each row to be inserted twice. The first
@@ -1187,7 +1190,18 @@ insert into base_tab_def_view values (12), (13);
11871190
insert into base_tab_def_view values (14, default, default, default, default);
11881191
insert into base_tab_def_view values (15, default, default, default, default),
11891192
(16, default, default, default, default);
1193+
insert into base_tab_def_view values (17), (default);
11901194
select * from base_tab_def order by a, c NULLS LAST;
11911195

11921196
drop view base_tab_def_view;
11931197
drop table base_tab_def;
1198+
1199+
-- Test defaults with array assignments
1200+
create table base_tab (a serial, b int[], c text, d text default 'Table default');
1201+
create view base_tab_view as select c, a, b from base_tab;
1202+
alter view base_tab_view alter column c set default 'View default';
1203+
insert into base_tab_view (b[1], c, a)
1204+
values (1, default, default), (10, 'C value', 100);
1205+
select * from base_tab order by a;
1206+
drop view base_tab_view;
1207+
drop table base_tab;

0 commit comments

Comments
 (0)