Relids in upper relations

Hi,
While reviewing aggregate pushdown patch [1] we noticed that
RelOptInfos for upper relations do not have relids set.
create_foreignscan_plan() copies the relids from RelOptInfo into
ForeignScan::fs_relids. That field is used to identify the RTIs
covered by the ForeignScan. For example, postgresBeginForeignScan()
uses it to get the user mapping
1281 /*
1282 * Identify which user to do the remote access as. This
should match what
1283 * ExecCheckRTEPerms() does. In case of a join, use the
lowest-numbered
1284 * member RTE as a representative; we would get the same
result from any.
1285 */
1286 if (fsplan->scan.scanrelid > 0)
1287 rtindex = fsplan->scan.scanrelid;
1288 else
1289 rtindex = bms_next_member(fsplan->fs_relids, -1);
1290 rte = rt_fetch(rtindex, estate->es_range_table);
1291 userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();

Core functions also use this field to get RTIs covered by ForeignScan
e.g. ExplainPreScanNode.

Since this field is not set in an upper relation, when we push down an
upper operation like grouping/aggregation, fs_relids remains NULL,
causing undesirable results. In case of postgres_fdw aggregate
pushdown, it crashed in rt_fetch().

We could prevent the crash by passing input_rel->relids to
fetch_upper_rel() in create_grouping_path() as seen in the attached
patch. preprocess_minmax_aggregates() needed to be fixed so that both
these functions add paths to the same relation. I am sure, if we go
this route, we will have to fix each call of fetch_upper_rel() in this
manner. But I am not sure if it's intended to be so.

The comment in fetch_upper_rel() says
847 * An "upper" relation is identified by an UpperRelationKind and
a Relids set.
848 * The meaning of the Relids set is not specified here, and very
likely will
849 * vary for different relation kinds.
which seems to indicate that relids in upper relation will be
different that those in the lower relations, but it doesn't say how.

We need to fix the usages of fs_relids or the calls to
fetch_upper_rel() for pushdown of upper operations. I am not sure
which. Please suggest, how to move forward with this.

[1] https://www.postgresql.org/message-id/flat/CANEvxPokcFi7OfEpi3%3DJ%2Bmvxu%2BPPAG2zXASLEkv5DzDPhSTk6A%40mail(dot)gmail(dot)com#CANEvxPokcFi7OfEpi3=J+mvxu+PPAG2zXASLEkv5DzDPhSTk6A(at)mail(dot)gmail(dot)com
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company