]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix calculation of which GENERATED columns need to be updated.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 5 Jan 2023 19:12:17 +0000 (14:12 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 5 Jan 2023 19:12:17 +0000 (14:12 -0500)
We were identifying the updatable generated columns of inheritance
children by transposing the calculation made for their parent.
However, there's nothing that says a traditional-inheritance child
can't have generated columns that aren't there in its parent, or that
have different dependencies than are in the parent's expression.
(At present it seems that we don't enforce that for partitioning
either, which is likely wrong to some degree or other; but the case
clearly needs to be handled with traditional inheritance.)

Hence, drop the very-klugy-anyway "extraUpdatedCols" RTE field
in favor of identifying which generated columns depend on updated
columns during executor startup.  In HEAD we can remove
extraUpdatedCols altogether; in back branches, it's still there but
always empty.  Another difference between the HEAD and back-branch
versions of this patch is that in HEAD we can add the new bitmap field
to ResultRelInfo, but that would cause an ABI break in back branches.
Like 4b3e37993, add a List field at the end of struct EState instead.

Back-patch to v13.  The bogus calculation is also being made in v12,
but it doesn't have the same visible effect because we don't use it
to decide which generated columns to recalculate; as a consequence of
which the patch doesn't apply easily.  I think that there might still
be a demonstrable bug associated with trigger firing conditions, but
that's such a weird corner-case usage that I'm content to leave it
unfixed in v12.

Amit Langote and Tom Lane

Discussion: https://postgr.es/m/CA+HiwqFshLKNvQUd1DgwJ-7tsTp=dwv7KZqXC4j2wYBV1aCDUA@mail.gmail.com
Discussion: https://postgr.es/m/2793383.1672944799@sss.pgh.pa.us

14 files changed:
contrib/postgres_fdw/postgres_fdw.c
src/backend/executor/execUtils.c
src/backend/executor/nodeModifyTable.c
src/backend/optimizer/util/inherit.c
src/backend/optimizer/util/plancat.c
src/backend/replication/logical/worker.c
src/backend/rewrite/rewriteHandler.c
src/include/nodes/execnodes.h
src/include/nodes/parsenodes.h
src/include/optimizer/inherit.h
src/include/optimizer/plancat.h
src/include/rewrite/rewriteHandler.h
src/test/regress/expected/generated.out
src/test/regress/sql/generated.sql

index 46f5e1c40077c78f51bca46f7e98c1355b6a5343..b08b314f9fdc7332a463233216775f7d0ee1c38b 100644 (file)
@@ -31,6 +31,7 @@
 #include "optimizer/appendinfo.h"
 #include "optimizer/clauses.h"
 #include "optimizer/cost.h"
+#include "optimizer/inherit.h"
 #include "optimizer/optimizer.h"
 #include "optimizer/pathnode.h"
 #include "optimizer/paths.h"
@@ -1813,7 +1814,8 @@ postgresPlanForeignModify(PlannerInfo *root,
        else if (operation == CMD_UPDATE)
        {
                int                     col;
-               Bitmapset  *allUpdatedCols = bms_union(rte->updatedCols, rte->extraUpdatedCols);
+               RelOptInfo *rel = find_base_rel(root, resultRelation);
+               Bitmapset  *allUpdatedCols = get_rel_all_updated_cols(root, rel);
 
                col = -1;
                while ((col = bms_next_member(allUpdatedCols, col)) >= 0)
index 358e5828831834c599c56aad0b1ca02c06bb6383..122de068022edb718802a9720afa13ee1f7f4571 100644 (file)
@@ -132,6 +132,8 @@ CreateExecutorState(void)
        estate->es_insert_pending_result_relations = NIL;
        estate->es_insert_pending_modifytables = NIL;
 
+       estate->es_resultrelinfo_extra = NIL;
+
        estate->es_param_list_info = NULL;
        estate->es_param_exec_vals = NULL;
 
@@ -1323,26 +1325,25 @@ ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate)
 Bitmapset *
 ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate)
 {
-       /* see ExecGetInsertedCols() */
-       if (relinfo->ri_RangeTableIndex != 0)
-       {
-               RangeTblEntry *rte = exec_rt_fetch(relinfo->ri_RangeTableIndex, estate);
+       Relation        rel = relinfo->ri_RelationDesc;
+       TupleDesc       tupdesc = RelationGetDescr(rel);
 
-               return rte->extraUpdatedCols;
-       }
-       else if (relinfo->ri_RootResultRelInfo)
+       if (tupdesc->constr && tupdesc->constr->has_generated_stored)
        {
-               ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo;
-               RangeTblEntry *rte = exec_rt_fetch(rootRelInfo->ri_RangeTableIndex, estate);
+               ListCell   *lc;
 
-               if (relinfo->ri_RootToPartitionMap != NULL)
-                       return execute_attr_map_cols(relinfo->ri_RootToPartitionMap->attrMap,
-                                                                                rte->extraUpdatedCols);
-               else
-                       return rte->extraUpdatedCols;
+               /* Assert that ExecInitStoredGenerated has been called. */
+               Assert(relinfo->ri_GeneratedExprs != NULL);
+               foreach(lc, estate->es_resultrelinfo_extra)
+               {
+                       ResultRelInfoExtra *rextra = (ResultRelInfoExtra *) lfirst(lc);
+
+                       if (rextra->rinfo == relinfo)
+                               return rextra->ri_extraUpdatedCols;
+               }
+               Assert(false);                  /* shouldn't get here */
        }
-       else
-               return NULL;
+       return NULL;
 }
 
 /* Return columns being updated, including generated columns */
index 8794528d6af6fcfc5ad3b0bded3db033a6f79085..4c49edef25038f3a45109d81e7f6fc7ce07b13b2 100644 (file)
@@ -54,6 +54,7 @@
 #include "foreign/fdwapi.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
+#include "optimizer/optimizer.h"
 #include "rewrite/rewriteHandler.h"
 #include "storage/bufmgr.h"
 #include "storage/lmgr.h"
@@ -353,69 +354,128 @@ ExecCheckTIDVisible(EState *estate,
 }
 
 /*
- * Compute stored generated columns for a tuple
+ * Initialize to compute stored generated columns for a tuple
+ *
+ * This fills the resultRelInfo's ri_GeneratedExprs field and makes an
+ * associated ResultRelInfoExtra struct to hold ri_extraUpdatedCols.
+ * (Currently, ri_extraUpdatedCols is consulted only in UPDATE, but we might
+ * as well fill it for INSERT too.)
  */
-void
-ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
-                                                  EState *estate, TupleTableSlot *slot,
-                                                  CmdType cmdtype)
+static void
+ExecInitStoredGenerated(ResultRelInfo *resultRelInfo,
+                                               EState *estate,
+                                               CmdType cmdtype)
 {
        Relation        rel = resultRelInfo->ri_RelationDesc;
        TupleDesc       tupdesc = RelationGetDescr(rel);
        int                     natts = tupdesc->natts;
+       Bitmapset  *updatedCols;
+       ResultRelInfoExtra *rextra;
        MemoryContext oldContext;
-       Datum      *values;
-       bool       *nulls;
 
-       Assert(tupdesc->constr && tupdesc->constr->has_generated_stored);
+       /* Don't call twice */
+       Assert(resultRelInfo->ri_GeneratedExprs == NULL);
+
+       /* Nothing to do if no generated columns */
+       if (!(tupdesc->constr && tupdesc->constr->has_generated_stored))
+               return;
 
        /*
-        * If first time through for this result relation, build expression
-        * nodetrees for rel's stored generation expressions.  Keep them in the
-        * per-query memory context so they'll survive throughout the query.
+        * In an UPDATE, we can skip computing any generated columns that do not
+        * depend on any UPDATE target column.  But if there is a BEFORE ROW
+        * UPDATE trigger, we cannot skip because the trigger might change more
+        * columns.
         */
-       if (resultRelInfo->ri_GeneratedExprs == NULL)
-       {
-               oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
+       if (cmdtype == CMD_UPDATE &&
+               !(rel->trigdesc && rel->trigdesc->trig_update_before_row))
+               updatedCols = ExecGetUpdatedCols(resultRelInfo, estate);
+       else
+               updatedCols = NULL;
 
-               resultRelInfo->ri_GeneratedExprs =
-                       (ExprState **) palloc(natts * sizeof(ExprState *));
-               resultRelInfo->ri_NumGeneratedNeeded = 0;
+       /*
+        * Make sure these data structures are built in the per-query memory
+        * context so they'll survive throughout the query.
+        */
+       oldContext = MemoryContextSwitchTo(estate->es_query_cxt);
+
+       resultRelInfo->ri_GeneratedExprs =
+               (ExprState **) palloc0(natts * sizeof(ExprState *));
+       resultRelInfo->ri_NumGeneratedNeeded = 0;
+
+       rextra = palloc_object(ResultRelInfoExtra);
+       rextra->rinfo = resultRelInfo;
+       rextra->ri_extraUpdatedCols = NULL;
+       estate->es_resultrelinfo_extra = lappend(estate->es_resultrelinfo_extra,
+                                                                                        rextra);
 
-               for (int i = 0; i < natts; i++)
+       for (int i = 0; i < natts; i++)
+       {
+               if (TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_STORED)
                {
-                       if (TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_STORED)
-                       {
-                               Expr       *expr;
+                       Expr       *expr;
 
-                               /*
-                                * If it's an update and the current column was not marked as
-                                * being updated, then we can skip the computation.  But if
-                                * there is a BEFORE ROW UPDATE trigger, we cannot skip
-                                * because the trigger might affect additional columns.
-                                */
-                               if (cmdtype == CMD_UPDATE &&
-                                       !(rel->trigdesc && rel->trigdesc->trig_update_before_row) &&
-                                       !bms_is_member(i + 1 - FirstLowInvalidHeapAttributeNumber,
-                                                                  ExecGetExtraUpdatedCols(resultRelInfo, estate)))
-                               {
-                                       resultRelInfo->ri_GeneratedExprs[i] = NULL;
-                                       continue;
-                               }
+                       /* Fetch the GENERATED AS expression tree */
+                       expr = (Expr *) build_column_default(rel, i + 1);
+                       if (expr == NULL)
+                               elog(ERROR, "no generation expression found for column number %d of table \"%s\"",
+                                        i + 1, RelationGetRelationName(rel));
+
+                       /*
+                        * If it's an update with a known set of update target columns,
+                        * see if we can skip the computation.
+                        */
+                       if (updatedCols)
+                       {
+                               Bitmapset  *attrs_used = NULL;
 
-                               expr = (Expr *) build_column_default(rel, i + 1);
-                               if (expr == NULL)
-                                       elog(ERROR, "no generation expression found for column number %d of table \"%s\"",
-                                                i + 1, RelationGetRelationName(rel));
+                               pull_varattnos((Node *) expr, 1, &attrs_used);
 
-                               resultRelInfo->ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
-                               resultRelInfo->ri_NumGeneratedNeeded++;
+                               if (!bms_overlap(updatedCols, attrs_used))
+                                       continue;       /* need not update this column */
                        }
-               }
 
-               MemoryContextSwitchTo(oldContext);
+                       /* No luck, so prepare the expression for execution */
+                       resultRelInfo->ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
+                       resultRelInfo->ri_NumGeneratedNeeded++;
+
+                       /* And mark this column in rextra->ri_extraUpdatedCols */
+                       rextra->ri_extraUpdatedCols =
+                               bms_add_member(rextra->ri_extraUpdatedCols,
+                                                          i + 1 - FirstLowInvalidHeapAttributeNumber);
+               }
        }
 
+       MemoryContextSwitchTo(oldContext);
+}
+
+/*
+ * Compute stored generated columns for a tuple
+ */
+void
+ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
+                                                  EState *estate, TupleTableSlot *slot,
+                                                  CmdType cmdtype)
+{
+       Relation        rel = resultRelInfo->ri_RelationDesc;
+       TupleDesc       tupdesc = RelationGetDescr(rel);
+       int                     natts = tupdesc->natts;
+       ExprContext *econtext = GetPerTupleExprContext(estate);
+       MemoryContext oldContext;
+       Datum      *values;
+       bool       *nulls;
+
+       /* We should not be called unless this is true */
+       Assert(tupdesc->constr && tupdesc->constr->has_generated_stored);
+
+       /*
+        * For relations named directly in the query, ExecInitStoredGenerated
+        * should have been called already; but this might not have happened yet
+        * for a partition child rel.  Also, it's convenient for outside callers
+        * to not have to call ExecInitStoredGenerated explicitly.
+        */
+       if (resultRelInfo->ri_GeneratedExprs == NULL)
+               ExecInitStoredGenerated(resultRelInfo, estate, cmdtype);
+
        /*
         * If no generated columns have been affected by this change, then skip
         * the rest.
@@ -435,14 +495,13 @@ ExecComputeStoredGenerated(ResultRelInfo *resultRelInfo,
        {
                Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
 
-               if (attr->attgenerated == ATTRIBUTE_GENERATED_STORED &&
-                       resultRelInfo->ri_GeneratedExprs[i])
+               if (resultRelInfo->ri_GeneratedExprs[i])
                {
-                       ExprContext *econtext;
                        Datum           val;
                        bool            isnull;
 
-                       econtext = GetPerTupleExprContext(estate);
+                       Assert(attr->attgenerated == ATTRIBUTE_GENERATED_STORED);
+
                        econtext->ecxt_scantuple = slot;
 
                        val = ExecEvalExpr(resultRelInfo->ri_GeneratedExprs[i], econtext, &isnull);
@@ -4088,6 +4147,15 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
                                        elog(ERROR, "could not find junk wholerow column");
                        }
                }
+
+               /*
+                * For INSERT and UPDATE, prepare to evaluate any generated columns.
+                * We must do this now, even if we never insert or update any rows,
+                * because we have to fill resultRelInfo->ri_extraUpdatedCols for
+                * possible use by the trigger machinery.
+                */
+               if (operation == CMD_INSERT || operation == CMD_UPDATE)
+                       ExecInitStoredGenerated(resultRelInfo, estate, operation);
        }
 
        /*
index 6dbffe121a59488e245cd27ba107a0c09b2be4ba..3c11f5db5adf8953561a4db3241514894ae5ce9c 100644 (file)
@@ -25,6 +25,7 @@
 #include "optimizer/inherit.h"
 #include "optimizer/optimizer.h"
 #include "optimizer/pathnode.h"
+#include "optimizer/plancat.h"
 #include "optimizer/planmain.h"
 #include "optimizer/planner.h"
 #include "optimizer/prep.h"
@@ -47,6 +48,10 @@ static void expand_single_inheritance_child(PlannerInfo *root,
                                                                                        Index *childRTindex_p);
 static Bitmapset *translate_col_privs(const Bitmapset *parent_privs,
                                                                          List *translated_vars);
+static Bitmapset *translate_col_privs_multilevel(PlannerInfo *root,
+                                                                                                RelOptInfo *rel,
+                                                                                                RelOptInfo *parent_rel,
+                                                                                                Bitmapset *parent_cols);
 static void expand_appendrel_subquery(PlannerInfo *root, RelOptInfo *rel,
                                                                          RangeTblEntry *rte, Index rti);
 
@@ -333,11 +338,6 @@ expand_partitioned_rtentry(PlannerInfo *root, RelOptInfo *relinfo,
                root->partColsUpdated =
                        has_partition_attrs(parentrel, parentrte->updatedCols, NULL);
 
-       /*
-        * There shouldn't be any generated columns in the partition key.
-        */
-       Assert(!has_partition_attrs(parentrel, parentrte->extraUpdatedCols, NULL));
-
        /* Nothing further to do here if there are no partitions. */
        if (partdesc->nparts == 0)
                return;
@@ -556,15 +556,12 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
                                                                                                         appinfo->translated_vars);
                childrte->updatedCols = translate_col_privs(parentrte->updatedCols,
                                                                                                        appinfo->translated_vars);
-               childrte->extraUpdatedCols = translate_col_privs(parentrte->extraUpdatedCols,
-                                                                                                                appinfo->translated_vars);
        }
        else
        {
                childrte->selectedCols = bms_copy(parentrte->selectedCols);
                childrte->insertedCols = bms_copy(parentrte->insertedCols);
                childrte->updatedCols = bms_copy(parentrte->updatedCols);
-               childrte->extraUpdatedCols = bms_copy(parentrte->extraUpdatedCols);
        }
 
        /*
@@ -648,6 +645,52 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
        }
 }
 
+/*
+ * get_rel_all_updated_cols
+ *             Returns the set of columns of a given "simple" relation that are
+ *             updated by this query.
+ */
+Bitmapset *
+get_rel_all_updated_cols(PlannerInfo *root, RelOptInfo *rel)
+{
+       Index           relid;
+       RangeTblEntry *rte;
+       Bitmapset  *updatedCols,
+                          *extraUpdatedCols;
+
+       Assert(root->parse->commandType == CMD_UPDATE);
+       Assert(IS_SIMPLE_REL(rel));
+
+       /*
+        * We obtain updatedCols for the query's result relation.  Then, if
+        * necessary, we map it to the column numbers of the relation for which
+        * they were requested.
+        */
+       relid = root->parse->resultRelation;
+       rte = planner_rt_fetch(relid, root);
+
+       updatedCols = rte->updatedCols;
+
+       if (rel->relid != relid)
+       {
+               RelOptInfo *top_parent_rel = find_base_rel(root, relid);
+
+               Assert(IS_OTHER_REL(rel));
+
+               updatedCols = translate_col_privs_multilevel(root, rel, top_parent_rel,
+                                                                                                        updatedCols);
+       }
+
+       /*
+        * Now we must check to see if there are any generated columns that depend
+        * on the updatedCols, and add them to the result.
+        */
+       extraUpdatedCols = get_dependent_generated_columns(root, rel->relid,
+                                                                                                          updatedCols);
+
+       return bms_union(updatedCols, extraUpdatedCols);
+}
+
 /*
  * translate_col_privs
  *       Translate a bitmapset representing per-column privileges from the
@@ -700,6 +743,44 @@ translate_col_privs(const Bitmapset *parent_privs,
        return child_privs;
 }
 
+/*
+ * translate_col_privs_multilevel
+ *             Recursively translates the column numbers contained in 'parent_cols'
+ *             to the column numbers of a descendant relation given by 'rel'
+ *
+ * Note that because this is based on translate_col_privs, it will expand
+ * a whole-row reference into all inherited columns.  This is not an issue
+ * for current usages, but beware.
+ */
+static Bitmapset *
+translate_col_privs_multilevel(PlannerInfo *root, RelOptInfo *rel,
+                                                          RelOptInfo *parent_rel,
+                                                          Bitmapset *parent_cols)
+{
+       AppendRelInfo *appinfo;
+
+       /* Fast path for easy case. */
+       if (parent_cols == NULL)
+               return NULL;
+
+       Assert(root->append_rel_array != NULL);
+       appinfo = root->append_rel_array[rel->relid];
+       Assert(appinfo != NULL);
+
+       /* Recurse if immediate parent is not the top parent. */
+       if (appinfo->parent_relid != parent_rel->relid)
+       {
+               RelOptInfo *next_parent = find_base_rel(root, appinfo->parent_relid);
+
+               parent_cols = translate_col_privs_multilevel(root, next_parent,
+                                                                                                        parent_rel,
+                                                                                                        parent_cols);
+       }
+
+       /* Now translate for this child. */
+       return translate_col_privs(parent_cols, appinfo->translated_vars);
+}
+
 /*
  * expand_appendrel_subquery
  *             Add "other rel" RelOptInfos for the children of an appendrel baserel
index c328a2f91294a317d5871ed2e85766fa1981c33b..419f2ac55fa11308b7a0af6ad51d2e52528ad68c 100644 (file)
@@ -2198,6 +2198,11 @@ has_row_triggers(PlannerInfo *root, Index rti, CmdType event)
        return result;
 }
 
+/*
+ * has_stored_generated_columns
+ *
+ * Does table identified by RTI have any STORED GENERATED columns?
+ */
 bool
 has_stored_generated_columns(PlannerInfo *root, Index rti)
 {
@@ -2217,6 +2222,57 @@ has_stored_generated_columns(PlannerInfo *root, Index rti)
        return result;
 }
 
+/*
+ * get_dependent_generated_columns
+ *
+ * Get the column numbers of any STORED GENERATED columns of the relation
+ * that depend on any column listed in target_cols.  Both the input and
+ * result bitmapsets contain column numbers offset by
+ * FirstLowInvalidHeapAttributeNumber.
+ */
+Bitmapset *
+get_dependent_generated_columns(PlannerInfo *root, Index rti,
+                                                               Bitmapset *target_cols)
+{
+       Bitmapset  *dependentCols = NULL;
+       RangeTblEntry *rte = planner_rt_fetch(rti, root);
+       Relation        relation;
+       TupleDesc       tupdesc;
+       TupleConstr *constr;
+
+       /* Assume we already have adequate lock */
+       relation = table_open(rte->relid, NoLock);
+
+       tupdesc = RelationGetDescr(relation);
+       constr = tupdesc->constr;
+
+       if (constr && constr->has_generated_stored)
+       {
+               for (int i = 0; i < constr->num_defval; i++)
+               {
+                       AttrDefault *defval = &constr->defval[i];
+                       Node       *expr;
+                       Bitmapset  *attrs_used = NULL;
+
+                       /* skip if not generated column */
+                       if (!TupleDescAttr(tupdesc, defval->adnum - 1)->attgenerated)
+                               continue;
+
+                       /* identify columns this generated column depends on */
+                       expr = stringToNode(defval->adbin);
+                       pull_varattnos(expr, 1, &attrs_used);
+
+                       if (bms_overlap(target_cols, attrs_used))
+                               dependentCols = bms_add_member(dependentCols,
+                                                                                          defval->adnum - FirstLowInvalidHeapAttributeNumber);
+               }
+       }
+
+       table_close(relation, NoLock);
+
+       return dependentCols;
+}
+
 /*
  * set_relation_partition_info
  *
index 1825daf903e7a3e3ed0a15c7e3d4427eca98f0e1..e2e3da0591083436beee2008b395110d335d8220 100644 (file)
@@ -1851,9 +1851,6 @@ apply_handle_update(StringInfo s)
                }
        }
 
-       /* Also populate extraUpdatedCols, in case we have generated columns */
-       fill_extraUpdatedCols(target_rte, rel->localrel);
-
        /* Build the search tuple. */
        oldctx = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
        slot_store_data(remoteslot, rel,
index 45bcc84cf02b8467829765f55db247ca41b5446a..dcfcdd0dd438daf561d86d9ec93a9bb6e5101d0f 100644 (file)
@@ -1626,43 +1626,6 @@ rewriteValuesRTEToNulls(Query *parsetree, RangeTblEntry *rte)
 }
 
 
