Re: BUG #17633: Define rule on views which do insert to another relation trigger cache lookup failed error. - Mailing list pgsql-bugs
From | Tom Lane |
---|---|
Subject | Re: BUG #17633: Define rule on views which do insert to another relation trigger cache lookup failed error. |
Date | |
Msg-id | 348879.1665502632@sss.pgh.pa.us Whole thread Raw |
In response to | Re: BUG #17633: Define rule on views which do insert to another relation trigger cache lookup failed error. (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: BUG #17633: Define rule on views which do insert to another relation trigger cache lookup failed error.
|
List | pgsql-bugs |
I wrote: > I think the basic problem is that the two calls of rewriteValuesRTE > are really dealing with fundamentally different cases, and we should > probably not have tried to make the same function do both. I'm going > to try splitting it into two functions, one for the force_nulls case > and one for the !force_nulls case. As attached. This seems *way* cleaner to me, even if it means we need two copies of the loops-over-VALUES-list-entries. I didn't write a regression test yet, but this fixes the submitted bug and passes check-world. regards, tom lane diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index d02fd83c0a..a6dd99cc98 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -79,8 +79,9 @@ static TargetEntry *process_matched_tle(TargetEntry *src_tle, static Node *get_assignment_input(Node *node); static Bitmapset *findDefaultOnlyColumns(RangeTblEntry *rte); static bool rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti, - Relation target_relation, bool force_nulls, + Relation target_relation, Bitmapset *unused_cols); +static void rewriteValuesRTEToNulls(Query *parsetree, RangeTblEntry *rte); static void markQueryForLocking(Query *qry, Node *jtnode, LockClauseStrength strength, LockWaitPolicy waitPolicy, bool pushedDown); @@ -1370,18 +1371,7 @@ findDefaultOnlyColumns(RangeTblEntry *rte) * all DEFAULT items are replaced, and if the target relation doesn't have a * default, the value is explicitly set to NULL. * - * Additionally, if force_nulls is true, the target relation's defaults are - * ignored and all DEFAULT items in the VALUES list are explicitly set to - * NULL, regardless of the target relation's type. This is used for the - * product queries generated by DO ALSO rules attached to an auto-updatable - * view, for which we will have already called this function with force_nulls - * false. For these product queries, we must then force any remaining DEFAULT - * items to NULL to provide concrete values for the rule actions. - * Essentially, this is a mix of the 2 cases above --- the original query is - * an insert into an auto-updatable view, and the product queries are inserts - * into a rule-updatable view. - * - * Finally, if a DEFAULT item is found in a column mentioned in unused_cols, + * Also, if a DEFAULT item is found in a column mentioned in unused_cols, * it is explicitly set to NULL. This happens for columns in the VALUES RTE * whose corresponding targetlist entries have already been replaced with the * relation's default expressions, so that any values in those columns of the @@ -1404,7 +1394,7 @@ findDefaultOnlyColumns(RangeTblEntry *rte) */ static bool rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti, - Relation target_relation, bool force_nulls, + Relation target_relation, Bitmapset *unused_cols) { List *newValues; @@ -1414,15 +1404,14 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti, int numattrs; int *attrnos; + Assert(rte->rtekind == RTE_VALUES); + /* * Rebuilding all the lists is a pretty expensive proposition in a big * VALUES list, and it's a waste of time if there aren't any DEFAULT * placeholders. So first scan to see if there are any. - * - * We skip this check if force_nulls is true, because we know that there - * are DEFAULT items present in that case. */ - if (!force_nulls && !searchForDefault(rte)) + if (!searchForDefault(rte)) return true; /* nothing to do */ /* @@ -1456,12 +1445,10 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti, /* * Check if the target relation is an auto-updatable view, in which case * unresolved defaults will be left untouched rather than being set to - * NULL. If force_nulls is true, we always set DEFAULT items to NULL, so - * skip this check in that case --- it isn't an auto-updatable view. + * NULL. */ isAutoUpdatableView = false; - if (!force_nulls && - target_relation->rd_rel->relkind == RELKIND_VIEW && + if (target_relation->rd_rel->relkind == RELKIND_VIEW && !view_has_instead_trigger(target_relation, CMD_INSERT)) { List *locks; @@ -1535,9 +1522,10 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti, if (attrno == 0) elog(ERROR, "cannot set value in column %d to DEFAULT", i); + Assert(attrno > 0 && attrno <= target_relation->rd_att->natts); att_tup = TupleDescAttr(target_relation->rd_att, attrno - 1); - if (!force_nulls && !att_tup->attisdropped) + if (!att_tup->attisdropped) new_expr = build_column_default(target_relation, attrno); else new_expr = NULL; /* force a NULL if dropped */ @@ -1587,6 +1575,50 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti, return allReplaced; } +/* + * Mop up any remaining DEFAULT items in the given VALUES RTE by + * replacing them with NULL constants. + * + * This is used for the product queries generated by DO ALSO rules attached to + * an auto-updatable view. The action can't depend on the "target relation" + * since the product query might not have one (it needn't be an INSERT). + * Essentially, such queries are treated as being attached to a rule-updatable + * view. + */ +static void +rewriteValuesRTEToNulls(Query *parsetree, RangeTblEntry *rte) +{ + List *newValues; + ListCell *lc; + + Assert(rte->rtekind == RTE_VALUES); + newValues = NIL; + foreach(lc, rte->values_lists) + { + List *sublist = (List *) lfirst(lc); + List *newList = NIL; + ListCell *lc2; + + foreach(lc2, sublist) + { + Node *col = (Node *) lfirst(lc2); + + if (IsA(col, SetToDefault)) + { + SetToDefault *def = (SetToDefault *) col; + + newList = lappend(newList, makeNullConst(def->typeId, + def->typeMod, + def->collation)); + } + else + newList = lappend(newList, col); + } + newValues = lappend(newValues, newList); + } + rte->values_lists = newValues; +} + /* * Record in target_rte->extraUpdatedCols the indexes of any generated columns @@ -3746,7 +3778,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events) &unused_values_attrnos); /* ... and the VALUES expression lists */ if (!rewriteValuesRTE(parsetree, values_rte, values_rte_index, - rt_entry_relation, false, + rt_entry_relation, unused_values_attrnos)) defaults_remaining = true; } @@ -3860,10 +3892,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events) RangeTblEntry *values_rte = rt_fetch(values_rte_index, pt->rtable); - rewriteValuesRTE(pt, values_rte, values_rte_index, - rt_entry_relation, - true, /* Force remaining defaults to NULL */ - NULL); + rewriteValuesRTEToNulls(pt, values_rte); } }
pgsql-bugs by date: