8000 Yet further fixes for multi-row VALUES lists for updatable views. · postgres/postgres@b8f2687 · GitHub
[go: up one dir, main page]
Skip to content

Commit b8f2687

Browse files
tglsfdctglsfdc
committed
Yet further fixes for multi-row VALUES lists for updatable views.
DEFAULT markers appearing in an INSERT on an updatable view could be mis-processed if they were in a multi-row VALUES clause. This would lead to strange errors such as "cache lookup failed for type NNNN", or in older branches even to crashes. The cause is that commit 41531e4 tried to re-use rewriteValuesRTE() to remove any SetToDefault nodes (that hadn't previously been replaced by the view's own default values) appearing in "product" queries, that is DO ALSO queries. That's fundamentally wrong because the DO ALSO queries might not even be INSERTs; and even if they are, their targetlists don't necessarily match the view's column list, so that almost all the logic in rewriteValuesRTE() is inapplicable. What we want is a narrow focus on replacing any such nodes with NULL constants. (That is, in this context we are interpreting the defaults as being strictly those of the view itself; and we already replaced any that aren't NULL.) We could add still more !force_nulls tests to further lobotomize rewriteValuesRTE(); but it seems cleaner to split out this case to a new function, restoring rewriteValuesRTE() to the charter it had before. Per bug #17633 from jiye_sw. Patch by me, but thanks to Richard Guo and Japin Li for initial investigation. Back-patch to all supported branches, as the previous fix was. Discussion: https://postgr.es/m/17633-98cc85e1fa91e905@postgresql.org
1 parent 422f86a commit b8f2687

File tree

3 files changed

+98
-28
lines changed

3 files changed

