public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v9 2/4] WIP: Check for volatile defaults
10+ messages / 4 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; 10+ 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] 10+ messages in thread
* Re: Push down time-related SQLValue functions to foreign server
@ 2022-01-17 23:08 Tom Lane <[email protected]>
2022-01-18 08:52 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lane @ 2022-01-17 23:08 UTC (permalink / raw)
To: Alexander Pyhalov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers
Alexander Pyhalov <[email protected]> writes:
>> Perhaps in a MACRO?
> Changed this check to a macro, also fixed condition in
> is_foreign_param() and added test for it.
> Also fixed comment in prepare_query_params().
I took a quick look at this. I'm unconvinced that you need the
TIME_RELATED_SQLVALUE_FUNCTION macro, mainly because I think testing
that in is_foreign_param() is pointless. The only way we'll be seeing
a SQLValueFunction in is_foreign_param() is if we decided it was
shippable, so you really don't need two duplicate tests.
(In the same vein, I would not bother with including a switch in
deparseSQLValueFunction that knows about these opcodes explicitly.
Just use the typmod field; exprTypmod() does.)
I also find it pretty bizarre that contain_unsafe_functions
isn't placed beside its one caller. Maybe that indicates that
is_foreign_expr is mis-located and should be in shippable.c.
More generally, it's annoying that you had to copy-and-paste
all of contain_mutable_functions to make this. That creates
a rather nasty maintenance hazard for future hackers, who will
need to keep these widely-separated functions in sync. Not
sure what to do about that though. Do we want to extend
contain_mutable_functions itself to cover this use-case?
The test cases seem a bit overkill --- what is the point of the
two nigh-identical PREPARE tests, or the GROUP BY test? If
anything is broken about GROUP BY, surely it's not specific
to this patch.
I'm not really convinced by the premise of 0002, particularly
this bit:
static bool
-contain_mutable_functions_checker(Oid func_id, void *context)
+contain_unsafe_functions_checker(Oid func_id, void *context)
{
- return (func_volatile(func_id) != PROVOLATILE_IMMUTABLE);
+ /* now() is stable, but we can ship it as it's replaced by parameter */
+ return !(func_volatile(func_id) == PROVOLATILE_IMMUTABLE || func_id == F_NOW);
}
The point of the check_functions_in_node callback API is to verify
the mutability of functions that are embedded in various sorts of
expression nodes ... but they might not be in a plain FuncExpr node,
which is the only case you'll deparse correctly. It might be that
now() cannot legally appear in any of the other node types that
check_functions_in_node knows about, but I'm not quite convinced
of that, and even less convinced that that'll stay true as we add
more expression node types. Also, if we commit this, for sure
some poor soul will try to expand the logic to some other stable
function that *can* appear in those contexts, and that'll be broken.
The implementation of converting now() to CURRENT_TIMESTAMP
seems like an underdocumented kluge, too.
On the whole I'm a bit inclined to drop 0002; I'm not sure it's
worth the trouble.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Push down time-related SQLValue functions to foreign server
2022-01-17 23:08 Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
@ 2022-01-18 08:52 ` Alexander Pyhalov <[email protected]>
2022-01-18 16:53 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Alexander Pyhalov @ 2022-01-18 08:52 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers
Hi.
Tom Lane писал 2022-01-18 02:08:
> Alexander Pyhalov <[email protected]> writes:
>>> Perhaps in a MACRO?
>
>> Changed this check to a macro, also fixed condition in
>> is_foreign_param() and added test for it.
>> Also fixed comment in prepare_query_params().
>
> I took a quick look at this. I'm unconvinced that you need the
> TIME_RELATED_SQLVALUE_FUNCTION macro, mainly because I think testing
> that in is_foreign_param() is pointless. The only way we'll be seeing
> a SQLValueFunction in is_foreign_param() is if we decided it was
> shippable, so you really don't need two duplicate tests.
> (In the same vein, I would not bother with including a switch in
> deparseSQLValueFunction that knows about these opcodes explicitly.
> Just use the typmod field; exprTypmod() does.)
Yes, sure, is_foreign_param() is called only when is_foreign_expr() is
true.
Simplified this part.
>
> I also find it pretty bizarre that contain_unsafe_functions
> isn't placed beside its one caller. Maybe that indicates that
> is_foreign_expr is mis-located and should be in shippable.c.
>
> More generally, it's annoying that you had to copy-and-paste
> all of contain_mutable_functions to make this. That creates
> a rather nasty maintenance hazard for future hackers, who will
> need to keep these widely-separated functions in sync. Not
> sure what to do about that though. Do we want to extend
> contain_mutable_functions itself to cover this use-case?
I've moved logic to contain_mutable_functions_skip_sqlvalues(), it
uses the same subroutines as contain_mutable_functions().
Should we instead just add one more parameter to
contain_mutable_functions()?
I'm not sure that it's a good idea given that
contain_mutable_functions() seems to be an
external interface.
>
> The test cases seem a bit overkill --- what is the point of the
> two nigh-identical PREPARE tests, or the GROUP BY test? If
> anything is broken about GROUP BY, surely it's not specific
> to this patch.
I've removed PREPARE tests, but GROUP BY test checks
foreign_grouping_ok()/is_foreign_param() path,
so I think it's useful.
>
> I'm not really convinced by the premise of 0002, particularly
> this bit:
>
> static bool
> -contain_mutable_functions_checker(Oid func_id, void *context)
> +contain_unsafe_functions_checker(Oid func_id, void *context)
> {
> - return (func_volatile(func_id) != PROVOLATILE_IMMUTABLE);
> + /* now() is stable, but we can ship it as it's replaced by parameter
> */
> + return !(func_volatile(func_id) == PROVOLATILE_IMMUTABLE || func_id
> == F_NOW);
> }
>
> The point of the check_functions_in_node callback API is to verify
> the mutability of functions that are embedded in various sorts of
> expression nodes ... but they might not be in a plain FuncExpr node,
> which is the only case you'll deparse correctly. It might be that
> now() cannot legally appear in any of the other node types that
> check_functions_in_node knows about, but I'm not quite convinced
> of that, and even less convinced that that'll stay true as we add
> more expression node types. Also, if we commit this, for sure
> some poor soul will try to expand the logic to some other stable
> function that *can* appear in those contexts, and that'll be broken.
>
> The implementation of converting now() to CURRENT_TIMESTAMP
> seems like an underdocumented kluge, too.
>
> On the whole I'm a bit inclined to drop 0002; I'm not sure it's
> worth the trouble.
>
OK. Let's drop it for now.
--
Best regards,
Alexander Pyhalov,
Postgres Professional
Attachments:
[text/x-diff] 0001-SQLValue-functions-pushdown.patch (15.5K, ../../[email protected]/2-0001-SQLValue-functions-pushdown.patch)
download | inline diff:
From 9a67c52b57e0b50a3702598aa0b3e8af89569a9c Mon Sep 17 00:00:00 2001
From: Alexander Pyhalov <[email protected]>
Date: Thu, 29 Jul 2021 11:45:28 +0300
Subject: [PATCH] SQLValue functions pushdown
current_timestamp, localtimestamp and similar SQLValue functions
can be computed locally and sent to remote side as parameters values.
---
contrib/postgres_fdw/deparse.c | 83 ++++++++++++++++-
.../postgres_fdw/expected/postgres_fdw.out | 88 +++++++++++++++++++
contrib/postgres_fdw/postgres_fdw.c | 9 +-
contrib/postgres_fdw/postgres_fdw.h | 1 +
contrib/postgres_fdw/shippable.c | 3 +
contrib/postgres_fdw/sql/postgres_fdw.sql | 22 +++++
src/backend/optimizer/util/clauses.c | 27 +++++-
src/include/optimizer/optimizer.h | 1 +
8 files changed, 226 insertions(+), 8 deletions(-)
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index bf12eac0288..a398e1b2174 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -109,6 +109,15 @@ typedef struct deparse_expr_cxt
appendStringInfo((buf), "%s%d.", REL_ALIAS_PREFIX, (varno))
#define SUBQUERY_REL_ALIAS_PREFIX "s"
#define SUBQUERY_COL_ALIAS_PREFIX "c"
+#define TIME_RELATED_SQLVALUE_FUNCTION(s) \
+ (s->op == SVFOP_CURRENT_TIMESTAMP || \
+ s->op == SVFOP_CURRENT_TIMESTAMP_N || \
+ s->op == SVFOP_CURRENT_TIME || \
+ s->op == SVFOP_CURRENT_TIME_N || \
+ s->op == SVFOP_LOCALTIMESTAMP || \
+ s->op == SVFOP_LOCALTIMESTAMP_N || \
+ s->op == SVFOP_LOCALTIME || \
+ s->op == SVFOP_LOCALTIME_N)
/*
* Functions to determine whether an expression can be evaluated safely on
@@ -158,6 +167,7 @@ static void deparseDistinctExpr(DistinctExpr *node, deparse_expr_cxt *context);
static void deparseScalarArrayOpExpr(ScalarArrayOpExpr *node,
deparse_expr_cxt *context);
static void deparseRelabelType(RelabelType *node, deparse_expr_cxt *context);
+static void deparseSQLValueFunction(SQLValueFunction *node, deparse_expr_cxt *context);
static void deparseBoolExpr(BoolExpr *node, deparse_expr_cxt *context);
static void deparseNullTest(NullTest *node, deparse_expr_cxt *context);
static void deparseCaseExpr(CaseExpr *node, deparse_expr_cxt *context);
@@ -274,7 +284,7 @@ is_foreign_expr(PlannerInfo *root,
* be able to make this choice with more granularity. (We check this last
* because it requires a lot of expensive catalog lookups.)
*/
- if (contain_mutable_functions((Node *) expr))
+ if (contain_mutable_functions_skip_sqlvalues((Node *) expr))
return false;
/* OK to evaluate on the remote server */
@@ -619,6 +629,30 @@ foreign_expr_walker(Node *node,
state = FDW_COLLATE_UNSAFE;
}
break;
+ case T_SQLValueFunction:
+ {
+ SQLValueFunction *s = (SQLValueFunction *) node;
+
+ /*
+ * For now only time-related SQLValue functions are supported.
+ * We can push down localtimestamp and localtime as we compute
+ * them locally.
+ */
+ if (s->op != SVFOP_CURRENT_TIMESTAMP &&
+ s->op != SVFOP_CURRENT_TIMESTAMP_N &&
+ s->op != SVFOP_CURRENT_TIME &&
+ s->op != SVFOP_CURRENT_TIME_N &&
+ s->op != SVFOP_LOCALTIMESTAMP &&
+ s->op != SVFOP_LOCALTIMESTAMP_N &&
+ s->op != SVFOP_LOCALTIME &&
+ s->op != SVFOP_LOCALTIME_N)
+ return false;
+
+ /* Timestamp or time are not collatable */
+ collation = InvalidOid;
+ state = FDW_COLLATE_NONE;
+ }
+ break;
case T_BoolExpr:
{
BoolExpr *b = (BoolExpr *) node;
@@ -1032,6 +1066,12 @@ is_foreign_param(PlannerInfo *root,
case T_Param:
/* Params always have to be sent to the foreign server */
return true;
+ case T_SQLValueFunction:
+ /*
+ * We can get here only if is_foreign_expr(expr) returned
+ * true, so it's a supported SQLValueFunction.
+ */
+ return true;
default:
break;
}
@@ -2604,6 +2644,9 @@ deparseExpr(Expr *node, deparse_expr_cxt *context)
case T_RelabelType:
deparseRelabelType((RelabelType *) node, context);
break;
+ case T_SQLValueFunction:
+ deparseSQLValueFunction((SQLValueFunction *) node, context);
+ break;
case T_BoolExpr:
deparseBoolExpr((BoolExpr *) node, context);
break;
@@ -3192,6 +3235,44 @@ deparseRelabelType(RelabelType *node, deparse_expr_cxt *context)
node->resulttypmod));
}
+/*
+ * Deparse a SQLValueFunction node
+ */
+static void
+deparseSQLValueFunction(SQLValueFunction *node, deparse_expr_cxt *context)
+{
+ int32 typmod = node->typmod;
+
+ Assert(node->type != InvalidOid);
+
+ /* Treat like a Param */
+ if (context->params_list)
+ {
+ int pindex = 0;
+ ListCell *lc;
+
+ /* find its index in params_list */
+ foreach(lc, *context->params_list)
+ {
+ pindex++;
+ if (equal(node, (Node *) lfirst(lc)))
+ break;
+ }
+ if (lc == NULL)
+ {
+ /* not in list, so add it */
+ pindex++;
+ *context->params_list = lappend(*context->params_list, node);
+ }
+
+ printRemoteParam(pindex, node->type, typmod, context);
+ }
+ else
+ {
+ printRemotePlaceholder(node->type, typmod, context);
+ }
+}
+
/*
* Deparse a BoolExpr node.
*/
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 7d6f7d9e3df..29345f251d7 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -1073,6 +1073,94 @@ SELECT * FROM ft1 t1 WHERE t1.c1 === t1.c2 order by t1.c2 limit 1;
1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
(1 row)
+-- Test SQLValue functions pushdown
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c4 FROM ft2 WHERE c4 = current_timestamp;
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft2
+ Output: c1, c2, c4
+ Remote SQL: SELECT "C 1", c2, c4 FROM "S 1"."T 1" WHERE ((c4 = $1::timestamp with time zone))
+(3 rows)
+
+SELECT c1,c2,c4 FROM ft2 WHERE c4 = current_timestamp;
+ c1 | c2 | c4
+----+----+----
+(0 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c5 FROM ft2 WHERE c5 > localtimestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft2
+ Output: c1, c2, c5
+ Remote SQL: SELECT "C 1", c2, c5 FROM "S 1"."T 1" WHERE (("C 1" > 990)) AND ((c5 > ($1::timestamp without time zone - '1000 years'::interval))) ORDER BY "C 1" ASC NULLS LAST
+(3 rows)
+
+SELECT c1,c2,c5 FROM ft2 WHERE c5 > localtimestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+ c1 | c2 | c5
+------+----+--------------------------
+ 991 | 1 | Thu Apr 02 00:00:00 1970
+ 992 | 2 | Fri Apr 03 00:00:00 1970
+ 993 | 3 | Sat Apr 04 00:00:00 1970
+ 994 | 4 | Sun Apr 05 00:00:00 1970
+ 995 | 5 | Mon Apr 06 00:00:00 1970
+ 996 | 6 | Tue Apr 07 00:00:00 1970
+ 997 | 7 | Wed Apr 08 00:00:00 1970
+ 998 | 8 | Thu Apr 09 00:00:00 1970
+ 999 | 9 | Fri Apr 10 00:00:00 1970
+ 1000 | 0 | Thu Jan 01 00:00:00 1970
+(10 rows)
+
+-- not shippable due to timestamptz arithmetic
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c4 FROM ft2 WHERE c4 > current_timestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft2
+ Output: c1, c2, c4
+ Filter: (ft2.c4 > (CURRENT_TIMESTAMP - '@ 1000 years'::interval))
+ Remote SQL: SELECT "C 1", c2, c4 FROM "S 1"."T 1" WHERE (("C 1" > 990)) ORDER BY "C 1" ASC NULLS LAST
+(4 rows)
+
+SELECT c1,c2,c4 FROM ft2 WHERE c4 > current_timestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+ c1 | c2 | c4
+------+----+------------------------------
+ 991 | 1 | Thu Apr 02 00:00:00 1970 PST
+ 992 | 2 | Fri Apr 03 00:00:00 1970 PST
+ 993 | 3 | Sat Apr 04 00:00:00 1970 PST
+ 994 | 4 | Sun Apr 05 00:00:00 1970 PST
+ 995 | 5 | Mon Apr 06 00:00:00 1970 PST
+ 996 | 6 | Tue Apr 07 00:00:00 1970 PST
+ 997 | 7 | Wed Apr 08 00:00:00 1970 PST
+ 998 | 8 | Thu Apr 09 00:00:00 1970 PST
+ 999 | 9 | Fri Apr 10 00:00:00 1970 PST
+ 1000 | 0 | Thu Jan 01 00:00:00 1970 PST
+(10 rows)
+
+-- direct modify
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE ft2 SET c4 = current_timestamp WHERE c4 < current_timestamp;
+ QUERY PLAN
+--------------------------------------------------------------------------------------------------------------------------
+ Update on public.ft2
+ -> Foreign Update on public.ft2
+ Remote SQL: UPDATE "S 1"."T 1" SET c4 = $1::timestamp with time zone WHERE ((c4 < $1::timestamp with time zone))
+(3 rows)
+
+-- check that we don't try to push down parameter in group by
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT sum(c1), current_timestamp FROM ft2 WHERE c1 > 990 GROUP BY current_timestamp;
+ QUERY PLAN
+-------------------------------------------------------------------------
+ GroupAggregate
+ Output: sum(c1), (CURRENT_TIMESTAMP)
+ Group Key: CURRENT_TIMESTAMP
+ -> Foreign Scan on public.ft2
+ Output: CURRENT_TIMESTAMP, c1
+ Remote SQL: SELECT "C 1" FROM "S 1"."T 1" WHERE (("C 1" > 990))
+(6 rows)
+
-- Test CASE pushdown
EXPLAIN (VERBOSE, COSTS OFF)
SELECT c1,c2,c3 FROM ft2 WHERE CASE WHEN c1 > 990 THEN c1 END < 1000 ORDER BY c1;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 09a3f5e23cb..6aa3a20f20c 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4834,12 +4834,9 @@ prepare_query_params(PlanState *node,
}
/*
- * Prepare remote-parameter expressions for evaluation. (Note: in
- * practice, we expect that all these expressions will be just Params, so
- * we could possibly do something more efficient than using the full
- * expression-eval machinery for this. But probably there would be little
- * benefit, and it'd require postgres_fdw to know more than is desirable
- * about Param evaluation.)
+ * Prepare remote-parameter expressions for evaluation. (Note: we cannot
+ * expect that all these expressions will be just Params, so we should use
+ * the full expression-eval machinery for this).
*/
*param_exprs = ExecInitExprList(fdw_exprs, node);
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 8ae79e97e44..0ab12e90e05 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -231,5 +231,6 @@ extern const char *get_jointype_name(JoinType jointype);
/* in shippable.c */
extern bool is_builtin(Oid objectId);
extern bool is_shippable(Oid objectId, Oid classId, PgFdwRelationInfo *fpinfo);
+extern bool contain_unsafe_functions(Node *clause);
#endif /* POSTGRES_FDW_H */
diff --git a/contrib/postgres_fdw/shippable.c b/contrib/postgres_fdw/shippable.c
index 8e759da00d5..7227a1be35b 100644
--- a/contrib/postgres_fdw/shippable.c
+++ b/contrib/postgres_fdw/shippable.c
@@ -25,9 +25,12 @@
#include "access/transam.h"
#include "catalog/dependency.h"
+#include "catalog/pg_proc.h"
+#include "nodes/nodeFuncs.h"
#include "postgres_fdw.h"
#include "utils/hsearch.h"
#include "utils/inval.h"
+#include "utils/lsyscache.h"
#include "utils/syscache.h"
/* Hash table for caching the results of shippability lookups */
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 9eb673e3693..69d15ede05e 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -413,6 +413,28 @@ EXPLAIN (VERBOSE, COSTS OFF)
SELECT * FROM ft1 t1 WHERE t1.c1 === t1.c2 order by t1.c2 limit 1;
SELECT * FROM ft1 t1 WHERE t1.c1 === t1.c2 order by t1.c2 limit 1;
+-- Test SQLValue functions pushdown
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c4 FROM ft2 WHERE c4 = current_timestamp;
+SELECT c1,c2,c4 FROM ft2 WHERE c4 = current_timestamp;
+
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c5 FROM ft2 WHERE c5 > localtimestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+SELECT c1,c2,c5 FROM ft2 WHERE c5 > localtimestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+
+-- not shippable due to timestamptz arithmetic
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c4 FROM ft2 WHERE c4 > current_timestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+SELECT c1,c2,c4 FROM ft2 WHERE c4 > current_timestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+
+-- direct modify
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE ft2 SET c4 = current_timestamp WHERE c4 < current_timestamp;
+
+-- check that we don't try to push down parameter in group by
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT sum(c1), current_timestamp FROM ft2 WHERE c1 > 990 GROUP BY current_timestamp;
+
-- Test CASE pushdown
EXPLAIN (VERBOSE, COSTS OFF)
SELECT c1,c2,c3 FROM ft2 WHERE CASE WHEN c1 > 990 THEN c1 END < 1000 ORDER BY c1;
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index a707dc9f26a..4975bfc62c3 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -91,6 +91,11 @@ typedef struct
List *safe_param_ids; /* PARAM_EXEC Param IDs to treat as safe */
} max_parallel_hazard_context;
+typedef struct
+{
+ bool skip_sqlvalues;
+} mutable_functions_context;
+
static bool contain_agg_clause_walker(Node *node, void *context);
static bool find_window_functions_walker(Node *node, WindowFuncLists *lists);
static bool contain_subplans_walker(Node *node, void *context);
@@ -366,6 +371,20 @@ contain_mutable_functions(Node *clause)
return contain_mutable_functions_walker(clause, NULL);
}
+/*
+ * contain_mutable_functions_skip_sqlvalues
+ * Special purpose version of contain_mutable_functions() for use in
+ * FDWs: ignore SQLValueFunction, but detect other mutable functions.
+ */
+bool
+contain_mutable_functions_skip_sqlvalues(Node *clause)
+{
+ mutable_functions_context context;
+
+ context.skip_sqlvalues = true;
+ return contain_mutable_functions_walker(clause, &context);
+}
+
static bool
contain_mutable_functions_checker(Oid func_id, void *context)
{
@@ -384,8 +403,14 @@ contain_mutable_functions_walker(Node *node, void *context)
if (IsA(node, SQLValueFunction))
{
+ bool skip_sqlvalues = false;
+
+ if (context && ((mutable_functions_context *) context)->skip_sqlvalues)
+ skip_sqlvalues = true;
+
/* all variants of SQLValueFunction are stable */
- return true;
+ if (!skip_sqlvalues)
+ return true;
}
if (IsA(node, NextValueExpr))
diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h
index 6b8ee0c69fa..bce1597d3c1 100644
--- a/src/include/optimizer/optimizer.h
+++ b/src/include/optimizer/optimizer.h
@@ -141,6 +141,7 @@ extern Expr *canonicalize_qual(Expr *qual, bool is_check);
/* in util/clauses.c: */
extern bool contain_mutable_functions(Node *clause);
+extern bool contain_mutable_functions_skip_sqlvalues(Node *clause);
extern bool contain_volatile_functions(Node *clause);
extern bool contain_volatile_functions_not_nextval(Node *clause);
--
2.25.1
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Push down time-related SQLValue functions to foreign server
2022-01-17 23:08 Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 08:52 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
@ 2022-01-18 16:53 ` Tom Lane <[email protected]>
2022-01-18 19:02 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lane @ 2022-01-18 16:53 UTC (permalink / raw)
To: Alexander Pyhalov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers
Alexander Pyhalov <[email protected]> writes:
> [ updated patch ]
Thanks for updating the patch. (BTW, please attach version numbers
to new patch versions, to avoid confusion.)
However, before we proceed any further with this patch, I think we
really ought to stop and think about the question I raised last
night: why are we building a one-off feature for SQLValueFunction?
Wouldn't the same parameter-substitution mechanism work for *any*
stable expression that doesn't contain remote Vars? That would
subsume the now() case as well as plenty of others.
So far the only counterexample I've been able to come up with is
that shipping values of reg* types might not be too safe, because
the remote side might not have the same objects. For example
consider these two potential quals:
WHERE remote_oid_column = CURRENT_ROLE::regrole
WHERE remote_text_column = CURRENT_ROLE::text
Say we're running as user 'joe' and that role doesn't exist on the
remote server. Then executing the first WHERE locally is fine, but
shipping it to the remote would cause a failure because the remote's
regrolein() will fail to convert the parameter value. But the second
case is quite non-problematic, because what will be sent over is just
some uninterpreted text.
In point of fact, this hazard doesn't have anything to do with stable
or not-stable subexpressions --- for example,
WHERE remote_oid_column = 'joe'::regrole
is just as unsafe, even though the value under consideration is a
*constant*. Maybe there is something in postgres_fdw that would stop
it from shipping this qual, but I don't recall seeing it, so I wonder
if there's a pre-existing bug here.
So it seems like we need a check to prevent generating remote Params
that are of "unsafe" types, but this is a type issue not an expression
issue --- as long as an expression is stable and does not yield an
unsafe-to-ship data type, why can't we treat it as a Param?
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Push down time-related SQLValue functions to foreign server
2022-01-17 23:08 Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 08:52 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 16:53 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
@ 2022-01-18 19:02 ` Alexander Pyhalov <[email protected]>
2022-01-18 19:11 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Alexander Pyhalov @ 2022-01-18 19:02 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers
Tom Lane писал 2022-01-18 19:53:
> Alexander Pyhalov <[email protected]> writes:
>> [ updated patch ]
>
> Thanks for updating the patch. (BTW, please attach version numbers
> to new patch versions, to avoid confusion.)
>
> However, before we proceed any further with this patch, I think we
> really ought to stop and think about the question I raised last
> night: why are we building a one-off feature for SQLValueFunction?
> Wouldn't the same parameter-substitution mechanism work for *any*
> stable expression that doesn't contain remote Vars? That would
> subsume the now() case as well as plenty of others.
>
Hi.
I think, I can extend it to allow any stable function (not just
immutable/sqlvalues) in is_foreign_expr(), but not so sure about
"expression".
Perhaps, at top of deparseExpr() we can check that expression doesn't
contain vars, params, but contains stable function, and deparse it as
param.
This means we'll translate something like
explain select * from t where d > now() - '1 day'::interval;
to
select * from t where d > $1;
The question is how will we reliably determine its typmod (given that I
read in exprTypmod() comment
"returns the type-specific modifier of the expression's result type, if
it can be determined. In many cases, it can't".
What do we save if we don't ship whole expression as param, but only
stable functions? Allowing them seems to be more straightforward.
--
Best regards,
Alexander Pyhalov,
Postgres Professional
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Push down time-related SQLValue functions to foreign server
2022-01-17 23:08 Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 08:52 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 16:53 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 19:02 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
@ 2022-01-18 19:11 ` Tom Lane <[email protected]>
2022-01-18 20:01 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lane @ 2022-01-18 19:11 UTC (permalink / raw)
To: Alexander Pyhalov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers
Alexander Pyhalov <[email protected]> writes:
> Tom Lane писал 2022-01-18 19:53:
>> However, before we proceed any further with this patch, I think we
>> really ought to stop and think about the question I raised last
>> night: why are we building a one-off feature for SQLValueFunction?
>> Wouldn't the same parameter-substitution mechanism work for *any*
>> stable expression that doesn't contain remote Vars? That would
>> subsume the now() case as well as plenty of others.
> This means we'll translate something like
> explain select * from t where d > now() - '1 day'::interval;
> to
> select * from t where d > $1;
Right.
> The question is how will we reliably determine its typmod (given that I
> read in exprTypmod() comment
> "returns the type-specific modifier of the expression's result type, if
> it can be determined. In many cases, it can't".
I don't think we need to. If exprTypmod() says the typmod is -1,
then that's what it is.
> What do we save if we don't ship whole expression as param, but only
> stable functions? Allowing them seems to be more straightforward.
How so? Right off the bat, you get rid of the need for your own
version of contain_mutable_function. ISTM this approach can probably
be implemented in a patch that's noticeably smaller than what you
have now. It'll likely be touching entirely different places in
postgres_fdw, of course.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Push down time-related SQLValue functions to foreign server
2022-01-17 23:08 Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 08:52 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 16:53 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 19:02 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 19:11 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
@ 2022-01-18 20:01 ` Tom Lane <[email protected]>
2022-01-20 13:42 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lane @ 2022-01-18 20:01 UTC (permalink / raw)
To: Alexander Pyhalov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers
I wrote:
> Alexander Pyhalov <[email protected]> writes:
>> This means we'll translate something like
>> explain select * from t where d > now() - '1 day'::interval;
>> to
>> select * from t where d > $1;
> Right.
After thinking about that a bit more, I see that this will result
in a major redefinition of what is "shippable". Right now, we do not
consider the above WHERE clause to be shippable, not only because of
now() but because the timestamptz-minus-interval operator is dependent
on the timezone setting, which might be different at the remote.
But if we evaluate that operator locally and send its result as a
parameter, the objection vanishes. In fact, I don't think we even
need to require the subexpression to contain only built-in functions.
Its result still has to be of a built-in type, but that's a much
weaker restriction.
So this is going to require significant restructuring of both
is_foreign_expr and deparse.c, which I realize may be more than
you bargained for. But if you want to tackle it, I think what
we want to do is divide potentially-shippable expressions into
three sorts of components:
1. Remote Vars, which obviously ship as-is.
2. Locally-evaluatable subexpressions, which must contain no
remote Vars, must be stable or immutable, and must have a result
type that we consider safe to ship. If such a subexpression
is just a Const, we ship it as a constant, otherwise we evaluate
the value at runtime and ship a parameter.
3. Superstructure, which consists of all expression nodes having
at least one remote Var below them. These nodes have to be
shippable according to the existing definition, so that we know
that the remote server will evaluate them just as we would.
If we keep the existing division of labor between is_foreign_expr
and deparse.c, I fear that there's going to be a lot of duplicated
code as well as duplicative planning effort. I wonder if it'd be
wise to combine those operations into a single expression-scanning
process that determines whether an expression is safe to ship and
produces the translated string immediately if it is.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Push down time-related SQLValue functions to foreign server
2022-01-17 23:08 Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 08:52 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 16:53 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 19:02 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 19:11 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 20:01 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
@ 2022-01-20 13:42 ` Alexander Pyhalov <[email protected]>
2022-01-21 19:57 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Alexander Pyhalov @ 2022-01-20 13:42 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers
Tom Lane писал 2022-01-18 23:01:
> I wrote:
>> Alexander Pyhalov <[email protected]> writes:
>>> This means we'll translate something like
>>> explain select * from t where d > now() - '1 day'::interval;
>>> to
>>> select * from t where d > $1;
>
>> Right.
>
> After thinking about that a bit more, I see that this will result
> in a major redefinition of what is "shippable". Right now, we do not
> consider the above WHERE clause to be shippable, not only because of
> now() but because the timestamptz-minus-interval operator is dependent
> on the timezone setting, which might be different at the remote.
> But if we evaluate that operator locally and send its result as a
> parameter, the objection vanishes. In fact, I don't think we even
> need to require the subexpression to contain only built-in functions.
> Its result still has to be of a built-in type, but that's a much
> weaker restriction.
>
Hi.
So far I have the following prototype. It seems to be working, but I
think it can be enhanced.
At least, some sort of caching seems to be necessary for
is_stable_expr().
1) Now expression can be either 'stable shippable' or 'shippable
according to old rules'. We check if it's 'stable shippable' in
foreign_expr_walker(),
is_foreign_param() and deparseExpr(). All such exprs are replaced by
params while deparsing.
2) contain_mutable_functions() now is calculated only for current node,
if node is not considered 'stable shippable'.
Is it step in the right direction or do I miss something?
--
Best regards,
Alexander Pyhalov,
Postgres Professional
Attachments:
[text/x-diff] v04-0001-Push-down-stable-expressions.patch (50.1K, ../../[email protected]/2-v04-0001-Push-down-stable-expressions.patch)
download | inline diff:
From 60e6e0bf98326cb557c70a365797026e9925b7a3 Mon Sep 17 00:00:00 2001
From: Alexander Pyhalov <[email protected]>
Date: Thu, 29 Jul 2021 11:45:28 +0300
Subject: [PATCH] Push down stable expressions
Stable expressions can be computed locally and sent to remote side as parameters values.
---
contrib/postgres_fdw/deparse.c | 1162 +++++++++--------
.../postgres_fdw/expected/postgres_fdw.out | 87 ++
contrib/postgres_fdw/postgres_fdw.c | 9 +-
contrib/postgres_fdw/postgres_fdw.h | 1 +
contrib/postgres_fdw/shippable.c | 74 ++
contrib/postgres_fdw/sql/postgres_fdw.sql | 22 +
6 files changed, 811 insertions(+), 544 deletions(-)
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index bf12eac0288..33a79026574 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -158,6 +158,7 @@ static void deparseDistinctExpr(DistinctExpr *node, deparse_expr_cxt *context);
static void deparseScalarArrayOpExpr(ScalarArrayOpExpr *node,
deparse_expr_cxt *context);
static void deparseRelabelType(RelabelType *node, deparse_expr_cxt *context);
+static void deparseStableExpr(Expr *node, deparse_expr_cxt *context);
static void deparseBoolExpr(BoolExpr *node, deparse_expr_cxt *context);
static void deparseNullTest(NullTest *node, deparse_expr_cxt *context);
static void deparseCaseExpr(CaseExpr *node, deparse_expr_cxt *context);
@@ -267,20 +268,17 @@ is_foreign_expr(PlannerInfo *root,
if (loc_cxt.state == FDW_COLLATE_UNSAFE)
return false;
- /*
- * An expression which includes any mutable functions can't be sent over
- * because its result is not stable. For example, sending now() remote
- * side could cause confusion from clock offsets. Future versions might
- * be able to make this choice with more granularity. (We check this last
- * because it requires a lot of expensive catalog lookups.)
- */
- if (contain_mutable_functions((Node *) expr))
- return false;
/* OK to evaluate on the remote server */
return true;
}
+static bool
+contain_mutable_functions_checker(Oid func_id, void *context)
+{
+ return (func_volatile(func_id) != PROVOLATILE_IMMUTABLE);
+}
+
/*
* Check if expression is safe to execute remotely, and return true if so.
*
@@ -321,616 +319,650 @@ foreign_expr_walker(Node *node,
inner_cxt.collation = InvalidOid;
inner_cxt.state = FDW_COLLATE_NONE;
- switch (nodeTag(node))
+ if (is_stable_expr(node))
+ {
+ collation = exprCollation(node);
+ if (collation == InvalidOid || collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
+ }
+ else
{
- case T_Var:
- {
- Var *var = (Var *) node;
- /*
- * If the Var is from the foreign table, we consider its
- * collation (if any) safe to use. If it is from another
- * table, we treat its collation the same way as we would a
- * Param's collation, ie it's not safe for it to have a
- * non-default collation.
- */
- if (bms_is_member(var->varno, glob_cxt->relids) &&
- var->varlevelsup == 0)
+ /*
+ * An expression which includes any mutable functions can't be sent
+ * over because its result is not stable.
+ */
+ if (check_functions_in_node(node, contain_mutable_functions_checker, NULL))
+ return false;
+
+ switch (nodeTag(node))
+ {
+ case T_Var:
{
- /* Var belongs to foreign table */
+ Var *var = (Var *) node;
/*
- * System columns other than ctid should not be sent to
- * the remote, since we don't make any effort to ensure
- * that local and remote values match (tableoid, in
- * particular, almost certainly doesn't match).
+ * If the Var is from the foreign table, we consider its
+ * collation (if any) safe to use. If it is from another
+ * table, we treat its collation the same way as we would
+ * a Param's collation, ie it's not safe for it to have a
+ * non-default collation.
*/
- if (var->varattno < 0 &&
- var->varattno != SelfItemPointerAttributeNumber)
- return false;
-
- /* Else check the collation */
- collation = var->varcollid;
- state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE;
- }
- else
- {
- /* Var belongs to some other table */
- collation = var->varcollid;
- if (collation == InvalidOid ||
- collation == DEFAULT_COLLATION_OID)
+ if (bms_is_member(var->varno, glob_cxt->relids) &&
+ var->varlevelsup == 0)
{
+ /* Var belongs to foreign table */
+
/*
- * It's noncollatable, or it's safe to combine with a
- * collatable foreign Var, so set state to NONE.
+ * System columns other than ctid should not be sent
+ * to the remote, since we don't make any effort to
+ * ensure that local and remote values match
+ * (tableoid, in particular, almost certainly doesn't
+ * match).
*/
- state = FDW_COLLATE_NONE;
+ if (var->varattno < 0 &&
+ var->varattno != SelfItemPointerAttributeNumber)
+ return false;
+
+ /* Else check the collation */
+ collation = var->varcollid;
+ state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE;
}
else
{
- /*
- * Do not fail right away, since the Var might appear
- * in a collation-insensitive context.
- */
- state = FDW_COLLATE_UNSAFE;
+ /* Var belongs to some other table */
+ collation = var->varcollid;
+ if (collation == InvalidOid ||
+ collation == DEFAULT_COLLATION_OID)
+ {
+ /*
+ * It's noncollatable, or it's safe to combine
+ * with a collatable foreign Var, so set state to
+ * NONE.
+ */
+ state = FDW_COLLATE_NONE;
+ }
+ else
+ {
+ /*
+ * Do not fail right away, since the Var might
+ * appear in a collation-insensitive context.
+ */
+ state = FDW_COLLATE_UNSAFE;
+ }
}
}
- }
- break;
- case T_Const:
- {
- Const *c = (Const *) node;
+ break;
+ case T_Const:
+ {
+ Const *c = (Const *) node;
- /*
- * If the constant has nondefault collation, either it's of a
- * non-builtin type, or it reflects folding of a CollateExpr.
- * It's unsafe to send to the remote unless it's used in a
- * non-collation-sensitive context.
- */
- collation = c->constcollid;
- if (collation == InvalidOid ||
- collation == DEFAULT_COLLATION_OID)
- state = FDW_COLLATE_NONE;
- else
- state = FDW_COLLATE_UNSAFE;
- }
- break;
- case T_Param:
- {
- Param *p = (Param *) node;
+ /*
+ * If the constant has nondefault collation, either it's
+ * of a non-builtin type, or it reflects folding of a
+ * CollateExpr. It's unsafe to send to the remote unless
+ * it's used in a non-collation-sensitive context.
+ */
+ collation = c->constcollid;
+ if (collation == InvalidOid ||
+ collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
+ }
+ break;
+ case T_Param:
+ {
+ Param *p = (Param *) node;
- /*
- * If it's a MULTIEXPR Param, punt. We can't tell from here
- * whether the referenced sublink/subplan contains any remote
- * Vars; if it does, handling that is too complicated to
- * consider supporting at present. Fortunately, MULTIEXPR
- * Params are not reduced to plain PARAM_EXEC until the end of
- * planning, so we can easily detect this case. (Normal
- * PARAM_EXEC Params are safe to ship because their values
- * come from somewhere else in the plan tree; but a MULTIEXPR
- * references a sub-select elsewhere in the same targetlist,
- * so we'd be on the hook to evaluate it somehow if we wanted
- * to handle such cases as direct foreign updates.)
- */
- if (p->paramkind == PARAM_MULTIEXPR)
- return false;
+ /*
+ * If it's a MULTIEXPR Param, punt. We can't tell from
+ * here whether the referenced sublink/subplan contains
+ * any remote Vars; if it does, handling that is too
+ * complicated to consider supporting at present.
+ * Fortunately, MULTIEXPR Params are not reduced to plain
+ * PARAM_EXEC until the end of planning, so we can easily
+ * detect this case. (Normal PARAM_EXEC Params are safe
+ * to ship because their values come from somewhere else
+ * in the plan tree; but a MULTIEXPR references a
+ * sub-select elsewhere in the same targetlist, so we'd be
+ * on the hook to evaluate it somehow if we wanted to
+ * handle such cases as direct foreign updates.)
+ */
+ if (p->paramkind == PARAM_MULTIEXPR)
+ return false;
- /*
- * Collation rule is same as for Consts and non-foreign Vars.
- */
- collation = p->paramcollid;
- if (collation == InvalidOid ||
- collation == DEFAULT_COLLATION_OID)
- state = FDW_COLLATE_NONE;
- else
- state = FDW_COLLATE_UNSAFE;
- }
- break;
- case T_SubscriptingRef:
- {
- SubscriptingRef *sr = (SubscriptingRef *) node;
+ /*
+ * Collation rule is same as for Consts and non-foreign
+ * Vars.
+ */
+ collation = p->paramcollid;
+ if (collation == InvalidOid ||
+ collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
+ }
+ break;
+ case T_SubscriptingRef:
+ {
+ SubscriptingRef *sr = (SubscriptingRef *) node;
- /* Assignment should not be in restrictions. */
- if (sr->refassgnexpr != NULL)
- return false;
+ /* Assignment should not be in restrictions. */
+ if (sr->refassgnexpr != NULL)
+ return false;
- /*
- * Recurse into the remaining subexpressions. The container
- * subscripts will not affect collation of the SubscriptingRef
- * result, so do those first and reset inner_cxt afterwards.
- */
- if (!foreign_expr_walker((Node *) sr->refupperindexpr,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
- inner_cxt.collation = InvalidOid;
- inner_cxt.state = FDW_COLLATE_NONE;
- if (!foreign_expr_walker((Node *) sr->reflowerindexpr,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
- inner_cxt.collation = InvalidOid;
- inner_cxt.state = FDW_COLLATE_NONE;
- if (!foreign_expr_walker((Node *) sr->refexpr,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
+ /*
+ * Recurse into the remaining subexpressions. The
+ * container subscripts will not affect collation of the
+ * SubscriptingRef result, so do those first and reset
+ * inner_cxt afterwards.
+ */
+ if (!foreign_expr_walker((Node *) sr->refupperindexpr,
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
+ inner_cxt.collation = InvalidOid;
+ inner_cxt.state = FDW_COLLATE_NONE;
+ if (!foreign_expr_walker((Node *) sr->reflowerindexpr,
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
+ inner_cxt.collation = InvalidOid;
+ inner_cxt.state = FDW_COLLATE_NONE;
+ if (!foreign_expr_walker((Node *) sr->refexpr,
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
- /*
- * Container subscripting typically yields same collation as
- * refexpr's, but in case it doesn't, use same logic as for
- * function nodes.
- */
- collation = sr->refcollid;
- if (collation == InvalidOid)
- state = FDW_COLLATE_NONE;
- else if (inner_cxt.state == FDW_COLLATE_SAFE &&
- collation == inner_cxt.collation)
- state = FDW_COLLATE_SAFE;
- else if (collation == DEFAULT_COLLATION_OID)
- state = FDW_COLLATE_NONE;
- else
- state = FDW_COLLATE_UNSAFE;
- }
- break;
- case T_FuncExpr:
- {
- FuncExpr *fe = (FuncExpr *) node;
+ /*
+ * Container subscripting typically yields same collation
+ * as refexpr's, but in case it doesn't, use same logic as
+ * for function nodes.
+ */
+ collation = sr->refcollid;
+ if (collation == InvalidOid)
+ state = FDW_COLLATE_NONE;
+ else if (inner_cxt.state == FDW_COLLATE_SAFE &&
+ collation == inner_cxt.collation)
+ state = FDW_COLLATE_SAFE;
+ else if (collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
+ }
+ break;
+ case T_FuncExpr:
+ {
+ FuncExpr *fe = (FuncExpr *) node;
- /*
- * If function used by the expression is not shippable, it
- * can't be sent to remote because it might have incompatible
- * semantics on remote side.
- */
- if (!is_shippable(fe->funcid, ProcedureRelationId, fpinfo))
- return false;
+ /*
+ * If function used by the expression is not shippable, it
+ * can't be sent to remote because it might have
+ * incompatible semantics on remote side.
+ */
+ if (!is_shippable(fe->funcid, ProcedureRelationId, fpinfo))
+ return false;
- /*
- * Recurse to input subexpressions.
- */
- if (!foreign_expr_walker((Node *) fe->args,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
+ /*
+ * Recurse to input subexpressions.
+ */
+ if (!foreign_expr_walker((Node *) fe->args,
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
- /*
- * If function's input collation is not derived from a foreign
- * Var, it can't be sent to remote.
- */
- if (fe->inputcollid == InvalidOid)
- /* OK, inputs are all noncollatable */ ;
- else if (inner_cxt.state != FDW_COLLATE_SAFE ||
- fe->inputcollid != inner_cxt.collation)
- return false;
+ /*
+ * If function's input collation is not derived from a
+ * foreign Var, it can't be sent to remote.
+ */
+ if (fe->inputcollid == InvalidOid)
+ /* OK, inputs are all noncollatable */ ;
+ else if (inner_cxt.state != FDW_COLLATE_SAFE ||
+ fe->inputcollid != inner_cxt.collation)
+ return false;
- /*
- * Detect whether node is introducing a collation not derived
- * from a foreign Var. (If so, we just mark it unsafe for now
- * rather than immediately returning false, since the parent
- * node might not care.)
- */
- collation = fe->funccollid;
- if (collation == InvalidOid)
- state = FDW_COLLATE_NONE;
- else if (inner_cxt.state == FDW_COLLATE_SAFE &&
- collation == inner_cxt.collation)
- state = FDW_COLLATE_SAFE;
- else if (collation == DEFAULT_COLLATION_OID)
- state = FDW_COLLATE_NONE;
- else
- state = FDW_COLLATE_UNSAFE;
- }
- break;
- case T_OpExpr:
- case T_DistinctExpr: /* struct-equivalent to OpExpr */
- {
- OpExpr *oe = (OpExpr *) node;
+ /*
+ * Detect whether node is introducing a collation not
+ * derived from a foreign Var. (If so, we just mark it
+ * unsafe for now rather than immediately returning false,
+ * since the parent node might not care.)
+ */
+ collation = fe->funccollid;
+ if (collation == InvalidOid)
+ state = FDW_COLLATE_NONE;
+ else if (inner_cxt.state == FDW_COLLATE_SAFE &&
+ collation == inner_cxt.collation)
+ state = FDW_COLLATE_SAFE;
+ else if (collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
+ }
+ break;
+ case T_OpExpr:
+ case T_DistinctExpr: /* struct-equivalent to OpExpr */
+ {
+ OpExpr *oe = (OpExpr *) node;
- /*
- * Similarly, only shippable operators can be sent to remote.
- * (If the operator is shippable, we assume its underlying
- * function is too.)
- */
- if (!is_shippable(oe->opno, OperatorRelationId, fpinfo))
- return false;
+ /*
+ * Similarly, only shippable operators can be sent to
+ * remote. (If the operator is shippable, we assume its
+ * underlying function is too.)
+ */
+ if (!is_shippable(oe->opno, OperatorRelationId, fpinfo))
+ return false;
- /*
- * Recurse to input subexpressions.
- */
- if (!foreign_expr_walker((Node *) oe->args,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
+ /*
+ * Recurse to input subexpressions.
+ */
+ if (!foreign_expr_walker((Node *) oe->args,
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
- /*
- * If operator's input collation is not derived from a foreign
- * Var, it can't be sent to remote.
- */
- if (oe->inputcollid == InvalidOid)
- /* OK, inputs are all noncollatable */ ;
- else if (inner_cxt.state != FDW_COLLATE_SAFE ||
- oe->inputcollid != inner_cxt.collation)
- return false;
-
- /* Result-collation handling is same as for functions */
- collation = oe->opcollid;
- if (collation == InvalidOid)
- state = FDW_COLLATE_NONE;
- else if (inner_cxt.state == FDW_COLLATE_SAFE &&
- collation == inner_cxt.collation)
- state = FDW_COLLATE_SAFE;
- else if (collation == DEFAULT_COLLATION_OID)
- state = FDW_COLLATE_NONE;
- else
- state = FDW_COLLATE_UNSAFE;
- }
- break;
- case T_ScalarArrayOpExpr:
- {
- ScalarArrayOpExpr *oe = (ScalarArrayOpExpr *) node;
+ /*
+ * If operator's input collation is not derived from a
+ * foreign Var, it can't be sent to remote.
+ */
+ if (oe->inputcollid == InvalidOid)
+ /* OK, inputs are all noncollatable */ ;
+ else if (inner_cxt.state != FDW_COLLATE_SAFE ||
+ oe->inputcollid != inner_cxt.collation)
+ return false;
- /*
- * Again, only shippable operators can be sent to remote.
- */
- if (!is_shippable(oe->opno, OperatorRelationId, fpinfo))
- return false;
+ /* Result-collation handling is same as for functions */
+ collation = oe->opcollid;
+ if (collation == InvalidOid)
+ state = FDW_COLLATE_NONE;
+ else if (inner_cxt.state == FDW_COLLATE_SAFE &&
+ collation == inner_cxt.collation)
+ state = FDW_COLLATE_SAFE;
+ else if (collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
+ }
+ break;
+ case T_ScalarArrayOpExpr:
+ {
+ ScalarArrayOpExpr *oe = (ScalarArrayOpExpr *) node;
- /*
- * Recurse to input subexpressions.
- */
- if (!foreign_expr_walker((Node *) oe->args,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
+ /*
+ * Again, only shippable operators can be sent to remote.
+ */
+ if (!is_shippable(oe->opno, OperatorRelationId, fpinfo))
+ return false;
- /*
- * If operator's input collation is not derived from a foreign
- * Var, it can't be sent to remote.
- */
- if (oe->inputcollid == InvalidOid)
- /* OK, inputs are all noncollatable */ ;
- else if (inner_cxt.state != FDW_COLLATE_SAFE ||
- oe->inputcollid != inner_cxt.collation)
- return false;
-
- /* Output is always boolean and so noncollatable. */
- collation = InvalidOid;
- state = FDW_COLLATE_NONE;
- }
- break;
- case T_RelabelType:
- {
- RelabelType *r = (RelabelType *) node;
+ /*
+ * Recurse to input subexpressions.
+ */
+ if (!foreign_expr_walker((Node *) oe->args,
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
- /*
- * Recurse to input subexpression.
- */
- if (!foreign_expr_walker((Node *) r->arg,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
+ /*
+ * If operator's input collation is not derived from a
+ * foreign Var, it can't be sent to remote.
+ */
+ if (oe->inputcollid == InvalidOid)
+ /* OK, inputs are all noncollatable */ ;
+ else if (inner_cxt.state != FDW_COLLATE_SAFE ||
+ oe->inputcollid != inner_cxt.collation)
+ return false;
- /*
- * RelabelType must not introduce a collation not derived from
- * an input foreign Var (same logic as for a real function).
- */
- collation = r->resultcollid;
- if (collation == InvalidOid)
- state = FDW_COLLATE_NONE;
- else if (inner_cxt.state == FDW_COLLATE_SAFE &&
- collation == inner_cxt.collation)
- state = FDW_COLLATE_SAFE;
- else if (collation == DEFAULT_COLLATION_OID)
+ /* Output is always boolean and so noncollatable. */
+ collation = InvalidOid;
state = FDW_COLLATE_NONE;
- else
- state = FDW_COLLATE_UNSAFE;
- }
- break;
- case T_BoolExpr:
- {
- BoolExpr *b = (BoolExpr *) node;
-
- /*
- * Recurse to input subexpressions.
- */
- if (!foreign_expr_walker((Node *) b->args,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
+ }
+ break;
+ case T_RelabelType:
+ {
+ RelabelType *r = (RelabelType *) node;
- /* Output is always boolean and so noncollatable. */
- collation = InvalidOid;
- state = FDW_COLLATE_NONE;
- }
- break;
- case T_NullTest:
- {
- NullTest *nt = (NullTest *) node;
+ /*
+ * Recurse to input subexpression.
+ */
+ if (!foreign_expr_walker((Node *) r->arg,
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
- /*
- * Recurse to input subexpressions.
- */
- if (!foreign_expr_walker((Node *) nt->arg,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
+ /*
+ * RelabelType must not introduce a collation not derived
+ * from an input foreign Var (same logic as for a real
+ * function).
+ */
+ collation = r->resultcollid;
+ if (collation == InvalidOid)
+ state = FDW_COLLATE_NONE;
+ else if (inner_cxt.state == FDW_COLLATE_SAFE &&
+ collation == inner_cxt.collation)
+ state = FDW_COLLATE_SAFE;
+ else if (collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
+ }
+ break;
+ case T_BoolExpr:
+ {
+ BoolExpr *b = (BoolExpr *) node;
- /* Output is always boolean and so noncollatable. */
- collation = InvalidOid;
- state = FDW_COLLATE_NONE;
- }
- break;
- case T_CaseExpr:
- {
- CaseExpr *ce = (CaseExpr *) node;
- foreign_loc_cxt arg_cxt;
- foreign_loc_cxt tmp_cxt;
- ListCell *lc;
+ /*
+ * Recurse to input subexpressions.
+ */
+ if (!foreign_expr_walker((Node *) b->args,
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
- /*
- * Recurse to CASE's arg expression, if any. Its collation
- * has to be saved aside for use while examining CaseTestExprs
- * within the WHEN expressions.
- */
- arg_cxt.collation = InvalidOid;
- arg_cxt.state = FDW_COLLATE_NONE;
- if (ce->arg)
+ /* Output is always boolean and so noncollatable. */
+ collation = InvalidOid;
+ state = FDW_COLLATE_NONE;
+ }
+ break;
+ case T_NullTest:
{
- if (!foreign_expr_walker((Node *) ce->arg,
- glob_cxt, &arg_cxt, case_arg_cxt))
+ NullTest *nt = (NullTest *) node;
+
+ /*
+ * Recurse to input subexpressions.
+ */
+ if (!foreign_expr_walker((Node *) nt->arg,
+ glob_cxt, &inner_cxt, case_arg_cxt))
return false;
- }
- /* Examine the CaseWhen subexpressions. */
- foreach(lc, ce->args)
+ /* Output is always boolean and so noncollatable. */
+ collation = InvalidOid;
+ state = FDW_COLLATE_NONE;
+ }
+ break;
+ case T_CaseExpr:
{
- CaseWhen *cw = lfirst_node(CaseWhen, lc);
+ CaseExpr *ce = (CaseExpr *) node;
+ foreign_loc_cxt arg_cxt;
+ foreign_loc_cxt tmp_cxt;
+ ListCell *lc;
+ /*
+ * Recurse to CASE's arg expression, if any. Its
+ * collation has to be saved aside for use while examining
+ * CaseTestExprs within the WHEN expressions.
+ */
+ arg_cxt.collation = InvalidOid;
+ arg_cxt.state = FDW_COLLATE_NONE;
if (ce->arg)
{
+ if (!foreign_expr_walker((Node *) ce->arg,
+ glob_cxt, &arg_cxt, case_arg_cxt))
+ return false;
+ }
+
+ /* Examine the CaseWhen subexpressions. */
+ foreach(lc, ce->args)
+ {
+ CaseWhen *cw = lfirst_node(CaseWhen, lc);
+
+ if (ce->arg)
+ {
+ /*
+ * In a CASE-with-arg, the parser should have
+ * produced WHEN clauses of the form "CaseTestExpr
+ * = RHS", possibly with an implicit coercion
+ * inserted above the CaseTestExpr. However in an
+ * expression that's been through the optimizer,
+ * the WHEN clause could be almost anything (since
+ * the equality operator could have been expanded
+ * into an inline function). In such cases forbid
+ * pushdown, because deparseCaseExpr can't handle
+ * it.
+ */
+ Node *whenExpr = (Node *) cw->expr;
+ List *opArgs;
+
+ if (!IsA(whenExpr, OpExpr))
+ return false;
+
+ opArgs = ((OpExpr *) whenExpr)->args;
+ if (list_length(opArgs) != 2 ||
+ !IsA(strip_implicit_coercions(linitial(opArgs)),
+ CaseTestExpr))
+ return false;
+ }
+
/*
- * In a CASE-with-arg, the parser should have produced
- * WHEN clauses of the form "CaseTestExpr = RHS",
- * possibly with an implicit coercion inserted above
- * the CaseTestExpr. However in an expression that's
- * been through the optimizer, the WHEN clause could
- * be almost anything (since the equality operator
- * could have been expanded into an inline function).
- * In such cases forbid pushdown, because
- * deparseCaseExpr can't handle it.
+ * Recurse to WHEN expression, passing down the arg
+ * info. Its collation doesn't affect the result
+ * (really, it should be boolean and thus not have a
+ * collation).
*/
- Node *whenExpr = (Node *) cw->expr;
- List *opArgs;
-
- if (!IsA(whenExpr, OpExpr))
+ tmp_cxt.collation = InvalidOid;
+ tmp_cxt.state = FDW_COLLATE_NONE;
+ if (!foreign_expr_walker((Node *) cw->expr,
+ glob_cxt, &tmp_cxt, &arg_cxt))
return false;
- opArgs = ((OpExpr *) whenExpr)->args;
- if (list_length(opArgs) != 2 ||
- !IsA(strip_implicit_coercions(linitial(opArgs)),
- CaseTestExpr))
+ /* Recurse to THEN expression. */
+ if (!foreign_expr_walker((Node *) cw->result,
+ glob_cxt, &inner_cxt, case_arg_cxt))
return false;
}
- /*
- * Recurse to WHEN expression, passing down the arg info.
- * Its collation doesn't affect the result (really, it
- * should be boolean and thus not have a collation).
- */
- tmp_cxt.collation = InvalidOid;
- tmp_cxt.state = FDW_COLLATE_NONE;
- if (!foreign_expr_walker((Node *) cw->expr,
- glob_cxt, &tmp_cxt, &arg_cxt))
- return false;
-
- /* Recurse to THEN expression. */
- if (!foreign_expr_walker((Node *) cw->result,
+ /* Recurse to ELSE expression. */
+ if (!foreign_expr_walker((Node *) ce->defresult,
glob_cxt, &inner_cxt, case_arg_cxt))
return false;
+
+ /*
+ * Detect whether node is introducing a collation not
+ * derived from a foreign Var. (If so, we just mark it
+ * unsafe for now rather than immediately returning false,
+ * since the parent node might not care.) This is the
+ * same as for function nodes, except that the input
+ * collation is derived from only the THEN and ELSE
+ * subexpressions.
+ */
+ collation = ce->casecollid;
+ if (collation == InvalidOid)
+ state = FDW_COLLATE_NONE;
+ else if (inner_cxt.state == FDW_COLLATE_SAFE &&
+ collation == inner_cxt.collation)
+ state = FDW_COLLATE_SAFE;
+ else if (collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
}
+ break;
+ case T_CaseTestExpr:
+ {
+ CaseTestExpr *c = (CaseTestExpr *) node;
- /* Recurse to ELSE expression. */
- if (!foreign_expr_walker((Node *) ce->defresult,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
+ /* Punt if we seem not to be inside a CASE arg WHEN. */
+ if (!case_arg_cxt)
+ return false;
- /*
- * Detect whether node is introducing a collation not derived
- * from a foreign Var. (If so, we just mark it unsafe for now
- * rather than immediately returning false, since the parent
- * node might not care.) This is the same as for function
- * nodes, except that the input collation is derived from only
- * the THEN and ELSE subexpressions.
- */
- collation = ce->casecollid;
- if (collation == InvalidOid)
- state = FDW_COLLATE_NONE;
- else if (inner_cxt.state == FDW_COLLATE_SAFE &&
- collation == inner_cxt.collation)
- state = FDW_COLLATE_SAFE;
- else if (collation == DEFAULT_COLLATION_OID)
- state = FDW_COLLATE_NONE;
- else
- state = FDW_COLLATE_UNSAFE;
- }
- break;
- case T_CaseTestExpr:
- {
- CaseTestExpr *c = (CaseTestExpr *) node;
+ /*
+ * Otherwise, any nondefault collation attached to the
+ * CaseTestExpr node must be derived from foreign Var(s)
+ * in the CASE arg.
+ */
+ collation = c->collation;
+ if (collation == InvalidOid)
+ state = FDW_COLLATE_NONE;
+ else if (case_arg_cxt->state == FDW_COLLATE_SAFE &&
+ collation == case_arg_cxt->collation)
+ state = FDW_COLLATE_SAFE;
+ else if (collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
+ }
+ break;
+ case T_ArrayExpr:
+ {
+ ArrayExpr *a = (ArrayExpr *) node;
- /* Punt if we seem not to be inside a CASE arg WHEN. */
- if (!case_arg_cxt)
- return false;
+ /*
+ * Recurse to input subexpressions.
+ */
+ if (!foreign_expr_walker((Node *) a->elements,
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
- /*
- * Otherwise, any nondefault collation attached to the
- * CaseTestExpr node must be derived from foreign Var(s) in
- * the CASE arg.
- */
- collation = c->collation;
- if (collation == InvalidOid)
- state = FDW_COLLATE_NONE;
- else if (case_arg_cxt->state == FDW_COLLATE_SAFE &&
- collation == case_arg_cxt->collation)
- state = FDW_COLLATE_SAFE;
- else if (collation == DEFAULT_COLLATION_OID)
- state = FDW_COLLATE_NONE;
- else
- state = FDW_COLLATE_UNSAFE;
- }
- break;
- case T_ArrayExpr:
- {
- ArrayExpr *a = (ArrayExpr *) node;
+ /*
+ * ArrayExpr must not introduce a collation not derived
+ * from an input foreign Var (same logic as for a
+ * function).
+ */
+ collation = a->array_collid;
+ if (collation == InvalidOid)
+ state = FDW_COLLATE_NONE;
+ else if (inner_cxt.state == FDW_COLLATE_SAFE &&
+ collation == inner_cxt.collation)
+ state = FDW_COLLATE_SAFE;
+ else if (collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
+ }
+ break;
+ case T_List:
+ {
+ List *l = (List *) node;
+ ListCell *lc;
- /*
- * Recurse to input subexpressions.
- */
- if (!foreign_expr_walker((Node *) a->elements,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
+ /*
+ * Recurse to component subexpressions.
+ */
+ foreach(lc, l)
+ {
+ if (!foreign_expr_walker((Node *) lfirst(lc),
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
+ }
- /*
- * ArrayExpr must not introduce a collation not derived from
- * an input foreign Var (same logic as for a function).
- */
- collation = a->array_collid;
- if (collation == InvalidOid)
- state = FDW_COLLATE_NONE;
- else if (inner_cxt.state == FDW_COLLATE_SAFE &&
- collation == inner_cxt.collation)
- state = FDW_COLLATE_SAFE;
- else if (collation == DEFAULT_COLLATION_OID)
- state = FDW_COLLATE_NONE;
- else
- state = FDW_COLLATE_UNSAFE;
- }
- break;
- case T_List:
- {
- List *l = (List *) node;
- ListCell *lc;
+ /*
+ * When processing a list, collation state just bubbles up
+ * from the list elements.
+ */
+ collation = inner_cxt.collation;
+ state = inner_cxt.state;
- /*
- * Recurse to component subexpressions.
- */
- foreach(lc, l)
+ /* Don't apply exprType() to the list. */
+ check_type = false;
+ }
+ break;
+ case T_Aggref:
{
- if (!foreign_expr_walker((Node *) lfirst(lc),
- glob_cxt, &inner_cxt, case_arg_cxt))
+ Aggref *agg = (Aggref *) node;
+ ListCell *lc;
+
+ /* Not safe to pushdown when not in grouping context */
+ if (!IS_UPPER_REL(glob_cxt->foreignrel))
return false;
- }
- /*
- * When processing a list, collation state just bubbles up
- * from the list elements.
- */
- collation = inner_cxt.collation;
- state = inner_cxt.state;
+ /* Only non-split aggregates are pushable. */
+ if (agg->aggsplit != AGGSPLIT_SIMPLE)
+ return false;
- /* Don't apply exprType() to the list. */
- check_type = false;
- }
- break;
- case T_Aggref:
- {
- Aggref *agg = (Aggref *) node;
- ListCell *lc;
+ /* As usual, it must be shippable. */
+ if (!is_shippable(agg->aggfnoid, ProcedureRelationId, fpinfo))
+ return false;
- /* Not safe to pushdown when not in grouping context */
- if (!IS_UPPER_REL(glob_cxt->foreignrel))
- return false;
+ /*
+ * Recurse to input args. aggdirectargs, aggorder and
+ * aggdistinct are all present in args, so no need to
+ * check their shippability explicitly.
+ */
+ foreach(lc, agg->args)
+ {
+ Node *n = (Node *) lfirst(lc);
- /* Only non-split aggregates are pushable. */
- if (agg->aggsplit != AGGSPLIT_SIMPLE)
- return false;
+ /* If TargetEntry, extract the expression from it */
+ if (IsA(n, TargetEntry))
+ {
+ TargetEntry *tle = (TargetEntry *) n;
- /* As usual, it must be shippable. */
- if (!is_shippable(agg->aggfnoid, ProcedureRelationId, fpinfo))
- return false;
+ n = (Node *) tle->expr;
+ }
- /*
- * Recurse to input args. aggdirectargs, aggorder and
- * aggdistinct are all present in args, so no need to check
- * their shippability explicitly.
- */
- foreach(lc, agg->args)
- {
- Node *n = (Node *) lfirst(lc);
+ if (!foreign_expr_walker(n,
+ glob_cxt, &inner_cxt, case_arg_cxt))
+ return false;
+ }
- /* If TargetEntry, extract the expression from it */
- if (IsA(n, TargetEntry))
+ /*
+ * For aggorder elements, check whether the sort operator,
+ * if specified, is shippable or not.
+ */
+ if (agg->aggorder)
{
- TargetEntry *tle = (TargetEntry *) n;
-
- n = (Node *) tle->expr;
+ ListCell *lc;
+
+ foreach(lc, agg->aggorder)
+ {
+ SortGroupClause *srt = (SortGroupClause *) lfirst(lc);
+ Oid sortcoltype;
+ TypeCacheEntry *typentry;
+ TargetEntry *tle;
+
+ tle = get_sortgroupref_tle(srt->tleSortGroupRef,
+ agg->args);
+ sortcoltype = exprType((Node *) tle->expr);
+ typentry = lookup_type_cache(sortcoltype,
+ TYPECACHE_LT_OPR | TYPECACHE_GT_OPR);
+
+ /*
+ * Check shippability of non-default sort
+ * operator.
+ */
+ if (srt->sortop != typentry->lt_opr &&
+ srt->sortop != typentry->gt_opr &&
+ !is_shippable(srt->sortop, OperatorRelationId,
+ fpinfo))
+ return false;
+ }
}
- if (!foreign_expr_walker(n,
+ /* Check aggregate filter */
+ if (!foreign_expr_walker((Node *) agg->aggfilter,
glob_cxt, &inner_cxt, case_arg_cxt))
return false;
- }
- /*
- * For aggorder elements, check whether the sort operator, if
- * specified, is shippable or not.
- */
- if (agg->aggorder)
- {
- ListCell *lc;
+ /*
+ * If aggregate's input collation is not derived from a
+ * foreign Var, it can't be sent to remote.
+ */
+ if (agg->inputcollid == InvalidOid)
+ /* OK, inputs are all noncollatable */ ;
+ else if (inner_cxt.state != FDW_COLLATE_SAFE ||
+ agg->inputcollid != inner_cxt.collation)
+ return false;
- foreach(lc, agg->aggorder)
- {
- SortGroupClause *srt = (SortGroupClause *) lfirst(lc);
- Oid sortcoltype;
- TypeCacheEntry *typentry;
- TargetEntry *tle;
-
- tle = get_sortgroupref_tle(srt->tleSortGroupRef,
- agg->args);
- sortcoltype = exprType((Node *) tle->expr);
- typentry = lookup_type_cache(sortcoltype,
- TYPECACHE_LT_OPR | TYPECACHE_GT_OPR);
- /* Check shippability of non-default sort operator. */
- if (srt->sortop != typentry->lt_opr &&
- srt->sortop != typentry->gt_opr &&
- !is_shippable(srt->sortop, OperatorRelationId,
- fpinfo))
- return false;
- }
+ /*
+ * Detect whether node is introducing a collation not
+ * derived from a foreign Var. (If so, we just mark it
+ * unsafe for now rather than immediately returning false,
+ * since the parent node might not care.)
+ */
+ collation = agg->aggcollid;
+ if (collation == InvalidOid)
+ state = FDW_COLLATE_NONE;
+ else if (inner_cxt.state == FDW_COLLATE_SAFE &&
+ collation == inner_cxt.collation)
+ state = FDW_COLLATE_SAFE;
+ else if (collation == DEFAULT_COLLATION_OID)
+ state = FDW_COLLATE_NONE;
+ else
+ state = FDW_COLLATE_UNSAFE;
}
-
- /* Check aggregate filter */
- if (!foreign_expr_walker((Node *) agg->aggfilter,
- glob_cxt, &inner_cxt, case_arg_cxt))
- return false;
-
- /*
- * If aggregate's input collation is not derived from a
- * foreign Var, it can't be sent to remote.
- */
- if (agg->inputcollid == InvalidOid)
- /* OK, inputs are all noncollatable */ ;
- else if (inner_cxt.state != FDW_COLLATE_SAFE ||
- agg->inputcollid != inner_cxt.collation)
- return false;
+ break;
+ default:
/*
- * Detect whether node is introducing a collation not derived
- * from a foreign Var. (If so, we just mark it unsafe for now
- * rather than immediately returning false, since the parent
- * node might not care.)
+ * If it's anything else, assume it's unsafe. This list can
+ * be expanded later, but don't forget to add deparse support
+ * below.
*/
- collation = agg->aggcollid;
- if (collation == InvalidOid)
- state = FDW_COLLATE_NONE;
- else if (inner_cxt.state == FDW_COLLATE_SAFE &&
- collation == inner_cxt.collation)
- state = FDW_COLLATE_SAFE;
- else if (collation == DEFAULT_COLLATION_OID)
- state = FDW_COLLATE_NONE;
- else
- state = FDW_COLLATE_UNSAFE;
- }
- break;
- default:
-
- /*
- * If it's anything else, assume it's unsafe. This list can be
- * expanded later, but don't forget to add deparse support below.
- */
- return false;
+ return false;
+ }
}
/*
@@ -1009,6 +1041,15 @@ is_foreign_param(PlannerInfo *root,
if (expr == NULL)
return false;
+ if (is_stable_expr((Node *) expr))
+ {
+ /*
+ * We can get here only if is_foreign_expr(expr) returned true, so
+ * it's a supported stable expression.
+ */
+ return true;
+ }
+
switch (nodeTag(expr))
{
case T_Var:
@@ -2575,6 +2616,12 @@ deparseExpr(Expr *node, deparse_expr_cxt *context)
if (node == NULL)
return;
+ if (is_stable_expr((Node *) node))
+ {
+ deparseStableExpr(node, context);
+ return;
+ }
+
switch (nodeTag(node))
{
case T_Var:
@@ -3192,6 +3239,45 @@ deparseRelabelType(RelabelType *node, deparse_expr_cxt *context)
node->resulttypmod));
}
+/*
+ * Deparse a stable expression as param
+ */
+static void
+deparseStableExpr(Expr *node, deparse_expr_cxt *context)
+{
+ int32 type = exprType((Node *) node);
+ int32 typmod = exprTypmod((Node *) node);
+
+ Assert(type != InvalidOid);
+
+ /* Treat like a Param */
+ if (context->params_list)
+ {
+ int pindex = 0;
+ ListCell *lc;
+
+ /* find its index in params_list */
+ foreach(lc, *context->params_list)
+ {
+ pindex++;
+ if (equal(node, (Node *) lfirst(lc)))
+ break;
+ }
+ if (lc == NULL)
+ {
+ /* not in list, so add it */
+ pindex++;
+ *context->params_list = lappend(*context->params_list, node);
+ }
+
+ printRemoteParam(pindex, type, typmod, context);
+ }
+ else
+ {
+ printRemotePlaceholder(type, typmod, context);
+ }
+}
+
/*
* Deparse a BoolExpr node.
*/
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 7d6f7d9e3df..5defc2d3758 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -1073,6 +1073,93 @@ SELECT * FROM ft1 t1 WHERE t1.c1 === t1.c2 order by t1.c2 limit 1;
1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
(1 row)
+-- Test stable expressions pushdown
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c4 FROM ft2 WHERE c4 = current_timestamp;
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft2
+ Output: c1, c2, c4
+ Remote SQL: SELECT "C 1", c2, c4 FROM "S 1"."T 1" WHERE ((c4 = $1::timestamp with time zone))
+(3 rows)
+
+SELECT c1,c2,c4 FROM ft2 WHERE c4 = current_timestamp;
+ c1 | c2 | c4
+----+----+----
+(0 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c5 FROM ft2 WHERE c5 > localtimestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft2
+ Output: c1, c2, c5
+ Remote SQL: SELECT "C 1", c2, c5 FROM "S 1"."T 1" WHERE (("C 1" > 990)) AND ((c5 > $1::timestamp without time zone)) ORDER BY "C 1" ASC NULLS LAST
+(3 rows)
+
+SELECT c1,c2,c5 FROM ft2 WHERE c5 > localtimestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+ c1 | c2 | c5
+------+----+--------------------------
+ 991 | 1 | Thu Apr 02 00:00:00 1970
+ 992 | 2 | Fri Apr 03 00:00:00 1970
+ 993 | 3 | Sat Apr 04 00:00:00 1970
+ 994 | 4 | Sun Apr 05 00:00:00 1970
+ 995 | 5 | Mon Apr 06 00:00:00 1970
+ 996 | 6 | Tue Apr 07 00:00:00 1970
+ 997 | 7 | Wed Apr 08 00:00:00 1970
+ 998 | 8 | Thu Apr 09 00:00:00 1970
+ 999 | 9 | Fri Apr 10 00:00:00 1970
+ 1000 | 0 | Thu Jan 01 00:00:00 1970
+(10 rows)
+
+-- also shippable as whole expression is executed locally
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c4 FROM ft2 WHERE c4 > current_timestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft2
+ Output: c1, c2, c4
+ Remote SQL: SELECT "C 1", c2, c4 FROM "S 1"."T 1" WHERE (("C 1" > 990)) AND ((c4 > $1::timestamp with time zone)) ORDER BY "C 1" ASC NULLS LAST
+(3 rows)
+
+SELECT c1,c2,c4 FROM ft2 WHERE c4 > current_timestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+ c1 | c2 | c4
+------+----+------------------------------
+ 991 | 1 | Thu Apr 02 00:00:00 1970 PST
+ 992 | 2 | Fri Apr 03 00:00:00 1970 PST
+ 993 | 3 | Sat Apr 04 00:00:00 1970 PST
+ 994 | 4 | Sun Apr 05 00:00:00 1970 PST
+ 995 | 5 | Mon Apr 06 00:00:00 1970 PST
+ 996 | 6 | Tue Apr 07 00:00:00 1970 PST
+ 997 | 7 | Wed Apr 08 00:00:00 1970 PST
+ 998 | 8 | Thu Apr 09 00:00:00 1970 PST
+ 999 | 9 | Fri Apr 10 00:00:00 1970 PST
+ 1000 | 0 | Thu Jan 01 00:00:00 1970 PST
+(10 rows)
+
+-- direct modify
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE ft2 SET c4 = current_timestamp WHERE c4 < current_timestamp;
+ QUERY PLAN
+--------------------------------------------------------------------------------------------------------------------------
+ Update on public.ft2
+ -> Foreign Update on public.ft2
+ Remote SQL: UPDATE "S 1"."T 1" SET c4 = $1::timestamp with time zone WHERE ((c4 < $1::timestamp with time zone))
+(3 rows)
+
+-- check that we don't try to push down parameter in group by
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT sum(c1), current_timestamp FROM ft2 WHERE c1 > 990 GROUP BY current_timestamp;
+ QUERY PLAN
+-------------------------------------------------------------------------
+ GroupAggregate
+ Output: sum(c1), (CURRENT_TIMESTAMP)
+ Group Key: CURRENT_TIMESTAMP
+ -> Foreign Scan on public.ft2
+ Output: CURRENT_TIMESTAMP, c1
+ Remote SQL: SELECT "C 1" FROM "S 1"."T 1" WHERE (("C 1" > 990))
+(6 rows)
+
-- Test CASE pushdown
EXPLAIN (VERBOSE, COSTS OFF)
SELECT c1,c2,c3 FROM ft2 WHERE CASE WHEN c1 > 990 THEN c1 END < 1000 ORDER BY c1;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 09a3f5e23cb..6aa3a20f20c 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4834,12 +4834,9 @@ prepare_query_params(PlanState *node,
}
/*
- * Prepare remote-parameter expressions for evaluation. (Note: in
- * practice, we expect that all these expressions will be just Params, so
- * we could possibly do something more efficient than using the full
- * expression-eval machinery for this. But probably there would be little
- * benefit, and it'd require postgres_fdw to know more than is desirable
- * about Param evaluation.)
+ * Prepare remote-parameter expressions for evaluation. (Note: we cannot
+ * expect that all these expressions will be just Params, so we should use
+ * the full expression-eval machinery for this).
*/
*param_exprs = ExecInitExprList(fdw_exprs, node);
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 8ae79e97e44..acb36394fc6 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -231,5 +231,6 @@ extern const char *get_jointype_name(JoinType jointype);
/* in shippable.c */
extern bool is_builtin(Oid objectId);
extern bool is_shippable(Oid objectId, Oid classId, PgFdwRelationInfo *fpinfo);
+extern bool is_stable_expr(Node *node);
#endif /* POSTGRES_FDW_H */
diff --git a/contrib/postgres_fdw/shippable.c b/contrib/postgres_fdw/shippable.c
index 8e759da00d5..5173d750035 100644
--- a/contrib/postgres_fdw/shippable.c
+++ b/contrib/postgres_fdw/shippable.c
@@ -25,9 +25,14 @@
#include "access/transam.h"
#include "catalog/dependency.h"
+#include "catalog/pg_proc.h"
+#include "nodes/nodeFuncs.h"
+#include "optimizer/clauses.h"
+#include "optimizer/optimizer.h"
#include "postgres_fdw.h"
#include "utils/hsearch.h"
#include "utils/inval.h"
+#include "utils/lsyscache.h"
#include "utils/syscache.h"
/* Hash table for caching the results of shippability lookups */
@@ -52,6 +57,7 @@ typedef struct
bool shippable;
} ShippableCacheEntry;
+static bool contain_params_walker(Node *node, bool *context);
/*
* Flush cache entries when pg_foreign_server is updated.
@@ -209,3 +215,71 @@ is_shippable(Oid objectId, Oid classId, PgFdwRelationInfo *fpinfo)
return entry->shippable;
}
+
+/*
+ * contain_params
+ * Does this node has params?
+ */
+static bool
+contain_params(Node *node)
+{
+ bool contains = false;
+
+ contain_params_walker(node, (void *) &contains);
+
+ return contains;
+}
+
+static bool
+contain_params_walker(Node *node, bool *context)
+{
+ if (node == NULL)
+ return false;
+ if (IsA(node, Param))
+ {
+ *context = true;
+
+ return false;
+ }
+ return expression_tree_walker(node, contain_params_walker,
+ (void *) context);
+}
+
+/*
+ * Check if expression is stable
+ */
+bool
+is_stable_expr(Node *node)
+{
+ if (node == NULL)
+ return false;
+
+ /* No need to turn on 'ship stable expression' machinery in these cases */
+ if (IsA(node, Const) || IsA(node, List))
+ return false;
+
+ /* Expression shouldn't reference any table */
+ if (contain_var_clause(node))
+ return false;
+
+ if (contain_volatile_functions(node))
+ return false;
+
+ if (contain_subplans(node))
+ return false;
+
+ if (contain_params(node))
+ return false;
+
+ if (contain_agg_clause(node))
+ return false;
+
+ if (contain_mutable_functions(node))
+ {
+ /* These are not volatile functions, so they are stable */
+ if (exprType(node) != InvalidOid)
+ return true;
+ }
+
+ return false;
+}
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 9eb673e3693..de084bc4ece 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -413,6 +413,28 @@ EXPLAIN (VERBOSE, COSTS OFF)
SELECT * FROM ft1 t1 WHERE t1.c1 === t1.c2 order by t1.c2 limit 1;
SELECT * FROM ft1 t1 WHERE t1.c1 === t1.c2 order by t1.c2 limit 1;
+-- Test stable expressions pushdown
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c4 FROM ft2 WHERE c4 = current_timestamp;
+SELECT c1,c2,c4 FROM ft2 WHERE c4 = current_timestamp;
+
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c5 FROM ft2 WHERE c5 > localtimestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+SELECT c1,c2,c5 FROM ft2 WHERE c5 > localtimestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+
+-- also shippable as whole expression is executed locally
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT c1,c2,c4 FROM ft2 WHERE c4 > current_timestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+SELECT c1,c2,c4 FROM ft2 WHERE c4 > current_timestamp - interval '1000 years' AND c1 > 990 ORDER BY c1;
+
+-- direct modify
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE ft2 SET c4 = current_timestamp WHERE c4 < current_timestamp;
+
+-- check that we don't try to push down parameter in group by
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT sum(c1), current_timestamp FROM ft2 WHERE c1 > 990 GROUP BY current_timestamp;
+
-- Test CASE pushdown
EXPLAIN (VERBOSE, COSTS OFF)
SELECT c1,c2,c3 FROM ft2 WHERE CASE WHEN c1 > 990 THEN c1 END < 1000 ORDER BY c1;
--
2.25.1
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Push down time-related SQLValue functions to foreign server
2022-01-17 23:08 Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 08:52 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 16:53 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 19:02 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 19:11 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 20:01 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-20 13:42 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
@ 2022-01-21 19:57 ` Tom Lane <[email protected]>
2022-08-02 20:59 ` Re: Push down time-related SQLValue functions to foreign server Jacob Champion <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Tom Lane @ 2022-01-21 19:57 UTC (permalink / raw)
To: Alexander Pyhalov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers
Alexander Pyhalov <[email protected]> writes:
> So far I have the following prototype. It seems to be working, but I
> think it can be enhanced.
> At least, some sort of caching seems to be necessary for
> is_stable_expr().
Yeah, from a performance standpoint this seems pretty horrid ---
it's probably exponential in the size of the expressions considered
because of the repeated recursions.
The approach I had in mind was to extend the responsibility of
foreign_expr_walker to make it deduce the classification of
an expression in a bottom-up fashion. Also, as I noted before,
we don't want to re-deduce that in a later deparse pass, so
maybe we should just go ahead and deparse during foreign_expr_walker.
Not sure about that part. It sounds expensive; but recording the
classification results in a way we could re-use later doesn't seem
too cheap either.
BTW, the patch is just about unreadable as-is. It would have been
better to not have re-indented the bulk of foreign_expr_walker,
leaving that for some later pgindent pass. But that may be moot,
because I don't think we can tolerate the double-recursion approach
you took here.
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Push down time-related SQLValue functions to foreign server
2022-01-17 23:08 Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 08:52 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 16:53 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 19:02 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 19:11 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 20:01 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-20 13:42 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-21 19:57 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
@ 2022-08-02 20:59 ` Jacob Champion <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Jacob Champion @ 2022-08-02 20:59 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Alexander Pyhalov <[email protected]>; +Cc: Ranier Vilela <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers
This entry has been waiting on author input for a while (our current
threshold is roughly two weeks), so I've marked it Returned with
Feedback.
Once you think the patchset is ready for review again, you (or any
interested party) can resurrect the patch entry by visiting
https://commitfest.postgresql.org/38/3289/
and changing the status to "Needs Review", and then changing the
status again to "Move to next CF". (Don't forget the second step;
hopefully we will have streamlined this in the near future!)
Thanks,
--Jacob
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2022-08-02 20:59 UTC | newest]
Thread overview: 10+ 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]>
2022-01-17 23:08 Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 08:52 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 16:53 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 19:02 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-18 19:11 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-18 20:01 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-01-20 13:42 ` Re: Push down time-related SQLValue functions to foreign server Alexander Pyhalov <[email protected]>
2022-01-21 19:57 ` Re: Push down time-related SQLValue functions to foreign server Tom Lane <[email protected]>
2022-08-02 20:59 ` Re: Push down time-related SQLValue functions to foreign server Jacob Champion <[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