pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #14890: Error grouping by same column twice using FDW
11+ messages / 4 participants
[nested] [flat]
* BUG #14890: Error grouping by same column twice using FDW
@ 2017-11-07 13:49 sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 11+ messages in thread
From: sean.johnston@edgeintelligence.com @ 2017-11-07 13:49 UTC (permalink / raw)
To: pgsql-bugs; +Cc: sean.johnston@edgeintelligence.com
The following bug has been logged on the website:
Bug reference: 14890
Logged by: Sean Johnston
Email address: sean.johnston@edgeintelligence.com
PostgreSQL version: 10.0
Operating system: Linux Ubuntu
Description:
Using user 'sean' with database 'sean':
create extension postgres_fdw;
create table tbl (col1 integer);
create server loop foreign data wrapper postgres_fdw options ( host
'localhost', dbname 'sean' );
create user mapping for sean server loop options ( user 'sean' );
create foreign table loop_tbl (col1 integer) server loop options (
table_name 'tbl' );
select col1, col1 from loop_tbl group by 1, 2;
psql:t.sql:7: ERROR: targetlist item has multiple sortgroupref labels
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #14890: Error grouping by same column twice using FDW
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
@ 2017-11-07 22:29 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-09 11:59 ` Re: BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2017-11-07 22:29 UTC (permalink / raw)
To: sean.johnston@edgeintelligence.com; +Cc: pgsql-bugs@postgresql.org, Robert Haas <robertmhaas@gmail.com>; Jeevan Chalke <jeevan.chalke@enterprisedb.com>; Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
sean.johnston@edgeintelligence.com writes:
> select col1, col1 from loop_tbl group by 1, 2;
> psql:t.sql:7: ERROR: targetlist item has multiple sortgroupref labels
This is the fault of commit 7012b132d ("Push down aggregates to remote
servers"), which imagined that postgres_fdw can use
apply_pathtarget_labeling_to_tlist() for situations well beyond that
function's limited abilities. I kinda suspect that foreign_grouping_ok()
has got more bugs than this with complicated sortgroupref situations, too;
its willingness to scribble on the sortgrouprefs doesn't look like a great
idea to me.
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #14890: Error grouping by same column twice using FDW
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-11-09 11:59 ` Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-10 12:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
0 siblings, 1 reply; 11+ messages in thread
From: Jeevan Chalke @ 2017-11-09 11:59 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs; Robert Haas <robertmhaas@gmail.com>; Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
On Wed, Nov 8, 2017 at 3:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> sean.johnston@edgeintelligence.com writes:
> > select col1, col1 from loop_tbl group by 1, 2;
> > psql:t.sql:7: ERROR: targetlist item has multiple sortgroupref labels
>
> This is the fault of commit 7012b132d ("Push down aggregates to remote
> servers"), which imagined that postgres_fdw can use
> apply_pathtarget_labeling_to_tlist() for situations well beyond that
> function's limited abilities. I kinda suspect that foreign_grouping_ok()
> has got more bugs than this with complicated sortgroupref situations, too;
> its willingness to scribble on the sortgrouprefs doesn't look like a great
> idea to me.
>
In the attached patch, I have removed apply_pathtarget_labeling_to_tlist()
call. Since we are anyways checking that whether the GROUP BY expression is
entirely present in a target list or not, we can safely create a tle for
that
GROUP BY expression and append that to the new targetlist by making sure
that
its sortgroupref is also transferred, eliminating the need for relabeling
afterwords.
Now with these changes i.e. as we have moved setting sortgroupref for tle
inside the if block itself, we don't have to reset the sortgroupref for any
ORDER BY expression. That code is removed. This also eliminated the need for
copying the pathtarget.
Please have a look at the changes and let me know if I missed any.
Thanks
>
> regards, tom lane
>
--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Attachments:
fix_multiple_sortgroupref_labels.patch (text/x-patch, 5.2K) download
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 4339bbf..69296fd 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -2648,6 +2648,23 @@ select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2
100 | 9 | 5 | 7.0
(10 rows)
+-- GROUP BY clause referring to same column multiple times
+explain (verbose, costs off)
+select c2, c2 from ft1 where c2 = 6 group by 1, 2;
+ QUERY PLAN
+-------------------------------------------------------------------------------
+ Foreign Scan
+ Output: c2, c2
+ Relations: Aggregate on (public.ft1)
+ Remote SQL: SELECT c2, c2 FROM "S 1"."T 1" WHERE ((c2 = 6)) GROUP BY c2, c2
+(4 rows)
+
+select c2, c2 from ft1 where c2 = 6 group by 1, 2;
+ c2 | c2
+----+----
+ 6 | 6
+(1 row)
+
-- Testing HAVING clause shippability
explain (verbose, costs off)
select c2, sum(c1) from ft2 group by c2 having avg(c1) < 500 and sum(c1) < 49800 order by c2;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index fb65e2e..0ad32a5 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4591,7 +4591,7 @@ static bool
foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
{
Query *query = root->parse;
- PathTarget *grouping_target;
+ PathTarget *grouping_target = root->upper_targets[UPPERREL_GROUP_AGG];
PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) grouped_rel->fdw_private;
PgFdwRelationInfo *ofpinfo;
List *aggvars;
@@ -4615,16 +4615,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
return false;
/*
- * The targetlist expected from this node and the targetlist pushed down
- * to the foreign server may be different. The latter requires
- * sortgrouprefs to be set to push down GROUP BY clause, but should not
- * have those arising from ORDER BY clause. These sortgrouprefs may be
- * different from those in the plan's targetlist. Use a copy of path
- * target to record the new sortgrouprefs.
- */
- grouping_target = copy_pathtarget(root->upper_targets[UPPERREL_GROUP_AGG]);
-
- /*
* Evaluate grouping targets and check whether they are safe to push down
* to the foreign side. All GROUP BY expressions will be part of the
* grouping target and thus there is no need to evaluate it separately.
@@ -4641,6 +4631,8 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
/* Check whether this expression is part of GROUP BY clause */
if (sgref && get_sortgroupref_clause_noerr(sgref, query->groupClause))
{
+ TargetEntry *tle;
+
/*
* If any of the GROUP BY expression is not shippable we can not
* push down aggregation to the foreign server.
@@ -4648,8 +4640,18 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
if (!is_foreign_expr(root, grouped_rel, expr))
return false;
- /* Pushable, add to tlist */
- tlist = add_to_flat_tlist(tlist, list_make1(expr));
+ /*
+ * Pushable, add to tlist. We need to create a tle for this
+ * expression and need to transfer the sortgroupref too. We cannot
+ * use add_to_flat_tlist() here as it avoids the duplicate entries
+ * in the targetlist but here we want those duplicate entries as
+ * there can be multiple GROUP BY expressions pointing to the same
+ * column at different positions.
+ */
+ tle = makeTargetEntry((Expr *) expr, list_length(tlist) + 1, NULL,
+ false);
+ tle->ressortgroupref = sgref;
+ tlist = lappend(tlist, tle);
}
else
{
@@ -4661,14 +4663,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
}
else
{
- /*
- * If we have sortgroupref set, then it means that we have an
- * ORDER BY entry pointing to this expression. Since we are
- * not pushing ORDER BY with GROUP BY, clear it.
- */
- if (sgref)
- grouping_target->sortgrouprefs[i] = 0;
-
/* Not matched exactly, pull the var with aggregates then */
aggvars = pull_var_clause((Node *) expr,
PVC_INCLUDE_AGGREGATES);
@@ -4771,9 +4765,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
}
}
- /* Transfer any sortgroupref data to the replacement tlist */
- apply_pathtarget_labeling_to_tlist(tlist, grouping_target);
-
/* Store generated targetlist */
fpinfo->grouped_tlist = tlist;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index ddfec79..84db0f9 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -636,6 +636,11 @@ explain (verbose, costs off)
select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2;
select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2;
+-- GROUP BY clause referring to same column multiple times
+explain (verbose, costs off)
+select c2, c2 from ft1 where c2 = 6 group by 1, 2;
+select c2, c2 from ft1 where c2 = 6 group by 1, 2;
+
-- Testing HAVING clause shippability
explain (verbose, costs off)
select c2, sum(c1) from ft2 group by c2 having avg(c1) < 500 and sum(c1) < 49800 order by c2;
* Re: BUG #14890: Error grouping by same column twice using FDW
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-09 11:59 ` Re: BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
@ 2017-11-10 12:44 ` Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2017-11-10 18:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-15 08:51 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
0 siblings, 2 replies; 11+ messages in thread
From: Ashutosh Bapat @ 2017-11-10 12:44 UTC (permalink / raw)
To: Jeevan Chalke <jeevan.chalke@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs; Robert Haas <robertmhaas@gmail.com>
On Thu, Nov 9, 2017 at 5:29 PM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:
>
>
> In the attached patch, I have removed apply_pathtarget_labeling_to_tlist()
> call. Since we are anyways checking that whether the GROUP BY expression is
> entirely present in a target list or not, we can safely create a tle for
> that
> GROUP BY expression and append that to the new targetlist by making sure
> that
> its sortgroupref is also transferred, eliminating the need for relabeling
> afterwords.
>
> Now with these changes i.e. as we have moved setting sortgroupref for tle
> inside the if block itself, we don't have to reset the sortgroupref for any
> ORDER BY expression. That code is removed. This also eliminated the need for
> copying the pathtarget.
Some review comments.
+ Remote SQL: SELECT c2, c2 FROM "S 1"."T 1" WHERE ((c2 = 6)) GROUP BY c2, c2
GROUP BY 1, 2 is changed to GROUP BY c2, c2 which is technically wrong. The
remote planner will think that both the GROUP BY entries refer to one of the
(possibly the first) entry in the SELECT clause. That's not what really it is.
This isn't a real problem, since we push only immutable expressions, but
nonetheless something to avoid if we can.
That brings another question, do we really bother to pass duplicate entries in
the GROUP BY clause as they are? Can we just pass a single one? The result will
be the same since both these entries are immutable. May be we should pass
repeated entries in GROUP BY clause just in case, we somehow manage to ship
non-immutable expressions in future. Thoughts?
- * have those arising from ORDER BY clause. These sortgrouprefs may be
- * different from those in the plan's targetlist.
May be we were not explaining this correctly earlier. The sortgrouprefs of
GROUP BY clause can not be different between those two tlists. The difference
is really the absence of ORDER BY entries. May be we should add some tests
where there some entries common between ORDER BY and GROUP BY.
+ /*
+ * Pushable, add to tlist. We need to create a tle for this
+ * expression and need to transfer the sortgroupref too. We cannot
+ * use add_to_flat_tlist() here as it avoids the duplicate entries
+ * in the targetlist but here we want those duplicate entries as
+ * there can be multiple GROUP BY expressions pointing to the same
+ * column at different positions.
+ */
+ tle = makeTargetEntry((Expr *) expr, list_length(tlist) + 1, NULL,
+ false);
+ tle->ressortgroupref = sgref;
+ tlist = lappend(tlist, tle);
May be we want to use add_flat_to_tlist() when sgref is not present to avoid
duplicate entries.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: BUG #14890: Error grouping by same column twice using FDW
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-09 11:59 ` Re: BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-10 12:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
@ 2017-11-10 18:44 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-15 09:06 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-17 12:09 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
1 sibling, 2 replies; 11+ messages in thread
From: Tom Lane @ 2017-11-10 18:44 UTC (permalink / raw)
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>; +Cc: Jeevan Chalke <jeevan.chalke@enterprisedb.com>; pgsql-bugs; Robert Haas <robertmhaas@gmail.com>
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> On Thu, Nov 9, 2017 at 5:29 PM, Jeevan Chalke
> <jeevan.chalke@enterprisedb.com> wrote:
>> + Remote SQL: SELECT c2, c2 FROM "S 1"."T 1" WHERE ((c2 = 6)) GROUP BY c2, c2
> GROUP BY 1, 2 is changed to GROUP BY c2, c2 which is technically wrong. The
> remote planner will think that both the GROUP BY entries refer to one of the
> (possibly the first) entry in the SELECT clause. That's not what really it is.
Yeah. I'm inclined to think that part of what needs to happen here is for
postgres_fdw to change over to always emitting GROUP BY column-number,
so that the grouping columns are clearly matched up with the tlist entries
it's considering, and the remote parser is certain to build
ressortgrouprefs that match what we thought was happening locally.
As you say, we can probably get away without that as long as we don't push
mutable grouping expressions ... but just because we think a grouping
expression is immutable at our end doesn't necessarily mean that it is at
the far end. Also, in view of the (as yet unfixed) bug discussed in
https://www.postgresql.org/message-id/flat/7dbdcf5c-b5a6-ef89-4958-da212fe10176@iki.fi
there's no hope of extending postgres_fdw to push GROUPING SETS correctly
unless it is able to distinguish textually-equal grouping columns.
> May be we were not explaining this correctly earlier. The sortgrouprefs of
> GROUP BY clause can not be different between those two tlists. The difference
> is really the absence of ORDER BY entries. May be we should add some tests
> where there some entries common between ORDER BY and GROUP BY.
As I alluded to upthread, I suspect that dropping ORDER BY markings from
the tlist is likely to break some cases (that is, the planner may expect
the output of the foreign scan to include those columns). If this isn't
fully exercised by the existing tests then we definitely need more tests.
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-09 11:59 ` Re: BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-10 12:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2017-11-10 18:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-11-15 09:06 ` Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-21 06:25 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
1 sibling, 1 reply; 11+ messages in thread
From: Jeevan Chalke @ 2017-11-15 09:06 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>; pgsql-bugs; Robert Haas <robertmhaas@gmail.com>
On Sat, Nov 11, 2017 at 12:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> > On Thu, Nov 9, 2017 at 5:29 PM, Jeevan Chalke
> > <jeevan.chalke@enterprisedb.com> wrote:
> >> + Remote SQL: SELECT c2, c2 FROM "S 1"."T 1" WHERE ((c2 = 6)) GROUP
> BY c2, c2
>
> > GROUP BY 1, 2 is changed to GROUP BY c2, c2 which is technically wrong.
> The
> > remote planner will think that both the GROUP BY entries refer to one of
> the
> > (possibly the first) entry in the SELECT clause. That's not what really
> it is.
>
> Yeah. I'm inclined to think that part of what needs to happen here is for
> postgres_fdw to change over to always emitting GROUP BY column-number,
> so that the grouping columns are clearly matched up with the tlist entries
> it's considering, and the remote parser is certain to build
> ressortgrouprefs that match what we thought was happening locally.
>
Understood.
Along with the last patch changes which does put duplicate entries if they
were referred in GROUP BY clause, in attached patch I have modified
deparsing
logic to emit GROUP BY column-number.
> As you say, we can probably get away without that as long as we don't push
> mutable grouping expressions ... but just because we think a grouping
> expression is immutable at our end doesn't necessarily mean that it is at
> the far end. Also, in view of the (as yet unfixed) bug discussed in
> https://www.postgresql.org/message-id/flat/7dbdcf5c-b5a6-
> ef89-4958-da212fe10176@iki.fi
> there's no hope of extending postgres_fdw to push GROUPING SETS correctly
> unless it is able to distinguish textually-equal grouping columns.
>
> > May be we were not explaining this correctly earlier. The sortgrouprefs
> of
> > GROUP BY clause can not be different between those two tlists. The
> difference
> > is really the absence of ORDER BY entries. May be we should add some
> tests
> > where there some entries common between ORDER BY and GROUP BY.
>
> As I alluded to upthread, I suspect that dropping ORDER BY markings from
> the tlist is likely to break some cases (that is, the planner may expect
> the output of the foreign scan to include those columns). If this isn't
> fully exercised by the existing tests then we definitely need more tests.
>
We do have test-coverage where ORDER BY and GROUP BY have common entries.
All ORDER BY expressions are part of the grouping targets and they must be
either plain vars or aggregate function. Plain vars are anyways part of the
GROUP BY expression otherwsie we will end-up in a error. And for aggregate
function, we do include those in a targetlist.
> regards, tom lane
>
Let me know if I missed any or wrongly interpreted anything over here.
Thanks
--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Attachments:
fix_multiple_sortgroupref_labels_v2.patch (text/x-patch, 44.9K) download
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 0876589..6ada941 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -178,7 +178,7 @@ static void appendGroupByClause(List *tlist, deparse_expr_cxt *context);
static void appendAggOrderBy(List *orderList, List *targetList,
deparse_expr_cxt *context);
static void appendFunctionName(Oid funcid, deparse_expr_cxt *context);
-static Node *deparseSortGroupClause(Index ref, List *tlist,
+static Node *deparseSortGroupClause(Index ref, List *tlist, bool force_colno,
deparse_expr_cxt *context);
/*
@@ -2853,7 +2853,7 @@ appendAggOrderBy(List *orderList, List *targetList, deparse_expr_cxt *context)
first = false;
sortexpr = deparseSortGroupClause(srt->tleSortGroupRef, targetList,
- context);
+ false, context);
sortcoltype = exprType(sortexpr);
/* See whether operator is default < or > for datatype */
typentry = lookup_type_cache(sortcoltype,
@@ -2960,7 +2960,7 @@ appendGroupByClause(List *tlist, deparse_expr_cxt *context)
appendStringInfoString(buf, ", ");
first = false;
- deparseSortGroupClause(grp->tleSortGroupRef, tlist, context);
+ deparseSortGroupClause(grp->tleSortGroupRef, tlist, true, context);
}
}
@@ -3047,7 +3047,8 @@ appendFunctionName(Oid funcid, deparse_expr_cxt *context)
* need not find it again.
*/
static Node *
-deparseSortGroupClause(Index ref, List *tlist, deparse_expr_cxt *context)
+deparseSortGroupClause(Index ref, List *tlist, bool force_colno,
+ deparse_expr_cxt *context)
{
StringInfo buf = context->buf;
TargetEntry *tle;
@@ -3056,7 +3057,13 @@ deparseSortGroupClause(Index ref, List *tlist, deparse_expr_cxt *context)
tle = get_sortgroupref_tle(ref, tlist);
expr = tle->expr;
- if (expr && IsA(expr, Const))
+ /* Use column-number form if requested by caller. */
+ if (force_colno)
+ {
+ Assert(!tle->resjunk);
+ appendStringInfo(buf, "%d", tle->resno);
+ }
+ else if (expr && IsA(expr, Const))
{
/*
* Force a typecast here so that we don't emit something like "GROUP
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 4339bbf..cbf1c7b 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -2462,8 +2462,8 @@ DROP ROLE regress_view_owner;
-- Simple aggregates
explain (verbose, costs off)
select count(c6), sum(c1), avg(c1), min(c2), max(c1), stddev(c2), sum(c1) * (random() <= 1)::int as sum2 from ft1 where c2 < 5 group by c2 order by 1, 2;
- QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------
Result
Output: (count(c6)), (sum(c1)), (avg(c1)), (min(c2)), (max(c1)), (stddev(c2)), ((sum(c1)) * ((random() <= '1'::double precision))::integer), c2
-> Sort
@@ -2472,7 +2472,7 @@ select count(c6), sum(c1), avg(c1), min(c2), max(c1), stddev(c2), sum(c1) * (ran
-> Foreign Scan
Output: (count(c6)), (sum(c1)), (avg(c1)), (min(c2)), (max(c1)), (stddev(c2)), c2
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT count(c6), sum("C 1"), avg("C 1"), min(c2), max("C 1"), stddev(c2), c2 FROM "S 1"."T 1" WHERE ((c2 < 5)) GROUP BY c2
+ Remote SQL: SELECT count(c6), sum("C 1"), avg("C 1"), min(c2), max("C 1"), stddev(c2), c2 FROM "S 1"."T 1" WHERE ((c2 < 5)) GROUP BY 7
(9 rows)
select count(c6), sum(c1), avg(c1), min(c2), max(c1), stddev(c2), sum(c1) * (random() <= 1)::int as sum2 from ft1 where c2 < 5 group by c2 order by 1, 2;
@@ -2531,15 +2531,15 @@ select sum(t1.c1), count(t2.c1) from ft1 t1 inner join ft2 t2 on (t1.c1 = t2.c1)
-- GROUP BY clause having expressions
explain (verbose, costs off)
select c2/2, sum(c2) * (c2/2) from ft1 group by c2/2 order by c2/2;
- QUERY PLAN
-------------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------
Sort
Output: ((c2 / 2)), ((sum(c2) * (c2 / 2)))
Sort Key: ((ft1.c2 / 2))
-> Foreign Scan
Output: ((c2 / 2)), ((sum(c2) * (c2 / 2)))
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT (c2 / 2), (sum(c2) * (c2 / 2)) FROM "S 1"."T 1" GROUP BY ((c2 / 2))
+ Remote SQL: SELECT (c2 / 2), (sum(c2) * (c2 / 2)) FROM "S 1"."T 1" GROUP BY 1
(7 rows)
select c2/2, sum(c2) * (c2/2) from ft1 group by c2/2 order by c2/2;
@@ -2555,8 +2555,8 @@ select c2/2, sum(c2) * (c2/2) from ft1 group by c2/2 order by c2/2;
-- Aggregates in subquery are pushed down.
explain (verbose, costs off)
select count(x.a), sum(x.a) from (select c2 a, sum(c1) b from ft1 group by c2, sqrt(c1) order by 1, 2) x;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------------
Aggregate
Output: count(ft1.c2), sum(ft1.c2)
-> Sort
@@ -2565,7 +2565,7 @@ select count(x.a), sum(x.a) from (select c2 a, sum(c1) b from ft1 group by c2, s
-> Foreign Scan
Output: ft1.c2, (sum(ft1.c1)), (sqrt((ft1.c1)::double precision))
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT c2, sum("C 1"), sqrt("C 1") FROM "S 1"."T 1" GROUP BY c2, (sqrt("C 1"))
+ Remote SQL: SELECT c2, sum("C 1"), sqrt("C 1") FROM "S 1"."T 1" GROUP BY 1, 3
(9 rows)
select count(x.a), sum(x.a) from (select c2 a, sum(c1) b from ft1 group by c2, sqrt(c1) order by 1, 2) x;
@@ -2585,7 +2585,7 @@ select c2 * (random() <= 1)::int as sum1, sum(c1) * c2 as sum2 from ft1 group by
-> Foreign Scan
Output: (c2 * ((random() <= '1'::double precision))::integer), ((sum(c1) * c2)), c2
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT (sum("C 1") * c2), c2 FROM "S 1"."T 1" GROUP BY c2
+ Remote SQL: SELECT (sum("C 1") * c2), c2 FROM "S 1"."T 1" GROUP BY 2
(7 rows)
select c2 * (random() <= 1)::int as sum1, sum(c1) * c2 as sum2 from ft1 group by c2 order by 1, 2;
@@ -2622,15 +2622,15 @@ select c2 * (random() <= 1)::int as c2 from ft2 group by c2 * (random() <= 1)::i
-- GROUP BY clause in various forms, cardinal, alias and constant expression
explain (verbose, costs off)
select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------
Sort
Output: (count(c2)), c2, 5, 7.0, 9
Sort Key: ft1.c2
-> Foreign Scan
Output: (count(c2)), c2, 5, 7.0, 9
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT count(c2), c2, 5, 7.0, 9 FROM "S 1"."T 1" GROUP BY c2, 5::integer, 9::integer
+ Remote SQL: SELECT count(c2), c2, 5, 7.0, 9 FROM "S 1"."T 1" GROUP BY 2, 3, 5
(7 rows)
select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2;
@@ -2648,18 +2648,41 @@ select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2
100 | 9 | 5 | 7.0
(10 rows)
+-- GROUP BY clause referring to same column multiple times
+-- Also, ORDER BY contains an aggregate function
+explain (verbose, costs off)
+select c2, c2 from ft1 where c2 > 6 group by 1, 2 order by sum(c1);
+ QUERY PLAN
+-----------------------------------------------------------------------------------------------
+ Sort
+ Output: c2, c2, (sum(c1))
+ Sort Key: (sum(ft1.c1))
+ -> Foreign Scan
+ Output: c2, c2, (sum(c1))
+ Relations: Aggregate on (public.ft1)
+ Remote SQL: SELECT c2, c2, sum("C 1") FROM "S 1"."T 1" WHERE ((c2 > 6)) GROUP BY 1, 2
+(7 rows)
+
+select c2, c2 from ft1 where c2 > 6 group by 1, 2 order by sum(c1);
+ c2 | c2
+----+----
+ 7 | 7
+ 8 | 8
+ 9 | 9
+(3 rows)
+
-- Testing HAVING clause shippability
explain (verbose, costs off)
select c2, sum(c1) from ft2 group by c2 having avg(c1) < 500 and sum(c1) < 49800 order by c2;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------------------------------
Sort
Output: c2, (sum(c1))
Sort Key: ft2.c2
-> Foreign Scan
Output: c2, (sum(c1))
Relations: Aggregate on (public.ft2)
- Remote SQL: SELECT c2, sum("C 1") FROM "S 1"."T 1" GROUP BY c2 HAVING ((avg("C 1") < 500::numeric)) AND ((sum("C 1") < 49800))
+ Remote SQL: SELECT c2, sum("C 1") FROM "S 1"."T 1" GROUP BY 1 HAVING ((avg("C 1") < 500::numeric)) AND ((sum("C 1") < 49800))
(7 rows)
select c2, sum(c1) from ft2 group by c2 having avg(c1) < 500 and sum(c1) < 49800 order by c2;
@@ -2672,15 +2695,15 @@ select c2, sum(c1) from ft2 group by c2 having avg(c1) < 500 and sum(c1) < 49800
-- Unshippable HAVING clause will be evaluated locally, and other qual in HAVING clause is pushed down
explain (verbose, costs off)
select count(*) from (select c5, count(c1) from ft1 group by c5, sqrt(c2) having (avg(c1) / avg(c1)) * random() <= 1 and avg(c1) < 500) x;
- QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------------------------------
Aggregate
Output: count(*)
-> Foreign Scan
Output: ft1.c5, NULL::bigint, (sqrt((ft1.c2)::double precision))
Filter: (((((avg(ft1.c1)) / (avg(ft1.c1))))::double precision * random()) <= '1'::double precision)
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT c5, NULL::bigint, sqrt(c2), avg("C 1") FROM "S 1"."T 1" GROUP BY c5, (sqrt(c2)) HAVING ((avg("C 1") < 500::numeric))
+ Remote SQL: SELECT c5, NULL::bigint, sqrt(c2), avg("C 1") FROM "S 1"."T 1" GROUP BY 1, 3 HAVING ((avg("C 1") < 500::numeric))
(7 rows)
select count(*) from (select c5, count(c1) from ft1 group by c5, sqrt(c2) having (avg(c1) / avg(c1)) * random() <= 1 and avg(c1) < 500) x;
@@ -2710,15 +2733,15 @@ select sum(c1) from ft1 group by c2 having avg(c1 * (random() <= 1)::int) > 100
-- ORDER BY within aggregate, same column used to order
explain (verbose, costs off)
select array_agg(c1 order by c1) from ft1 where c1 < 100 group by c2 order by 1;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------------------------
Sort
Output: (array_agg(c1 ORDER BY c1)), c2
Sort Key: (array_agg(ft1.c1 ORDER BY ft1.c1))
-> Foreign Scan
Output: (array_agg(c1 ORDER BY c1)), c2
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT array_agg("C 1" ORDER BY "C 1" ASC NULLS LAST), c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) GROUP BY c2
+ Remote SQL: SELECT array_agg("C 1" ORDER BY "C 1" ASC NULLS LAST), c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) GROUP BY 2
(7 rows)
select array_agg(c1 order by c1) from ft1 where c1 < 100 group by c2 order by 1;
@@ -2756,15 +2779,15 @@ select array_agg(c5 order by c1 desc) from ft2 where c2 = 6 and c1 < 50;
-- DISTINCT within aggregate
explain (verbose, costs off)
select array_agg(distinct (t1.c1)%5) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Sort
Output: (array_agg(DISTINCT (t1.c1 % 5))), ((t2.c1 % 3))
Sort Key: (array_agg(DISTINCT (t1.c1 % 5)))
-> Foreign Scan
Output: (array_agg(DISTINCT (t1.c1 % 5))), ((t2.c1 % 3))
Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
- Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5)), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY ((r2.c1 % 3))
+ Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5)), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY 2
(7 rows)
select array_agg(distinct (t1.c1)%5) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
@@ -2777,15 +2800,15 @@ select array_agg(distinct (t1.c1)%5) from ft4 t1 full join ft5 t2 on (t1.c1 = t2
-- DISTINCT combined with ORDER BY within aggregate
explain (verbose, costs off)
select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
- QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Sort
Output: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5))), ((t2.c1 % 3))
Sort Key: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5)))
-> Foreign Scan
Output: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5))), ((t2.c1 % 3))
Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
- Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5) ORDER BY ((r1.c1 % 5)) ASC NULLS LAST), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY ((r2.c1 % 3))
+ Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5) ORDER BY ((r1.c1 % 5)) ASC NULLS LAST), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY 2
(7 rows)
select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
@@ -2797,15 +2820,15 @@ select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5) from ft4 t1 full join ft
explain (verbose, costs off)
select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5 desc nulls last) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
- QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Sort
Output: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5) DESC NULLS LAST)), ((t2.c1 % 3))
Sort Key: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5) DESC NULLS LAST))
-> Foreign Scan
Output: (array_agg(DISTINCT (t1.c1 % 5) ORDER BY (t1.c1 % 5) DESC NULLS LAST)), ((t2.c1 % 3))
Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
- Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5) ORDER BY ((r1.c1 % 5)) DESC NULLS LAST), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY ((r2.c1 % 3))
+ Remote SQL: SELECT array_agg(DISTINCT (r1.c1 % 5) ORDER BY ((r1.c1 % 5)) DESC NULLS LAST), (r2.c1 % 3) FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) WHERE (((r1.c1 < 20) OR ((r1.c1 IS NULL) AND (r2.c1 < 5)))) GROUP BY 2
(7 rows)
select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5 desc nulls last) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) where t1.c1 < 20 or (t1.c1 is null and t2.c1 < 5) group by (t2.c1)%3 order by 1;
@@ -2818,15 +2841,15 @@ select array_agg(distinct (t1.c1)%5 order by (t1.c1)%5 desc nulls last) from ft4
-- FILTER within aggregate
explain (verbose, costs off)
select sum(c1) filter (where c1 < 100 and c2 > 5) from ft1 group by c2 order by 1 nulls last;
- QUERY PLAN
---------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------------------------
Sort
Output: (sum(c1) FILTER (WHERE ((c1 < 100) AND (c2 > 5)))), c2
Sort Key: (sum(ft1.c1) FILTER (WHERE ((ft1.c1 < 100) AND (ft1.c2 > 5))))
-> Foreign Scan
Output: (sum(c1) FILTER (WHERE ((c1 < 100) AND (c2 > 5)))), c2
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT sum("C 1") FILTER (WHERE (("C 1" < 100) AND (c2 > 5))), c2 FROM "S 1"."T 1" GROUP BY c2
+ Remote SQL: SELECT sum("C 1") FILTER (WHERE (("C 1" < 100) AND (c2 > 5))), c2 FROM "S 1"."T 1" GROUP BY 2
(7 rows)
select sum(c1) filter (where c1 < 100 and c2 > 5) from ft1 group by c2 order by 1 nulls last;
@@ -2847,12 +2870,12 @@ select sum(c1) filter (where c1 < 100 and c2 > 5) from ft1 group by c2 order by
-- DISTINCT, ORDER BY and FILTER within aggregate
explain (verbose, costs off)
select sum(c1%3), sum(distinct c1%3 order by c1%3) filter (where c1%3 < 2), c2 from ft1 where c2 = 6 group by c2;
- QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Foreign Scan
Output: (sum((c1 % 3))), (sum(DISTINCT (c1 % 3) ORDER BY (c1 % 3)) FILTER (WHERE ((c1 % 3) < 2))), c2
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT sum(("C 1" % 3)), sum(DISTINCT ("C 1" % 3) ORDER BY (("C 1" % 3)) ASC NULLS LAST) FILTER (WHERE (("C 1" % 3) < 2)), c2 FROM "S 1"."T 1" WHERE ((c2 = 6)) GROUP BY c2
+ Remote SQL: SELECT sum(("C 1" % 3)), sum(DISTINCT ("C 1" % 3) ORDER BY (("C 1" % 3)) ASC NULLS LAST) FILTER (WHERE (("C 1" % 3) < 2)), c2 FROM "S 1"."T 1" WHERE ((c2 = 6)) GROUP BY 3
(4 rows)
select sum(c1%3), sum(distinct c1%3 order by c1%3) filter (where c1%3 < 2), c2 from ft1 where c2 = 6 group by c2;
@@ -2948,15 +2971,15 @@ select sum(c2) filter (where c2 in (select c2 from ft1 where c2 < 5)) from ft1;
-- Ordered-sets within aggregate
explain (verbose, costs off)
select c2, rank('10'::varchar) within group (order by c6), percentile_cont(c2/10::numeric) within group (order by c1) from ft1 where c2 < 10 group by c2 having percentile_cont(c2/10::numeric) within group (order by c1) < 500 order by c2;
- QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Sort
Output: c2, (rank('10'::character varying) WITHIN GROUP (ORDER BY c6)), (percentile_cont((((c2)::numeric / '10'::numeric))::double precision) WITHIN GROUP (ORDER BY ((c1)::double precision)))
Sort Key: ft1.c2
-> Foreign Scan
Output: c2, (rank('10'::character varying) WITHIN GROUP (ORDER BY c6)), (percentile_cont((((c2)::numeric / '10'::numeric))::double precision) WITHIN GROUP (ORDER BY ((c1)::double precision)))
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT c2, rank('10'::character varying) WITHIN GROUP (ORDER BY c6 ASC NULLS LAST), percentile_cont((c2 / 10::numeric)) WITHIN GROUP (ORDER BY ("C 1") ASC NULLS LAST) FROM "S 1"."T 1" WHERE ((c2 < 10)) GROUP BY c2 HAVING ((percentile_cont((c2 / 10::numeric)) WITHIN GROUP (ORDER BY ("C 1") ASC NULLS LAST) < 500::double precision))
+ Remote SQL: SELECT c2, rank('10'::character varying) WITHIN GROUP (ORDER BY c6 ASC NULLS LAST), percentile_cont((c2 / 10::numeric)) WITHIN GROUP (ORDER BY ("C 1") ASC NULLS LAST) FROM "S 1"."T 1" WHERE ((c2 < 10)) GROUP BY 1 HAVING ((percentile_cont((c2 / 10::numeric)) WITHIN GROUP (ORDER BY ("C 1") ASC NULLS LAST) < 500::double precision))
(7 rows)
select c2, rank('10'::varchar) within group (order by c6), percentile_cont(c2/10::numeric) within group (order by c1) from ft1 where c2 < 10 group by c2 having percentile_cont(c2/10::numeric) within group (order by c1) < 500 order by c2;
@@ -2972,12 +2995,12 @@ select c2, rank('10'::varchar) within group (order by c6), percentile_cont(c2/10
-- Using multiple arguments within aggregates
explain (verbose, costs off)
select c1, rank(c1, c2) within group (order by c1, c2) from ft1 group by c1, c2 having c1 = 6 order by 1;
- QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------
Foreign Scan
Output: c1, (rank(c1, c2) WITHIN GROUP (ORDER BY c1, c2)), c2
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT "C 1", rank("C 1", c2) WITHIN GROUP (ORDER BY "C 1" ASC NULLS LAST, c2 ASC NULLS LAST), c2 FROM "S 1"."T 1" WHERE (("C 1" = 6)) GROUP BY "C 1", c2
+ Remote SQL: SELECT "C 1", rank("C 1", c2) WITHIN GROUP (ORDER BY "C 1" ASC NULLS LAST, c2 ASC NULLS LAST), c2 FROM "S 1"."T 1" WHERE (("C 1" = 6)) GROUP BY 1, 3
(4 rows)
select c1, rank(c1, c2) within group (order by c1, c2) from ft1 group by c1, c2 having c1 = 6 order by 1;
@@ -3015,15 +3038,15 @@ alter server loopback options (set extensions 'postgres_fdw');
-- Now aggregate will be pushed. Aggregate will display VARIADIC argument.
explain (verbose, costs off)
select c2, least_agg(c1) from ft1 where c2 < 100 group by c2 order by c2;
- QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------------------------------------------------------------
Sort
Output: c2, (least_agg(VARIADIC ARRAY[c1]))
Sort Key: ft1.c2
-> Foreign Scan
Output: c2, (least_agg(VARIADIC ARRAY[c1]))
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT c2, public.least_agg(VARIADIC ARRAY["C 1"]) FROM "S 1"."T 1" WHERE ((c2 < 100)) GROUP BY c2
+ Remote SQL: SELECT c2, public.least_agg(VARIADIC ARRAY["C 1"]) FROM "S 1"."T 1" WHERE ((c2 < 100)) GROUP BY 1
(7 rows)
select c2, least_agg(c1) from ft1 where c2 < 100 group by c2 order by c2;
@@ -3115,12 +3138,12 @@ alter server loopback options (set extensions 'postgres_fdw');
-- Now this will be pushed as sort operator is part of the extension.
explain (verbose, costs off)
select array_agg(c1 order by c1 using operator(public.<^)) from ft2 where c2 = 6 and c1 < 100 group by c2;
- QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------------------------------------------------------------------------------------------------
Foreign Scan
Output: (array_agg(c1 ORDER BY c1 USING <^ NULLS LAST)), c2
Relations: Aggregate on (public.ft2)
- Remote SQL: SELECT array_agg("C 1" ORDER BY "C 1" USING OPERATOR(public.<^) NULLS LAST), c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) AND ((c2 = 6)) GROUP BY c2
+ Remote SQL: SELECT array_agg("C 1" ORDER BY "C 1" USING OPERATOR(public.<^) NULLS LAST), c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) AND ((c2 = 6)) GROUP BY 2
(4 rows)
select array_agg(c1 order by c1 using operator(public.<^)) from ft2 where c2 = 6 and c1 < 100 group by c2;
@@ -3179,8 +3202,8 @@ select count(t1.c3) from ft1 t1, ft1 t2 where t1.c1 = postgres_fdw_abs(t1.c2);
-- Subquery in FROM clause having aggregate
explain (verbose, costs off)
select count(*), x.b from ft1, (select c2 a, sum(c1) b from ft1 group by c2) x where ft1.c2 = x.a group by x.b order by 1, 2;
- QUERY PLAN
-------------------------------------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------------------------------------
Sort
Output: (count(*)), x.b
Sort Key: (count(*)), x.b
@@ -3201,7 +3224,7 @@ select count(*), x.b from ft1, (select c2 a, sum(c1) b from ft1 group by c2) x w
-> Foreign Scan
Output: ft1_1.c2, (sum(ft1_1.c1))
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT c2, sum("C 1") FROM "S 1"."T 1" GROUP BY c2
+ Remote SQL: SELECT c2, sum("C 1") FROM "S 1"."T 1" GROUP BY 1
(21 rows)
select count(*), x.b from ft1, (select c2 a, sum(c1) b from ft1 group by c2) x where ft1.c2 = x.a group by x.b order by 1, 2;
@@ -3222,15 +3245,15 @@ select count(*), x.b from ft1, (select c2 a, sum(c1) b from ft1 group by c2) x w
-- FULL join with IS NULL check in HAVING
explain (verbose, costs off)
select avg(t1.c1), sum(t2.c1) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) group by t2.c1 having (avg(t1.c1) is null and sum(t2.c1) < 10) or sum(t2.c1) is null order by 1 nulls last, 2;
- QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Sort
Output: (avg(t1.c1)), (sum(t2.c1)), t2.c1
Sort Key: (avg(t1.c1)), (sum(t2.c1))
-> Foreign Scan
Output: (avg(t1.c1)), (sum(t2.c1)), t2.c1
Relations: Aggregate on ((public.ft4 t1) FULL JOIN (public.ft5 t2))
- Remote SQL: SELECT avg(r1.c1), sum(r2.c1), r2.c1 FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) GROUP BY r2.c1 HAVING ((((avg(r1.c1) IS NULL) AND (sum(r2.c1) < 10)) OR (sum(r2.c1) IS NULL)))
+ Remote SQL: SELECT avg(r1.c1), sum(r2.c1), r2.c1 FROM ("S 1"."T 3" r1 FULL JOIN "S 1"."T 4" r2 ON (((r1.c1 = r2.c1)))) GROUP BY 3 HAVING ((((avg(r1.c1) IS NULL) AND (sum(r2.c1) < 10)) OR (sum(r2.c1) IS NULL)))
(7 rows)
select avg(t1.c1), sum(t2.c1) from ft4 t1 full join ft5 t2 on (t1.c1 = t2.c1) group by t2.c1 having (avg(t1.c1) is null and sum(t2.c1) < 10) or sum(t2.c1) is null order by 1 nulls last, 2;
@@ -3284,8 +3307,8 @@ select sum(c2) * (random() <= 1)::int as sum from ft1 order by 1;
set enable_hashagg to false;
explain (verbose, costs off)
select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 3 and t1."C 1" < 100 order by 1;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------
Sort
Output: t1.c2, qry.sum
Sort Key: t1.c2
@@ -3301,7 +3324,7 @@ select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum fr
-> Foreign Scan
Output: (sum((t2.c1 + t1."C 1"))), t2.c1
Relations: Aggregate on (public.ft2 t2)
- Remote SQL: SELECT sum(("C 1" + $1::integer)), "C 1" FROM "S 1"."T 1" GROUP BY "C 1"
+ Remote SQL: SELECT sum(("C 1" + $1::integer)), "C 1" FROM "S 1"."T 1" GROUP BY 2
(16 rows)
select c2, sum from "S 1"."T 1" t1, lateral (select sum(t2.c1 + t1."C 1") sum from ft2 t2 group by t2.c1) qry where t1.c2 * 2 = qry.sum and t1.c2 < 3 and t1."C 1" < 100 order by 1;
@@ -3447,8 +3470,8 @@ select c2, sum(c1), grouping(c2) from ft1 where c2 < 3 group by c2 order by 1 nu
-- DISTINCT itself is not pushed down, whereas underneath aggregate is pushed
explain (verbose, costs off)
select distinct sum(c1)/1000 s from ft2 where c2 < 6 group by c2 order by 1;
- QUERY PLAN
---------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------------
Unique
Output: ((sum(c1) / 1000)), c2
-> Sort
@@ -3457,7 +3480,7 @@ select distinct sum(c1)/1000 s from ft2 where c2 < 6 group by c2 order by 1;
-> Foreign Scan
Output: ((sum(c1) / 1000)), c2
Relations: Aggregate on (public.ft2)
- Remote SQL: SELECT (sum("C 1") / 1000), c2 FROM "S 1"."T 1" WHERE ((c2 < 6)) GROUP BY c2
+ Remote SQL: SELECT (sum("C 1") / 1000), c2 FROM "S 1"."T 1" WHERE ((c2 < 6)) GROUP BY 2
(9 rows)
select distinct sum(c1)/1000 s from ft2 where c2 < 6 group by c2 order by 1;
@@ -3470,8 +3493,8 @@ select distinct sum(c1)/1000 s from ft2 where c2 < 6 group by c2 order by 1;
-- WindowAgg
explain (verbose, costs off)
select c2, sum(c2), count(c2) over (partition by c2%2) from ft2 where c2 < 10 group by c2 order by 1;
- QUERY PLAN
--------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------
Sort
Output: c2, (sum(c2)), (count(c2) OVER (?)), ((c2 % 2))
Sort Key: ft2.c2
@@ -3483,7 +3506,7 @@ select c2, sum(c2), count(c2) over (partition by c2%2) from ft2 where c2 < 10 gr
-> Foreign Scan
Output: c2, ((c2 % 2)), (sum(c2))
Relations: Aggregate on (public.ft2)
- Remote SQL: SELECT c2, (c2 % 2), sum(c2) FROM "S 1"."T 1" WHERE ((c2 < 10)) GROUP BY c2
+ Remote SQL: SELECT c2, (c2 % 2), sum(c2) FROM "S 1"."T 1" WHERE ((c2 < 10)) GROUP BY 1
(12 rows)
select c2, sum(c2), count(c2) over (partition by c2%2) from ft2 where c2 < 10 group by c2 order by 1;
@@ -3503,8 +3526,8 @@ select c2, sum(c2), count(c2) over (partition by c2%2) from ft2 where c2 < 10 gr
explain (verbose, costs off)
select c2, array_agg(c2) over (partition by c2%2 order by c2 desc) from ft1 where c2 < 10 group by c2 order by 1;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------
Sort
Output: c2, (array_agg(c2) OVER (?)), ((c2 % 2))
Sort Key: ft1.c2
@@ -3516,7 +3539,7 @@ select c2, array_agg(c2) over (partition by c2%2 order by c2 desc) from ft1 wher
-> Foreign Scan
Output: c2, ((c2 % 2))
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT c2, (c2 % 2) FROM "S 1"."T 1" WHERE ((c2 < 10)) GROUP BY c2
+ Remote SQL: SELECT c2, (c2 % 2) FROM "S 1"."T 1" WHERE ((c2 < 10)) GROUP BY 1
(12 rows)
select c2, array_agg(c2) over (partition by c2%2 order by c2 desc) from ft1 where c2 < 10 group by c2 order by 1;
@@ -3536,8 +3559,8 @@ select c2, array_agg(c2) over (partition by c2%2 order by c2 desc) from ft1 wher
explain (verbose, costs off)
select c2, array_agg(c2) over (partition by c2%2 order by c2 range between current row and unbounded following) from ft1 where c2 < 10 group by c2 order by 1;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------
Sort
Output: c2, (array_agg(c2) OVER (?)), ((c2 % 2))
Sort Key: ft1.c2
@@ -3549,7 +3572,7 @@ select c2, array_agg(c2) over (partition by c2%2 order by c2 range between curre
-> Foreign Scan
Output: c2, ((c2 % 2))
Relations: Aggregate on (public.ft1)
- Remote SQL: SELECT c2, (c2 % 2) FROM "S 1"."T 1" WHERE ((c2 < 10)) GROUP BY c2
+ Remote SQL: SELECT c2, (c2 % 2) FROM "S 1"."T 1" WHERE ((c2 < 10)) GROUP BY 1
(12 rows)
select c2, array_agg(c2) over (partition by c2%2 order by c2 range between current row and unbounded following) from ft1 where c2 < 10 group by c2 order by 1;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index fb65e2e..0ad32a5 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4591,7 +4591,7 @@ static bool
foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
{
Query *query = root->parse;
- PathTarget *grouping_target;
+ PathTarget *grouping_target = root->upper_targets[UPPERREL_GROUP_AGG];
PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) grouped_rel->fdw_private;
PgFdwRelationInfo *ofpinfo;
List *aggvars;
@@ -4615,16 +4615,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
return false;
/*
- * The targetlist expected from this node and the targetlist pushed down
- * to the foreign server may be different. The latter requires
- * sortgrouprefs to be set to push down GROUP BY clause, but should not
- * have those arising from ORDER BY clause. These sortgrouprefs may be
- * different from those in the plan's targetlist. Use a copy of path
- * target to record the new sortgrouprefs.
- */
- grouping_target = copy_pathtarget(root->upper_targets[UPPERREL_GROUP_AGG]);
-
- /*
* Evaluate grouping targets and check whether they are safe to push down
* to the foreign side. All GROUP BY expressions will be part of the
* grouping target and thus there is no need to evaluate it separately.
@@ -4641,6 +4631,8 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
/* Check whether this expression is part of GROUP BY clause */
if (sgref && get_sortgroupref_clause_noerr(sgref, query->groupClause))
{
+ TargetEntry *tle;
+
/*
* If any of the GROUP BY expression is not shippable we can not
* push down aggregation to the foreign server.
@@ -4648,8 +4640,18 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
if (!is_foreign_expr(root, grouped_rel, expr))
return false;
- /* Pushable, add to tlist */
- tlist = add_to_flat_tlist(tlist, list_make1(expr));
+ /*
+ * Pushable, add to tlist. We need to create a tle for this
+ * expression and need to transfer the sortgroupref too. We cannot
+ * use add_to_flat_tlist() here as it avoids the duplicate entries
+ * in the targetlist but here we want those duplicate entries as
+ * there can be multiple GROUP BY expressions pointing to the same
+ * column at different positions.
+ */
+ tle = makeTargetEntry((Expr *) expr, list_length(tlist) + 1, NULL,
+ false);
+ tle->ressortgroupref = sgref;
+ tlist = lappend(tlist, tle);
}
else
{
@@ -4661,14 +4663,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
}
else
{
- /*
- * If we have sortgroupref set, then it means that we have an
- * ORDER BY entry pointing to this expression. Since we are
- * not pushing ORDER BY with GROUP BY, clear it.
- */
- if (sgref)
- grouping_target->sortgrouprefs[i] = 0;
-
/* Not matched exactly, pull the var with aggregates then */
aggvars = pull_var_clause((Node *) expr,
PVC_INCLUDE_AGGREGATES);
@@ -4771,9 +4765,6 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel)
}
}
- /* Transfer any sortgroupref data to the replacement tlist */
- apply_pathtarget_labeling_to_tlist(tlist, grouping_target);
-
/* Store generated targetlist */
fpinfo->grouped_tlist = tlist;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index ddfec79..81d2329 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -636,6 +636,12 @@ explain (verbose, costs off)
select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2;
select count(c2) w, c2 x, 5 y, 7.0 z from ft1 group by 2, y, 9.0::int order by 2;
+-- GROUP BY clause referring to same column multiple times
+-- Also, ORDER BY contains an aggregate function
+explain (verbose, costs off)
+select c2, c2 from ft1 where c2 > 6 group by 1, 2 order by sum(c1);
+select c2, c2 from ft1 where c2 > 6 group by 1, 2 order by sum(c1);
+
-- Testing HAVING clause shippability
explain (verbose, costs off)
select c2, sum(c1) from ft2 group by c2 having avg(c1) < 500 and sum(c1) < 49800 order by c2;
* Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-09 11:59 ` Re: BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-10 12:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2017-11-10 18:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-15 09:06 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
@ 2017-11-21 06:25 ` Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2017-12-30 07:13 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2018-01-12 21:55 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 11+ messages in thread
From: Ashutosh Bapat @ 2017-11-21 06:25 UTC (permalink / raw)
To: Jeevan Chalke <jeevan.chalke@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs; Robert Haas <robertmhaas@gmail.com>
On Wed, Nov 15, 2017 at 2:36 PM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:
>>
>> As I alluded to upthread, I suspect that dropping ORDER BY markings from
>> the tlist is likely to break some cases (that is, the planner may expect
>> the output of the foreign scan to include those columns). If this isn't
>> fully exercised by the existing tests then we definitely need more tests.
>
>
> We do have test-coverage where ORDER BY and GROUP BY have common entries.
>
> All ORDER BY expressions are part of the grouping targets and they must be
> either plain vars or aggregate function. Plain vars are anyways part of the
> GROUP BY expression otherwsie we will end-up in a error. And for aggregate
> function, we do include those in a targetlist.
All the ORDER BY expressions are part of the targetlist. If the whole
expression is pushable, we check if the Vars and Aggrefs involved in
that expressions are pushable and include those in the targetlist to
the pushed down. If they are not pushable, we do not push
aggregate/grouping. So, we are good wrt that. I also verified that the
current tests are enough.
I am fine with the patch. It introduces a lot of expected output diff,
since we always deparse GROUP BY clause in positional notation. That's
fine. There is no point in adding extra logic to use positional
notation only when there are duplicate entries in the GROUP BY clause.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-09 11:59 ` Re: BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-10 12:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2017-11-10 18:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-15 09:06 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-21 06:25 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
@ 2017-12-30 07:13 ` Jeevan Chalke <jeevan.chalke@enterprisedb.com>
1 sibling, 0 replies; 11+ messages in thread
From: Jeevan Chalke @ 2017-12-30 07:13 UTC (permalink / raw)
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs; Robert Haas <robertmhaas@gmail.com>
Hi,
I have added this to the 2018-01 Commitfest.
Here is the link to the entry:
https://commitfest.postgresql.org/16/1454/
Earlier attached patch still applies cleanly on the latest HEAD.
Thanks
--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-09 11:59 ` Re: BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-10 12:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2017-11-10 18:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-15 09:06 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-21 06:25 ` Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
@ 2018-01-12 21:55 ` Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 11+ messages in thread
From: Tom Lane @ 2018-01-12 21:55 UTC (permalink / raw)
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>; +Cc: Jeevan Chalke <jeevan.chalke@enterprisedb.com>; pgsql-bugs; Robert Haas <robertmhaas@gmail.com>
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> I am fine with the patch. It introduces a lot of expected output diff,
> since we always deparse GROUP BY clause in positional notation. That's
> fine. There is no point in adding extra logic to use positional
> notation only when there are duplicate entries in the GROUP BY clause.
Yeah, agreed, particularly since that behavior has only been there since
10.0 anyway. Pushed with a few cosmetic adjustments (mostly, improving
shaky English in the comments in foreign_grouping_ok()).
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-09 11:59 ` Re: BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-10 12:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2017-11-10 18:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
@ 2017-11-17 12:09 ` Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
1 sibling, 0 replies; 11+ messages in thread
From: Ashutosh Bapat @ 2017-11-17 12:09 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Jeevan Chalke <jeevan.chalke@enterprisedb.com>; pgsql-bugs; Robert Haas <robertmhaas@gmail.com>
On Sat, Nov 11, 2017 at 12:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> As you say, we can probably get away without that as long as we don't push
> mutable grouping expressions ... but just because we think a grouping
> expression is immutable at our end doesn't necessarily mean that it is at
> the far end.
Well, I thought of that case but then we may have problems without
GROUP BY. use_physical_tlist() doesn't list ForeignScan as an
exception. deparseTargetList() constructs the tlist based on
attrs_used, which doesn't know if the same column was repeated in the
SELECT clause. build_tlist_to_deparse() flattens the tlist to be
deparsed. I think the assumption is that the user won't use immutable
expression as a column.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: [BUGS] BUG #14890: Error grouping by same column twice using FDW
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Re: BUG #14890: Error grouping by same column twice using FDW Tom Lane <tgl@sss.pgh.pa.us>
2017-11-09 11:59 ` Re: BUG #14890: Error grouping by same column twice using FDW Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-10 12:44 ` Re: BUG #14890: Error grouping by same column twice using FDW Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
@ 2017-11-15 08:51 ` Jeevan Chalke <jeevan.chalke@enterprisedb.com>
1 sibling, 0 replies; 11+ messages in thread
From: Jeevan Chalke @ 2017-11-15 08:51 UTC (permalink / raw)
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs; Robert Haas <robertmhaas@gmail.com>
On Fri, Nov 10, 2017 at 6:14 PM, Ashutosh Bapat <
ashutosh.bapat@enterprisedb.com> wrote:
>
> + /*
> + * Pushable, add to tlist. We need to create a tle for this
> + * expression and need to transfer the sortgroupref too. We
> cannot
> + * use add_to_flat_tlist() here as it avoids the duplicate
> entries
> + * in the targetlist but here we want those duplicate entries
> as
> + * there can be multiple GROUP BY expressions pointing to the
> same
> + * column at different positions.
> + */
> + tle = makeTargetEntry((Expr *) expr, list_length(tlist) + 1,
> NULL,
> + false);
> + tle->ressortgroupref = sgref;
> + tlist = lappend(tlist, tle);
>
> May be we want to use add_flat_to_tlist() when sgref is not present to
> avoid
> duplicate entries.
>
>
These code changes are already inside sgref check. And for other places we
do use add_to_flat_tlist().
>
> --
> Best Wishes,
> Ashutosh Bapat
> EnterpriseDB Corporation
> The Postgres Database Company
>
--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2018-01-12 21:55 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-11-07 13:49 BUG #14890: Error grouping by same column twice using FDW sean.johnston@edgeintelligence.com
2017-11-07 22:29 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-09 11:59 ` Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-10 12:44 ` Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2017-11-10 18:44 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-15 09:06 ` Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2017-11-21 06:25 ` Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2017-12-30 07:13 ` Jeevan Chalke <jeevan.chalke@enterprisedb.com>
2018-01-12 21:55 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-11-17 12:09 ` Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
2017-11-15 08:51 ` Jeevan Chalke <jeevan.chalke@enterprisedb.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox