On Thu, Mar 8, 2018 at 7:31 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> This kind of goes along with the suggestion I made yesterday to
> introduce a new function, which at the time I proposed calling
> initialize_grouping_rel(), to set up new grouped or partially grouped
> relations. By doing that it would be easier to ensure the
> initialization is always done in a consistent way but only for the
> relations we actually need. But maybe we should call it
> fetch_grouping_rel() instead. The idea would be that instead of
> calling fetch_upper_rel() we would call fetch_grouping_rel() when it
> is a question of the grouped or partially grouped relation. It would
> either return the existing relation or initialize a new one for us. I
> think that would make it fairly easy to initialize only the ones we're
> going to need.
Hmm. I am working on refactoring the code to do something like this.
> On a related note, I'm not sure that this code is correct:
>
> + if (!isPartialAgg)
> + {
> + grouped_rel->part_scheme = input_rel->part_scheme;
> + grouped_rel->nparts = nparts;
> + grouped_rel->boundinfo = input_rel->boundinfo;
> + grouped_rel->part_rels = part_rels;
> + }
>
> It's not obvious to me why this should be done only when
> !isPartialAgg. The comments claim that the partially grouped child
> rels can't be considered partitions of the top-level partitially
> grouped rel, but it seems to me that we could consider them that way.
> Maybe I'm missing something.
When we are performing partial aggregates, GROUP clause does not have
partition keys. This means that the targetlist of the grouped relation
and partially grouped relation do not have bare partition keys. So,
for a relation sitting on top of this (partially) grouped relation the
partition key doesn't exist. So, we can't consider grouped or
partially grouped relation as partitioned.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company