> 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