-/*
- * Record in target_rte->extraUpdatedCols the indexes of any generated columns
- * that depend on any columns mentioned in target_rte->updatedCols.
- */
-void
-fill_extraUpdatedCols(RangeTblEntry *target_rte, Relation target_relation)
-{
-       TupleDesc       tupdesc = RelationGetDescr(target_relation);
-       TupleConstr *constr = tupdesc->constr;
-
-       target_rte->extraUpdatedCols = NULL;
-
-       if (constr && constr->has_generated_stored)
-       {
-               for (int i = 0; i < constr->num_defval; i++)
-               {
-                       AttrDefault *defval = &constr->defval[i];
-                       Node       *expr;
-                       Bitmapset  *attrs_used = NULL;
-
-                       /* skip if not generated column */
-                       if (!TupleDescAttr(tupdesc, defval->adnum - 1)->attgenerated)
-                               continue;
-
-                       /* identify columns this generated column depends on */
-                       expr = stringToNode(defval->adbin);
-                       pull_varattnos(expr, 1, &attrs_used);
-
-                       if (bms_overlap(target_rte->updatedCols, attrs_used))
-                               target_rte->extraUpdatedCols =
-                                       bms_add_member(target_rte->extraUpdatedCols,
-                                                                  defval->adnum - FirstLowInvalidHeapAttributeNumber);
-               }
-       }
-}
-
-
 /*
  * matchLocks -
  *       match the list of locks and returns the matching rules
@@ -3831,9 +3794,6 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
                                                                        parsetree->override,
                                                                        rt_entry_relation,
                                                                        NULL, 0, NULL);
-
-                       /* Also populate extraUpdatedCols (for generated columns) */
-                       fill_extraUpdatedCols(rt_entry, rt_entry_relation);
                }
                else if (event == CMD_MERGE)
                {
index 580e99242be956870368f3ec07c0013a65c1f404..f34d06eff4f8499cd89e5cacec40fdec69027d9f 100644 (file)
@@ -558,6 +558,19 @@ typedef struct ResultRelInfo
        List       *ri_ancestorResultRels;
 } ResultRelInfo;
 
+/*
+ * To avoid an ABI-breaking change in the size of ResultRelInfo in back
+ * branches, we create one of these for each result relation for which we've
+ * computed extraUpdatedCols, and store it in EState.es_resultrelinfo_extra.
+ */
+typedef struct ResultRelInfoExtra
+{
+       ResultRelInfo *rinfo;           /* owning ResultRelInfo */
+
+       /* For INSERT/UPDATE, attnums of generated columns to be computed */
+       Bitmapset  *ri_extraUpdatedCols;
+} ResultRelInfoExtra;
+
 /* ----------------
  *       AsyncRequest
  *
@@ -684,6 +697,9 @@ typedef struct EState
         */
        List       *es_insert_pending_result_relations;
        List       *es_insert_pending_modifytables;
+
+       /* List of ResultRelInfoExtra structs (see above) */
+       List       *es_resultrelinfo_extra;
 } EState;
 
 
index 1e7280340b4b6cbfa3eef08c14601caf4e6ac566..6944362c7ac70870f5739d7e9274d2855b3bdd43 100644 (file)
@@ -977,14 +977,7 @@ typedef struct PartitionCmd
  *       which triggers to fire and in FDWs to know which changed columns they
  *       need to ship off.
  *
- *       Generated columns that are caused to be updated by an update to a base
- *       column are listed in extraUpdatedCols.  This is not considered for
- *       permission checking, but it is useful in those places that want to know
- *       the full set of columns being updated as opposed to only the ones the
- *       user explicitly mentioned in the query.  (There is currently no need for
- *       an extraInsertedCols, but it could exist.)  Note that extraUpdatedCols
- *       is populated during query rewrite, NOT in the parser, since generated
- *       columns could be added after a rule has been parsed and stored.
+ *       extraUpdatedCols is no longer used or maintained; it's always empty.
  *
  *       securityQuals is a list of security barrier quals (boolean expressions),
  *       to be tested in the listed order before returning a row from the
