aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--contrib/postgres_fdw/deparse.c49
-rw-r--r--contrib/postgres_fdw/expected/postgres_fdw.out40
-rw-r--r--contrib/postgres_fdw/postgres_fdw.c32
-rw-r--r--contrib/postgres_fdw/postgres_fdw.h3
-rw-r--r--contrib/postgres_fdw/sql/postgres_fdw.sql10
5 files changed, 129 insertions, 5 deletions
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 079406f4f33..c4e331166f9 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -842,6 +842,55 @@ foreign_expr_walker(Node *node,
}
/*
+ * Returns true if given expr is something we'd have to send the value of
+ * to the foreign server.
+ *
+ * This should return true when the expression is a shippable node that
+ * deparseExpr would add to context->params_list. Note that we don't care
+ * if the expression *contains* such a node, only whether one appears at top
+ * level. We need this to detect cases where setrefs.c would recognize a
+ * false match between an fdw_exprs item (which came from the params_list)
+ * and an entry in fdw_scan_tlist (which we're considering putting the given
+ * expression into).
+ */
+bool
+is_foreign_param(PlannerInfo *root,
+ RelOptInfo *baserel,
+ Expr *expr)
+{
+ if (expr == NULL)
+ return false;
+
+ switch (nodeTag(expr))
+ {
+ case T_Var:
+ {
+ /* It would have to be sent unless it's a foreign Var */
+ Var *var = (Var *) expr;
+ PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) (baserel->fdw_private);
+ Relids relids;
+
+ if (IS_UPPER_REL(baserel))
+ relids = fpinfo->outerrel->relids;
+ else
+ relids = baserel->relids;
+
+ if (bms_is_member(var->varno, relids) && var->varlevelsup == 0)
+ return false; /* foreign Var, so not a param */
+ else
+ return true; /* it'd have to be a param */
+ break;
+ }
+ case T_Param:
+ /* Params always have to be sent to the foreign server */
+ return true;
+ default:
+ break;
+ }
+ return false;
+}
+
+/*
* Convert type OID + typmod info into a type name we can ship to the remote
* server. Someplace else had better have verified that this type name is
* expected to be known on the remote end.
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 48ea67aaeca..e034b030f15 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -2707,6 +2707,46 @@ select sum(c1) from ft1 group by c2 having avg(c1 * (random() <= 1)::int) > 100
Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1"
(10 rows)
+-- Remote aggregate in combination with a local Param (for the output
+-- of an initplan) can be trouble, per bug #15781
+explain (verbose, costs off)
+select exists(select 1 from pg_enum), sum(c1) from ft1;
+ QUERY PLAN
+--------------------------------------------------
+ Foreign Scan
+ Output: $0, (sum(ft1.c1))
+ Relations: Aggregate on (public.ft1)
+ Remote SQL: SELECT sum("C 1") FROM "S 1"."T 1"
+ InitPlan 1 (returns $0)
+ -> Seq Scan on pg_catalog.pg_enum
+(6 rows)
+
+select exists(select 1 from pg_enum), sum(c1) from ft1;
+ exists | sum
+--------+--------
+ t | 500500
+(1 row)
+
+explain (verbose, costs off)
+select exists(select 1 from pg_enum), sum(c1) from ft1 group by 1;
+ QUERY PLAN
+---------------------------------------------------
+ GroupAggregate
+ Output: ($0), sum(ft1.c1)
+ Group Key: $0
+ InitPlan 1 (returns $0)
+ -> Seq Scan on pg_catalog.pg_enum
+ -> Foreign Scan on public.ft1
+ Output: $0, ft1.c1
+ Remote SQL: SELECT "C 1" FROM "S 1"."T 1"
+(8 rows)
+
+select exists(select 1 from pg_enum), sum(c1) from ft1 group by 1;
+ exists | sum
+--------+--------
+ t | 500500
+(1 row)
+
-- Testing ORDER BY, DISTINCT, FILTER, Ordered-sets and VARIADIC within aggregates
-- ORDER BY within aggregate, same column used to order
explain (verbose, costs off)
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 794363c184c..6c9e320af99 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -5486,7 +5486,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) grouped_rel->fdw_private;
PathTarget *grouping_target = grouped_rel->reltarget;
PgFdwRelationInfo *ofpinfo;
- List *aggvars;
ListCell *lc;
int i;
List *tlist = NIL;
@@ -5512,6 +5511,15 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
* server. All GROUP BY expressions will be part of the grouping target
* and thus there is no need to search for them separately. Add grouping
* expressions into target list which will be passed to foreign server.
+ *
+ * A tricky fine point is that we must not put any expression into the
+ * target list that is just a foreign param (that is, something that
+ * deparse.c would conclude has to be sent to the foreign server). If we
+ * do, the expression will also appear in the fdw_exprs list of the plan
+ * node, and setrefs.c will get confused and decide that the fdw_exprs
+ * entry is actually a reference to the fdw_scan_tlist entry, resulting in
+ * a broken plan. Somewhat oddly, it's OK if the expression contains such
+ * a node, as long as it's not at top level; then no match is possible.
*/
i = 0;
foreach(lc, grouping_target->exprs)
@@ -5533,6 +5541,13 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
return false;
/*
+ * If it would be a foreign param, we can't put it into the tlist,
+ * so we have to fail.
+ */
+ if (is_foreign_param(root, grouped_rel, expr))
+ return false;
+
+ /*
* Pushable, so add to tlist. We need to create a TLE for this
* expression and apply the sortgroupref to it. We cannot use
* add_to_flat_tlist() here because that avoids making duplicate
@@ -5547,9 +5562,11 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
else
{
/*
- * Non-grouping expression we need to compute. Is it shippable?
+ * Non-grouping expression we need to compute. Can we ship it
+ * as-is to the foreign server?
*/
- if (is_foreign_expr(root, grouped_rel, expr))
+ if (is_foreign_expr(root, grouped_rel, expr) &&
+ !is_foreign_param(root, grouped_rel, expr))
{
/* Yes, so add to tlist as-is; OK to suppress duplicates */
tlist = add_to_flat_tlist(tlist, list_make1(expr));
@@ -5557,12 +5574,16 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
else
{
/* Not pushable as a whole; extract its Vars and aggregates */
+ List *aggvars;
+
aggvars = pull_var_clause((Node *) expr,
PVC_INCLUDE_AGGREGATES);
/*
* If any aggregate expression is not shippable, then we
- * cannot push down aggregation to the foreign server.
+ * cannot push down aggregation to the foreign server. (We
+ * don't have to check is_foreign_param, since that certainly
+ * won't return true for any such expression.)
*/
if (!is_foreign_expr(root, grouped_rel, (Expr *) aggvars))
return false;
@@ -5649,7 +5670,8 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
* If aggregates within local conditions are not safe to push
* down, then we cannot push down the query. Vars are already
* part of GROUP BY clause which are checked above, so no need to
- * access them again here.
+ * access them again here. Again, we need not check
+ * is_foreign_param for a foreign aggregate.
*/
if (IsA(expr, Aggref))
{
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index b3829450675..3e4603d718e 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -146,6 +146,9 @@ extern void classifyConditions(PlannerInfo *root,
extern bool is_foreign_expr(PlannerInfo *root,
RelOptInfo *baserel,
Expr *expr);
+extern bool is_foreign_param(PlannerInfo *root,
+ RelOptInfo *baserel,
+ Expr *expr);
extern void deparseInsertSql(StringInfo buf, RangeTblEntry *rte,
Index rtindex, Relation rel,
List *targetAttrs, bool doNothing,
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 127cd30a922..73852f1ae69 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -685,6 +685,16 @@ select count(*) from (select c5, count(c1) from ft1 group by c5, sqrt(c2) having
explain (verbose, costs off)
select sum(c1) from ft1 group by c2 having avg(c1 * (random() <= 1)::int) > 100 order by 1;
+-- Remote aggregate in combination with a local Param (for the output
+-- of an initplan) can be trouble, per bug #15781
+explain (verbose, costs off)
+select exists(select 1 from pg_enum), sum(c1) from ft1;
+select exists(select 1 from pg_enum), sum(c1) from ft1;
+
+explain (verbose, costs off)
+select exists(select 1 from pg_enum), sum(c1) from ft1 group by 1;
+select exists(select 1 from pg_enum), sum(c1) from ft1 group by 1;
+
-- Testing ORDER BY, DISTINCT, FILTER, Ordered-sets and VARIADIC within aggregates
generated by cgit v1.2.3 (git 2.25.1) at 2025-05-20 18:10:34 +0000