Postgres Pro
Downloads
Home   >   mailing lists

Re: [HACKERS] Partition-wise aggregation/grouping - Mailing list pgsql-hackers

From Jeevan Chalke
Subject Re: [HACKERS] Partition-wise aggregation/grouping
Date
Msg-id CAM2+6=XpUzoELzjT1ZSZRWzB4Cmp7A83aWpXUATXnxckfO+ckw@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Partition-wise aggregation/grouping  (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>)
Responses Re: [HACKERS] Partition-wise aggregation/grouping  (Jeevan Chalke <jeevan.chalke@enterprisedb.com>)
List pgsql-hackers
Tree view
Hi,

I have resolved all the comments/issues reported in this new patch-set.

Changes done by Ashutosh Bapat for splitting out create_grouping_paths() into separate functions so that partitionwise aggregate code will use them were based on my partitionwise aggregate changes. Those were like refactoring changes. And thus, I have refactored them separately and before any partitionwise changes (see 0005-Split-create_grouping_paths-and-Add-GroupPathExtraDa.patch). And then I have re-based all partitionwise changes over it including all fixes.

The patch-set is complete now. But still, there is a scope of some comment improvements due to all these refactorings. I will work on it. Also, need to update few documentations and indentations etc. Will post those changes in next patch-set. But meanwhile, this patch-set is ready to review.


On Tue, Mar 13, 2018 at 9:12 AM, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote:
On Mon, Mar 12, 2018 at 7:49 PM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:
>
>
> On Mon, Mar 12, 2018 at 6:07 PM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
>>
>> On Fri, Mar 9, 2018 at 4:21 PM, Ashutosh Bapat
>>
We don't need UpperRelationKind member in that structure. That will be
provided by the RelOptInfo passed.

The problem here is the extra information required for grouping is not
going to be the same for that needed for window aggregate and
certainly not for ordering. If we try to jam everything in the same
structure, it will become large with many members useless for a given
operation. A reader will not have an idea about which of them are
useful and which of them are not. So, instead we should try some
polymorphism. I think we can pass a void * to GetForeignUpperPaths()
and corresponding FDW hook knows what to cast it to based on the
UpperRelationKind passed.

Yep. Done this way.
 

BTW, the patch has added an argument to GetForeignUpperPaths() but has
not documented the change in API. If we go the route of polymorphism,
we will need to document the mapping between UpperRelationKind and the
type of structure passed in.

Oops. Will do that in next patchset.

Thanks for pointing out, I have missed this at first place it self.

--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Attachment

pgsql-hackers by date:

Previous
From: David Steele
Date:
Subject: Re: PATCH: Unlogged tables re-initialization tests
Next
From: Peter Eisentraut
Date:
Subject: Re: JIT compiling with LLVM v11