public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v9 2/4] WIP: Check for volatile defaults 2+ messages / 2 participants [nested] [flat]
* [PATCH v9 2/4] WIP: Check for volatile defaults @ 2020-12-02 05:11 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Justin Pryzby @ 2020-12-02 05:11 UTC (permalink / raw) We want to check if any column *uses* a volatile default value, but after parsing and rewriting, that information appears to be lost about which column values are defaults and which were specified. insertedcols doesn't appear to be useful for this. So add a field to track if a TargetEntry is planned with column default. --- src/backend/executor/nodeModifyTable.c | 64 ++++++++++++++++++++++++-- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/makefuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/nodes/readfuncs.c | 1 + src/backend/optimizer/util/tlist.c | 1 + src/backend/rewrite/rewriteHandler.c | 3 ++ src/include/nodes/primnodes.h | 2 + 9 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index a53cbeeb2c..2059428e2a 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -50,6 +50,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" @@ -2258,6 +2259,61 @@ ExecModifyTable(PlanState *pstate) return NULL; } +/* + * Determine if a table has volatile column defaults which are used by a given + * planned statement (if the column is not specified or specified as DEFAULT). + * This works only for INSERT. + */ +static bool +has_volatile_defaults(ResultRelInfo *resultRelInfo, ModifyTable *node) +{ + TupleDesc tupDesc = RelationGetDescr(resultRelInfo->ri_RelationDesc); + Plan *plan; + + Assert(list_length(node->plans) == 1); + plan = linitial(node->plans); + + for (int attnum = 1; attnum <= tupDesc->natts; attnum++) + { + Form_pg_attribute att = TupleDescAttr(tupDesc, attnum - 1); + Expr *defexpr; + TargetEntry *tle; + + /* We don't need to check dropped/generated attributes */ + if (att->attisdropped || att->attgenerated) + continue; + + tle = list_nth(plan->targetlist, attnum - 1); + Assert(tle != NULL); + Assert(tle->resno == attnum); + + /* + * If the column was specified with a non-default value, then don't + * check the volatility of its default + */ + if (!tle->isdefault) + continue; + + /* Check the column's default value if one exists */ + defexpr = (Expr *) build_column_default(resultRelInfo->ri_RelationDesc, attnum); + if (defexpr == NULL) + continue; + + /* Run the expression through planner */ + // defexpr = expression_planner(defexpr); + // (void) ExecInitExpr(defexpr, NULL); + expression_planner(defexpr); + + if (contain_volatile_functions_not_nextval((Node *) defexpr)) + { + elog(DEBUG1, "found volatile att %d", attnum); + return true; + } + } + + return false; +} + /* ---------------------------------------------------------------- * ExecInitModifyTable * ---------------------------------------------------------------- @@ -2352,10 +2408,10 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) * are any statement level insert triggers. */ mtstate->miinfo = NULL; - else if (mtstate->rootResultRelInfo->ri_FdwRoutine != NULL - /* || cstate->volatile_defexprs */ ) - // XXX contain_volatile_functions_not_nextval((Node *) defexpr); - /* Can't support multi-inserts to foreign tables or if there are any */ + else if (mtstate->rootResultRelInfo->ri_FdwRoutine != NULL || + has_volatile_defaults(mtstate->rootResultRelInfo, node)) + /* Can't support multi-inserts to foreign tables or if there are any + * volatile default expressions in the table. */ mtstate->miinfo = NULL; else { diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index ba3ccc712c..2008e8e5d6 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -2178,6 +2178,7 @@ _copyTargetEntry(const TargetEntry *from) COPY_SCALAR_FIELD(resorigtbl); COPY_SCALAR_FIELD(resorigcol); COPY_SCALAR_FIELD(resjunk); + COPY_SCALAR_FIELD(isdefault); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index a2ef853dc2..aa3cdf3729 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -770,6 +770,7 @@ _equalTargetEntry(const TargetEntry *a, const TargetEntry *b) COMPARE_SCALAR_FIELD(resorigtbl); COMPARE_SCALAR_FIELD(resorigcol); COMPARE_SCALAR_FIELD(resjunk); + COMPARE_SCALAR_FIELD(isdefault); return true; } diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c index 01c110cd2f..aeeba7032f 100644 --- a/src/backend/nodes/makefuncs.c +++ b/src/backend/nodes/makefuncs.c @@ -254,6 +254,7 @@ makeTargetEntry(Expr *expr, tle->ressortgroupref = 0; tle->resorigtbl = InvalidOid; tle->resorigcol = 0; + tle->isdefault = false; tle->resjunk = resjunk; diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 8392be6d44..924ffda1c6 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -1661,6 +1661,7 @@ _outTargetEntry(StringInfo str, const TargetEntry *node) WRITE_OID_FIELD(resorigtbl); WRITE_INT_FIELD(resorigcol); WRITE_BOOL_FIELD(resjunk); + WRITE_BOOL_FIELD(isdefault); } static void diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index d2c8d58070..642b6c63c5 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1275,6 +1275,7 @@ _readTargetEntry(void) READ_OID_FIELD(resorigtbl); READ_INT_FIELD(resorigcol); READ_BOOL_FIELD(resjunk); + READ_BOOL_FIELD(isdefault); READ_DONE(); } diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c index 89853a0630..7ef1517027 100644 --- a/src/backend/optimizer/util/tlist.c +++ b/src/backend/optimizer/util/tlist.c @@ -349,6 +349,7 @@ apply_tlist_labeling(List *dest_tlist, List *src_tlist) dest_tle->resorigtbl = src_tle->resorigtbl; dest_tle->resorigcol = src_tle->resorigcol; dest_tle->resjunk = src_tle->resjunk; + dest_tle->isdefault = src_tle->isdefault; } } diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 0c7508a0d8..7ab13e51e5 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -986,10 +986,13 @@ rewriteTargetListIU(List *targetList, } if (new_expr) + { new_tle = makeTargetEntry((Expr *) new_expr, attrno, pstrdup(NameStr(att_tup->attname)), false); + new_tle->isdefault = true; + } } /* diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index d4ce037088..888bd36a07 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1437,6 +1437,8 @@ typedef struct TargetEntry AttrNumber resorigcol; /* column's number in source table */ bool resjunk; /* set to true to eliminate the attribute from * final target list */ + bool isdefault; /* true if using the column default, either + * by "DEFAULT" or omission of the column */ } TargetEntry; -- 2.17.0 --3yNHWXBV/QO9xKNm Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0003-COPY-flush-multi-insert-buffer-based-on-accumulat.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* Invalid index on partitioned table - is this a bug or feature? @ 2025-01-22 17:51 Dmitry Koval <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Dmitry Koval @ 2025-01-22 17:51 UTC (permalink / raw) To: pgsql-hackers Hi! If you create an index on a partitioned table using a method that uses pg_dump: (a) "ALTER TABLE ONLY ... ADD CONSTRAINT .. PRIMARY KEY ..." or (b) "CREATE INDEX ... ON ONLY ..." then the index for the partitioned table is created with the INVALID flag.This can be verified using an example (on "master" branch): -------------------------------------- -- The standard way to create indexes. -- CREATE TABLE t_int (i int PRIMARY KEY, v int, x int) PARTITION BY RANGE (i); CREATE TABLE t_int_1 PARTITION OF t_int FOR VALUES FROM (1) TO (100); CREATE INDEX t_int_v ON t_int (v); -- VALID indexes --Indexes: -- "t_int_pkey" PRIMARY KEY, btree (i) -- "t_int_v" btree (v) \d+ t_int DROP TABLE t_int; --------------------------------- -- pg_dump way to create indexes. -- CREATE TABLE t_int (i int NOT NULL, v int, x int) PARTITION BY RANGE (i); CREATE TABLE t_int_1 PARTITION OF t_int FOR VALUES FROM (1) TO (100); ALTER TABLE ONLY public.t_int ADD CONSTRAINT t_int_pkey PRIMARY KEY (i); ALTER TABLE ONLY public.t_int_1 ADD CONSTRAINT t_int_1_pkey PRIMARY KEY (i); CREATE INDEX t_int_v ON ONLY public.t_int USING btree (v); CREATE INDEX t_int_1_v_idx ON public.t_int_1 USING btree (v); -- INVALID indexes --Indexes: -- "t_int_pkey" PRIMARY KEY, btree (i) INVALID -- "t_int_v" btree (v) INVALID \d+ t_int DROP TABLE t_int; --------------------------------- I was unable to change the index flag INVALID -> VALID using the REINDEX command. I understand that setting the INVALID flag might be correct (see comment above [1]). But the INVALID flag without the possibility of changing to VALID looks strange anyway. This is also the reason why some functions may fail, for example RelationGetPrimaryKeyIndex() returns InvalidOid for INVALID PK. Is this a bug or a feature? Should this be corrected? Links. [1] https://github.com/postgres/postgres/blob/master/src/backend/commands/indexcmds.c#L1211 -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2025-01-22 17:51 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]> 2025-01-22 17:51 Invalid index on partitioned table - is this a bug or feature? Dmitry Koval <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox