public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v9 2/4] WIP: Check for volatile defaults
27+ messages / 3 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; 27+ 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] 27+ messages in thread
* [PATCH v8 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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 3428d9f48a..47a5271b91 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 910906f639..3bfa59f1a5 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2177,6 +2177,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 687609f59e..84530468b8 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -769,6 +769,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 ee033ae779..1cd14fed3d 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 9c73c605a4..ef6d14e072 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1660,6 +1660,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 169d5581b9..ba2288f7b0 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1274,6 +1274,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 02a3c6b165..1043ee7b66 100644
--- a/src/backend/optimizer/util/tlist.c
+++ b/src/backend/optimizer/util/tlist.c
@@ -354,6 +354,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 c25012f325..9998db2268 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 cdbe781c73..c5e626f175 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1428,6 +1428,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
--AZuoSAvZwvV/ife4
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0003-COPY-flush-multi-insert-buffer-based-on-accumulat.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v10 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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 0907b3ebd5..e018946c0e 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"
@@ -2401,6 +2402,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
* ----------------------------------------------------------------
@@ -2495,10 +2551,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 65bbc18ecb..0088539df7 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 c2d73626fc..12260f803f 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 f5dcedf6e8..50f5bfda8e 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 4388aae71d..973b558eec 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1315,6 +1315,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 0672f497c6..3d93e24004 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -987,10 +987,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
--8nsIa27JVQLqB7/C
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0003-COPY-flush-multi-insert-buffer-based-on-accumula.patch"
^ permalink raw reply [nested|flat] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* [PATCH v9 2/4] WIP: Check for volatile defaults
@ 2020-12-02 05:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 27+ 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] 27+ messages in thread
* Re: pgbnech: allow to cancel queries during benchmark
@ 2024-01-15 07:49 Tatsuo Ishii <[email protected]>
2024-01-19 07:51 ` Re: pgbnech: allow to cancel queries during benchmark Yugo NAGATA <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Tatsuo Ishii @ 2024-01-15 07:49 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]
> On Wed, 6 Sep 2023 20:13:34 +0900
> Yugo NAGATA <[email protected]> wrote:
>
>> I attached the updated patch v3. The changes since the previous
>> patch includes the following;
>>
>> I removed the unnecessary condition (&& false) that you
>> pointed out in [1].
>>
>> The test was rewritten by using IPC::Run signal() and integrated
>> to "001_pgbench_with_server.pl". This test is skipped on Windows
>> because SIGINT causes to terminate the test itself as discussed
>> in [2] about query cancellation test in psql.
>>
>> I added some comments to describe how query cancellation is
>> handled as I explained in [1].
>>
>> Also, I found the previous patch didn't work on Windows so fixed it.
>> On non-Windows system, a thread waiting a response of long query can
>> be interrupted by SIGINT, but on Windows, threads do not return from
>> waiting until queries they are running are cancelled. This is because,
>> when the signal is received, the system just creates a new thread to
>> execute the callback function specified by setup_cancel_handler, and
>> other thread continue to run[3]. Therefore, the queries have to be
>> cancelled in the callback function.
>>
>> [1] https://www.postgresql.org/message-id/a58388ac-5411-4760-ea46-71324d8324cb%40mines-paristech.fr
>> [2] https://www.postgresql.org/message-id/20230906004524.2fd6ee049f8a6c6f2690b99c%40sraoss.co.jp
>> [3] https://learn.microsoft.com/en-us/windows/console/handlerroutine
>
> I found that --disable-thread-safety option was removed in 68a4b58eca0329.
> So, I removed codes involving ENABLE_THREAD_SAFETY from the patch.
>
> Also, I wrote a commit log draft.
> Previously, Ctrl+C during benchmark killed pgbench immediately,
> but queries running at that time were not cancelled.
Better to mention explicitely that queries keep on running on the
backend. What about this?
Previously, Ctrl+C during benchmark killed pgbench immediately, but
queries were not canceled and they keep on running on the backend
until they tried to send the result to pgbench.
> The commit
> fixes this so that cancel requests are sent for all connections
> before pgbench exits.
"sent for" -> "sent to"
> Attached is the updated version, v4.
+/* send cancel requests to all connections */
+static void
+cancel_all()
+{
+ for (int i = 0; i < nclients; i++)
+ {
+ char errbuf[1];
+ if (client_states[i].cancel != NULL)
+ (void) PQcancel(client_states[i].cancel, errbuf, sizeof(errbuf));
+ }
+}
+
Why in case of errors from PQCancel the error message is neglected? I
think it's better to print out the error message in case of error.
+ * On non-Windows, any callback function is not set. When SIGINT is
+ * received, CancelRequested is just set, and only thread #0 is interrupted
+ * and returns from waiting input from the backend. After that, the thread
+ * sends cancel requests to all benchmark queries.
The second line is a little bit long according to the coding
standard. Fix like this?
* On non-Windows, any callback function is not set. When SIGINT is
* received, CancelRequested is just set, and only thread #0 is
* interrupted and returns from waiting input from the backend. After
* that, the thread sends cancel requests to all benchmark queries.
Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: pgbnech: allow to cancel queries during benchmark
2024-01-15 07:49 Re: pgbnech: allow to cancel queries during benchmark Tatsuo Ishii <[email protected]>
@ 2024-01-19 07:51 ` Yugo NAGATA <[email protected]>
2024-01-19 08:46 ` Re: pgbnech: allow to cancel queries during benchmark Tatsuo Ishii <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Yugo NAGATA @ 2024-01-19 07:51 UTC (permalink / raw)
To: Tatsuo Ishii <[email protected]>; +Cc: [email protected]; [email protected]
On Mon, 15 Jan 2024 16:49:44 +0900 (JST)
Tatsuo Ishii <[email protected]> wrote:
> > On Wed, 6 Sep 2023 20:13:34 +0900
> > Yugo NAGATA <[email protected]> wrote:
> >
> >> I attached the updated patch v3. The changes since the previous
> >> patch includes the following;
> >>
> >> I removed the unnecessary condition (&& false) that you
> >> pointed out in [1].
> >>
> >> The test was rewritten by using IPC::Run signal() and integrated
> >> to "001_pgbench_with_server.pl". This test is skipped on Windows
> >> because SIGINT causes to terminate the test itself as discussed
> >> in [2] about query cancellation test in psql.
> >>
> >> I added some comments to describe how query cancellation is
> >> handled as I explained in [1].
> >>
> >> Also, I found the previous patch didn't work on Windows so fixed it.
> >> On non-Windows system, a thread waiting a response of long query can
> >> be interrupted by SIGINT, but on Windows, threads do not return from
> >> waiting until queries they are running are cancelled. This is because,
> >> when the signal is received, the system just creates a new thread to
> >> execute the callback function specified by setup_cancel_handler, and
> >> other thread continue to run[3]. Therefore, the queries have to be
> >> cancelled in the callback function.
> >>
> >> [1] https://www.postgresql.org/message-id/a58388ac-5411-4760-ea46-71324d8324cb%40mines-paristech.fr
> >> [2] https://www.postgresql.org/message-id/20230906004524.2fd6ee049f8a6c6f2690b99c%40sraoss.co.jp
> >> [3] https://learn.microsoft.com/en-us/windows/console/handlerroutine
> >
> > I found that --disable-thread-safety option was removed in 68a4b58eca0329.
> > So, I removed codes involving ENABLE_THREAD_SAFETY from the patch.
> >
> > Also, I wrote a commit log draft.
>
> > Previously, Ctrl+C during benchmark killed pgbench immediately,
> > but queries running at that time were not cancelled.
>
> Better to mention explicitely that queries keep on running on the
> backend. What about this?
>
> Previously, Ctrl+C during benchmark killed pgbench immediately, but
> queries were not canceled and they keep on running on the backend
> until they tried to send the result to pgbench.
Thank you for your comments. I agree with you, so I fixed the message
as your suggestion.
> > The commit
> > fixes this so that cancel requests are sent for all connections
> > before pgbench exits.
>
> "sent for" -> "sent to"
Fixed.
> > Attached is the updated version, v4.
>
> +/* send cancel requests to all connections */
> +static void
> +cancel_all()
> +{
> + for (int i = 0; i < nclients; i++)
> + {
> + char errbuf[1];
> + if (client_states[i].cancel != NULL)
> + (void) PQcancel(client_states[i].cancel, errbuf, sizeof(errbuf));
> + }
> +}
> +
>
> Why in case of errors from PQCancel the error message is neglected? I
> think it's better to print out the error message in case of error.
Is the message useful for pgbench users? I saw the error is ignored
in pg_dump, for example in bin/pg_dump/parallel.c
/*
* Send QueryCancel to leader connection, if enabled. Ignore errors,
* there's not much we can do about them anyway.
*/
if (signal_info.myAH != NULL && signal_info.myAH->connCancel != NULL)
(void) PQcancel(signal_info.myAH->connCancel,
errbuf, sizeof(errbuf));
> + * On non-Windows, any callback function is not set. When SIGINT is
> + * received, CancelRequested is just set, and only thread #0 is interrupted
> + * and returns from waiting input from the backend. After that, the thread
> + * sends cancel requests to all benchmark queries.
>
> The second line is a little bit long according to the coding
> standard. Fix like this?
>
> * On non-Windows, any callback function is not set. When SIGINT is
> * received, CancelRequested is just set, and only thread #0 is
> * interrupted and returns from waiting input from the backend. After
> * that, the thread sends cancel requests to all benchmark queries.
Fixed.
The attached is the updated patch, v5.
Regards,
Yugo Nagata
> Best reagards,
> --
> Tatsuo Ishii
> SRA OSS LLC
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
--
Yugo NAGATA <[email protected]>
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: pgbnech: allow to cancel queries during benchmark
2024-01-15 07:49 Re: pgbnech: allow to cancel queries during benchmark Tatsuo Ishii <[email protected]>
2024-01-19 07:51 ` Re: pgbnech: allow to cancel queries during benchmark Yugo NAGATA <[email protected]>
@ 2024-01-19 08:46 ` Tatsuo Ishii <[email protected]>
2024-01-24 13:17 ` Re: pgbnech: allow to cancel queries during benchmark Yugo NAGATA <[email protected]>
0 siblings, 1 reply; 27+ messages in thread
From: Tatsuo Ishii @ 2024-01-19 08:46 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]
>> +/* send cancel requests to all connections */
>> +static void
>> +cancel_all()
>> +{
>> + for (int i = 0; i < nclients; i++)
>> + {
>> + char errbuf[1];
>> + if (client_states[i].cancel != NULL)
>> + (void) PQcancel(client_states[i].cancel, errbuf, sizeof(errbuf));
>> + }
>> +}
>> +
>>
>> Why in case of errors from PQCancel the error message is neglected? I
>> think it's better to print out the error message in case of error.
>
> Is the message useful for pgbench users? I saw the error is ignored
> in pg_dump, for example in bin/pg_dump/parallel.c
I think the situation is different from pg_dump. Unlike pg_dump, if
PQcancel does not work, users can fix the problem by using
pg_terminate_backend or kill command. In order to make this work, an
appropriate error message is essential.
Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
^ permalink raw reply [nested|flat] 27+ messages in thread
* Re: pgbnech: allow to cancel queries during benchmark
2024-01-15 07:49 Re: pgbnech: allow to cancel queries during benchmark Tatsuo Ishii <[email protected]>
2024-01-19 07:51 ` Re: pgbnech: allow to cancel queries during benchmark Yugo NAGATA <[email protected]>
2024-01-19 08:46 ` Re: pgbnech: allow to cancel queries during benchmark Tatsuo Ishii <[email protected]>
@ 2024-01-24 13:17 ` Yugo NAGATA <[email protected]>
0 siblings, 0 replies; 27+ messages in thread
From: Yugo NAGATA @ 2024-01-24 13:17 UTC (permalink / raw)
To: Tatsuo Ishii <[email protected]>; +Cc: [email protected]; [email protected]
On Fri, 19 Jan 2024 17:46:03 +0900 (JST)
Tatsuo Ishii <[email protected]> wrote:
> >> +/* send cancel requests to all connections */
> >> +static void
> >> +cancel_all()
> >> +{
> >> + for (int i = 0; i < nclients; i++)
> >> + {
> >> + char errbuf[1];
> >> + if (client_states[i].cancel != NULL)
> >> + (void) PQcancel(client_states[i].cancel, errbuf, sizeof(errbuf));
> >> + }
> >> +}
> >> +
> >>
> >> Why in case of errors from PQCancel the error message is neglected? I
> >> think it's better to print out the error message in case of error.
> >
> > Is the message useful for pgbench users? I saw the error is ignored
> > in pg_dump, for example in bin/pg_dump/parallel.c
>
> I think the situation is different from pg_dump. Unlike pg_dump, if
> PQcancel does not work, users can fix the problem by using
> pg_terminate_backend or kill command. In order to make this work, an
> appropriate error message is essential.
Makes sense. I fixed to emit an error message when PQcancel fails.
Also, I added some comments about the signal handling on Windows
to explain why the different way than non-Windows is required;
+ * On Windows, a callback function is set in which query cancel requests
+ * are sent to all benchmark queries running in the backend. This is
+ * required because all threads running queries continue to run without
+ * interrupted even when the signal is received.
+ *
Attached is the updated patch, v6.
> Best reagards,
> --
> Tatsuo Ishii
> SRA OSS LLC
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
>
>
--
Yugo NAGATA <[email protected]>
^ permalink raw reply [nested|flat] 27+ messages in thread
end of thread, other threads:[~2024-01-24 13:17 UTC | newest]
Thread overview: 27+ 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]>
2020-12-02 05:11 [PATCH v8 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v10 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2020-12-02 05:11 [PATCH v9 2/4] WIP: Check for volatile defaults Justin Pryzby <[email protected]>
2024-01-15 07:49 Re: pgbnech: allow to cancel queries during benchmark Tatsuo Ishii <[email protected]>
2024-01-19 07:51 ` Re: pgbnech: allow to cancel queries during benchmark Yugo NAGATA <[email protected]>
2024-01-19 08:46 ` Re: pgbnech: allow to cancel queries during benchmark Tatsuo Ishii <[email protected]>
2024-01-24 13:17 ` Re: pgbnech: allow to cancel queries during benchmark Yugo NAGATA <[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