index adcb1d737220f1e3bc541c85185891fc704ede63..8ebd42b757a31de9fd0a549eb1adaaad78fd9373 100644 (file)
@@ -20,6 +20,8 @@
 extern void expand_inherited_rtentry(PlannerInfo *root, RelOptInfo *rel,
                                                                         RangeTblEntry *rte, Index rti);
 
+extern Bitmapset *get_rel_all_updated_cols(PlannerInfo *root, RelOptInfo *rel);
+
 extern bool apply_child_basequals(PlannerInfo *root, RelOptInfo *parentrel,
                                                                  RelOptInfo *childrel, RangeTblEntry *childRTE,
                                                                  AppendRelInfo *appinfo);
index ad29b5d107d0eb8e4a639e8c0e1d1583c1670880..b0c06ca14e1b702d72301911fad9480d654211ff 100644 (file)
@@ -74,4 +74,7 @@ extern bool has_row_triggers(PlannerInfo *root, Index rti, CmdType event);
 
 extern bool has_stored_generated_columns(PlannerInfo *root, Index rti);
 
+extern Bitmapset *get_dependent_generated_columns(PlannerInfo *root, Index rti,
+                                                                                                 Bitmapset *target_cols);
+
 #endif                                                 /* PLANCAT_H */
index 90ecf109afcffb8884d336c4e629bcf811afcf58..b4f96f298b6b7c4eeb54baf2ccdd799b74f2754b 100644 (file)
@@ -24,9 +24,6 @@ extern void AcquireRewriteLocks(Query *parsetree,
 
 extern Node *build_column_default(Relation rel, int attrno);
 
-extern void fill_extraUpdatedCols(RangeTblEntry *target_rte,
-                                                                 Relation target_relation);
-
 extern Query *get_view_query(Relation view);
 extern const char *view_query_is_auto_updatable(Query *viewquery,
                                                                                                bool check_cols);
index bb4190340e76b52a05c586b003461def60557736..1db5f9ed47b1c0da31b8b1a53e347ecc8b4f53c0 100644 (file)
@@ -337,6 +337,25 @@ CREATE TABLE gtest1_2 () INHERITS (gtest0, gtesty);  -- error
 NOTICE:  merging multiple inherited definitions of column "b"
 ERROR:  inherited column "b" has a generation conflict
 DROP TABLE gtesty;
+-- test correct handling of GENERATED column that's only in child
+CREATE TABLE gtestp (f1 int);
+CREATE TABLE gtestc (f2 int GENERATED ALWAYS AS (f1+1) STORED) INHERITS(gtestp);
+INSERT INTO gtestc values(42);
+TABLE gtestc;
+ f1 | f2 
+----+----
+ 42 | 43
+(1 row)
+
+UPDATE gtestp SET f1 = f1 * 10;
+TABLE gtestc;
+ f1  | f2  
+-----+-----
+ 420 | 421
+(1 row)
+
+DROP TABLE gtestp CASCADE;
+NOTICE:  drop cascades to table gtestc
 -- test stored update
 CREATE TABLE gtest3 (a int, b int GENERATED ALWAYS AS (a * 3) STORED);
 INSERT INTO gtest3 (a) VALUES (1), (2), (3), (NULL);
index 378297e6ea675851f79b263ecc51039544d1e318..39eec40bce94b23f2c2344393e5b0c2eecae5334 100644 (file)
@@ -149,6 +149,15 @@ CREATE TABLE gtesty (x int, b int DEFAULT 55);
 CREATE TABLE gtest1_2 () INHERITS (gtest0, gtesty);  -- error
 DROP TABLE gtesty;
 
+-- test correct handling of GENERATED column that's only in child
+CREATE TABLE gtestp (f1 int);
+CREATE TABLE gtestc (f2 int GENERATED ALWAYS AS (f1+1) STORED) INHERITS(gtestp);
+INSERT INTO gtestc values(42);
+TABLE gtestc;
+UPDATE gtestp SET f1 = f1 * 10;
+TABLE gtestc;
+DROP TABLE gtestp CASCADE;
+
 -- test stored update
 CREATE TABLE gtest3 (a int, b int GENERATED ALWAYS AS (a * 3) STORED);
 INSERT INTO gtest3 (a) VALUES (1), (2), (3), (NULL);