+98
-28
lines changed

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 59 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,9 @@ static TargetEntry *process_matched_tle(TargetEntry *src_tle,
7979
static Node *get_assignment_input(Node *node);
8080
static Bitmapset *findDefaultOnlyColumns(RangeTblEntry *rte);
8181
static bool rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
82-
Relation target_relation, bool force_nulls,
82+
Relation target_relation,
8383
Bitmapset *unused_cols);
84+
static void rewriteValuesRTEToNulls(Query *parsetree, RangeTblEntry *rte);
8485
static void markQueryForLocking(Query *qry, Node *jtnode,
8586
LockClauseStrength strength, LockWaitPolicy waitPolicy,
8687
bool pushedDown);
@@ -1370,18 +1371,7 @@ findDefaultOnlyColumns(RangeTblEntry *rte)
13701371
* all DEFAULT items are replaced, and if the target relation doesn't have a
13711372
* default, the value is explicitly set to NULL.
13721373
*
1373-
* Additionally, if force_nulls is true, the target relation's defaults are
1374-
* ignored and all DEFAULT items in the VALUES list are explicitly set to
1375-
* NULL, regardless of the target relation's type. This is used for the
1376-
* product queries generated by DO ALSO rules attached to an auto-updatable
1377-
* view, for which we will have already called this function with force_nulls
1378-
* false. For these product queries, we must then force any remaining DEFAULT
1379-
* items to NULL to provide concrete values for the rule actions.
1380-
* Essentially, this is a mix of the 2 cases above --- the original query is
1381-
* an insert into an auto-updatable view, and the product queries are inserts
1382-
* into a rule-updatable view.
1383-
*
1384-
* Finally, if a DEFAULT item is found in a column mentioned in unused_cols,
1374+
* Also, if a DEFAULT item is found in a column mentioned in unused_cols,
13851375
* it is explicitly set to NULL. This happens for columns in the VALUES RTE
13861376
* whose corresponding targetlist entries have already been replaced with the
13871377
* relation's default expressions, so that any values in those columns of the
@@ -1404,7 +1394,7 @@ findDefaultOnlyColumns(RangeTblEntry *rte)
14041394
*/
14051395
static bool
14061396
rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
1407-
Relation target_relation, bool force_nulls,
1397+
Relation target_relation,
14081398
Bitmapset *unused_cols)
14091399
{
14101400
List *newValues;
@@ -1414,15 +1404,16 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
14141404
int numattrs;
14151405
int *attrnos;
14161406

1407+
/* Steps below are not sensible for non-INSERT queries */
1408+
Assert(parsetree->commandType == CMD_INSERT);
1409+
Assert(rte->rtekind == RTE_VALUES);
1410+
14171411
/*
14181412
* Rebuilding all the lists is a pretty expensive proposition in a big
14191413
* VALUES list, and it's a waste of time if there aren't any DEFAULT
14201414
* placeholders. So first scan to see if there are any.
1421-
*
1422-
* We skip this check if force_nulls is true, because we know that there
1423-
* are DEFAULT items present in that case.
14241415
*/
1425-
if (!force_nulls && !searchForDefault(rte))
1416+
if (!searchForDefault(rte))
14261417
return true; /* nothing to do */
14271418

14281419
/*
@@ -1456,12 +1447,10 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
14561447
/*
14571448
* Check if the target relation is an auto-updatable view, in which case
14581449
* unresolved defaults will be left untouched rather than being set to
1459-
* NULL. If force_nulls is true, we always set DEFAULT items to NULL, so
1460-
* skip this check in that case --- it isn't an auto-updatable view.
1450+
* NULL.
14611451
*/
14621452
isAutoUpdatableView = false;
1463-
if (!force_nulls &&
1464-
target_relation->rd_rel->relkind == RELKIND_VIEW &&
1453+
if (target_relation->rd_rel->relkind == RELKIND_VIEW &&
14651454
!view_has_instead_trigger(target_relation, CMD_INSERT))
14661455
{
14671456
List *locks;
@@ -1535,9 +1524,10 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
15351524

15361525
if (attrno == 0)
15371526
elog(ERROR, "cannot set value in column %d to DEFAULT", i);
1527+
Assert(attrno > 0 && attrno <= target_relation->rd_att->natts);
15381528
att_tup = TupleDescAttr(target_relation->rd_att, attrno - 1);
15391529

1540-
if (!force_nulls && !att_tup->attisdropped)
1530+
if (!att_tup->attisdropped)
15411531
new_expr = build_column_default(target_relation, attrno);
15421532
else
15431533
new_expr = NULL; /* force a NULL if dropped */
@@ -1587,6 +1577,50 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
15871577
return allReplaced;
15881578
}
15891579

1580+
/*
1581+
* Mop up any remaining DEFAULT items in the given VALUES RTE by
1582+
* replacing them with NULL constants.
1583+
*
1584+
* This is used for the product queries generated by DO ALSO rules attached to
1585+
* an auto-updatable view. The action can't depend on the "target relation"
1586+
* since the product query might not have one (it needn't be an INSERT).
1587+
* Essentially, such queries are treated as being attached to a rule-updatable
1588+
* view.
1589+
*/
1590+
static void
1591+
rewriteValuesRTEToNulls(Query *parsetree, RangeTblEntry *rte)
1592+
{
1593+
List *newValues;
1594+
ListCell *lc;
1595+
1596+
Assert(rte->rtekind == RTE_VALUES);
1597+
newValues = NIL;
1598+
foreach(lc, rte->values_lists)
1599+
{
1600+
List *sublist = (List *) lfirst(lc);
1601+
List *newList = NIL;
1602+
ListCell *lc2;
1603+
1604+
foreach(lc2, sublist)
1605+
{
1606+
Node *col = (Node *) lfirst(lc2);
1607+
1608+
if (IsA(col, SetToDefault))
1609+
{
1610+
SetToDefault *def = (SetToDefault *) col;
1611+
1612+
newList = lappend(newList, makeNullConst(def->typeId,
1613+
def->typeMod,
1614+
def->collation));
1615+
}
1616+
else
1617+
newList = lappend(newList, col);
1618+
}
1619+
newValues = lappend(newValues, newList);
1620+
}
1621+
rte->values_lists = newValues;
1622+
}
1623+
15901624

15911625
/*
15921626
* Record in target_rte->extraUpdatedCols the indexes of any generated columns
@@ -3746,7 +3780,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
37463780
&unused_values_attrnos);
37473781
/* ... and the VALUES expression lists */
37483782
if (!rewriteValuesRTE(parsetree, values_rte, values_rte_index,
3749-
rt_entry_relation, false,
3783+
rt_entry_relation,
37503784
unused_values_attrnos))
37513785
defaults_remaining = true;
37523786
}
@@ -3860,10 +3894,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
38603894
RangeTblEntry *values_rte = rt_fetch(values_rte_index,
38613895
pt->rtable);
38623896

3863-
rewriteValuesRTE(pt, values_rte, values_rte_index,
3864-
rt_entry_relation,
3865-
true, /* Force remaining defaults to NULL */
3866-
NULL);
3897+
rewriteValuesRTEToNulls(pt, values_rte);
38673898
}
38683899
}
38693900

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,8 +433,30 @@ EXPLAIN (costs off) DELETE FROM rw_view1 WHERE a=5;
433433
Index Cond: ((a > 0) AND (a = 5))
434434
(3 rows)
435435

436+
-- it's still updatable if we add a DO ALSO rule
437+
CREATE TABLE base_tbl_hist(ts timestamptz default now(), a int, b text);
438+
CREATE RULE base_tbl_log AS ON INSERT TO rw_view1 DO ALSO
439+
INSERT INTO base_tbl_hist(a,b) VALUES(new.a, new.b);
440+
SELECT table_name, is_updatable, is_insertable_into
441+
FROM information_schema.views
442+
WHERE table_name = 'rw_view1';
443+
table_name | is_updatable | is_insertable_into
444+
------------+--------------+--------------------
445+
rw_view1 | YES | YES
446+
(1 row)
447+
448+
-- Check behavior with DEFAULTs (bug #17633)
449+
INSERT INTO rw_view1 VALUES (9, DEFAULT), (10, DEFAULT);
450+
SELECT a, b FROM base_tbl_hist;
451+
a | b
452+
----+---
453+
9 |
454+
10 |
455+
(2 rows)
456+
436457
DROP TABLE base_tbl CASCADE;
437458
NOTICE: drop cascades to view rw_view1
459+
DROP TABLE base_tbl_hist;
438460
-- view on top of view
439461
CREATE TABLE base_tbl (a int PRIMARY KEY, b text DEFAULT 'Unspecified');
440462
INSERT INTO base_tbl SELECT i, 'Row ' || i FROM generate_series(-2, 2) g(i);

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,24 @@ SELECT * FROM base_tbl;
148148
EXPLAIN (costs off) UPDATE rw_view1 SET a=6 WHERE a=5;
149149
EXPLAIN (costs off) DELETE FROM rw_view1 WHERE a=5;
150150

151+
-- it's still updatable if we add a DO ALSO rule
152+
153+
CREATE TABLE base_tbl_hist(ts timestamptz default now(), a int, b text);
154+
155+
CREATE RULE base_tbl_log AS ON INSERT TO rw_view1 DO ALSO
156+
INSERT INTO base_tbl_hist(a,b) VALUES(new.a, new.b);
157+
158+
SELECT table_name, is_updatable, is_insertable_into
159+
FROM information_schema.views
160+
WHERE table_name = 'rw_view1';
161+
162+
-- Check behavior with DEFAULTs (bug #17633)
163+
164+
INSERT INTO rw_view1 VALUES (9, DEFAULT), (10, DEFAULT);
165+
SELECT a, b FROM base_tbl_hist;
166+
151167
DROP TABLE base_tbl CASCADE;
168+
DROP TABLE base_tbl_hist;
152169

153170
-- view on top of view
154171

0 commit comments

Comments
 (0)
0