Re: [HACKERS] Partition-wise aggregation/grouping

Поиск
Список
Период
Сортировка
От Jeevan Chalke
Тема Re: [HACKERS] Partition-wise aggregation/grouping
Дата
Msg-id CAM2+6=XpUzoELzjT1ZSZRWzB4Cmp7A83aWpXUATXnxckfO+ckw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [HACKERS] Partition-wise aggregation/grouping  (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>)
Ответы Re: [HACKERS] Partition-wise aggregation/grouping  (Jeevan Chalke <jeevan.chalke@enterprisedb.com>)
Список pgsql-hackers
Дерево обсуждения
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

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: David Steele
Дата:
Сообщение: Re: PATCH: Unlogged tables re-initialization tests
Следующее
От: Peter Eisentraut
Дата:
Сообщение: Re: JIT compiling with LLVM v11