Thread: [HACKERS] moving some partitioning code to executor
Hi. It seems to me that some of the code in partition.c is better placed somewhere under the executor directory. There was even a suggestion recently [1] to introduce a execPartition.c to house some code around tuple-routing. IMO, catalog/partition.c should present an interface for handling operations on a *single* partitioned table and avoid pretending to support any operations on the whole partition tree. For example, the PartitionDispatch structure embeds some knowledge about the partition tree it's part of, which is useful when used for tuple-routing, because of the way it works now (lock and form ResultRelInfos of *all* leaf partitions before the first input row is processed). So, let's move that structure, along with the code that creates and manipulates the same, out of partition.c/h and to execPartition.c/h. Attached patch attempts to do that. While doing the same, I didn't move *all* of get_partition_for_tuple() out to execPartition.c, instead modified its signature as shown below: -extern int get_partition_for_tuple(PartitionDispatch *pd, - TupleTableSlot *slot, - EState *estate, - PartitionDispatchData **failed_at, - TupleTableSlot **failed_slot); +extern int get_partition_for_tuple(Relation relation, Datum *values, + bool *isnull); That way, we keep the core partition bound comparison logic inside partition.c and move rest of the stuff to its caller ExecFindPartition(), which includes navigating the enveloping PartitionDispatch's. Thoughts? PS: 0001 of the attached is the patch from [2] which is here to be applied on HEAD before applying the main patch (0002) itself Thanks, Amit [1] https://www.postgresql.org/message-id/CA%2BTgmoafr%3DhUrM%3Dcbx-k%3DBDHOF2OfXa w95HQSNAK4mHBwmSjtw%40mail.gmail.com [2] https://www.postgresql.org/message-id/7fe0007b-7ad1-a593-df11-ad05364ebce4%40l ab.ntt.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Repeating links for better accessibility: On 2017/09/14 16:13, Amit Langote wrote: > [1] https://www.postgresql.org/message-id/CA%2BTgmoafr%3DhUrM%3Dcbx-k%3DBDHOF2OfXaw95HQSNAK4mHBwmSjtw%40mail.gmail.com > [2] https://www.postgresql.org/message-id/7fe0007b-7ad1-a593-df11-ad05364ebce4%40lab.ntt.co.jp Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On 2017/09/14 16:13, Amit Langote wrote: > Hi. > > It seems to me that some of the code in partition.c is better placed > somewhere under the executor directory. There was even a suggestion > recently [1] to introduce a execPartition.c to house some code around > tuple-routing. > > IMO, catalog/partition.c should present an interface for handling > operations on a *single* partitioned table and avoid pretending to support > any operations on the whole partition tree. For example, the > PartitionDispatch structure embeds some knowledge about the partition tree > it's part of, which is useful when used for tuple-routing, because of the > way it works now (lock and form ResultRelInfos of *all* leaf partitions > before the first input row is processed). > > So, let's move that structure, along with the code that creates and > manipulates the same, out of partition.c/h and to execPartition.c/h. > Attached patch attempts to do that. > > While doing the same, I didn't move *all* of get_partition_for_tuple() out > to execPartition.c, instead modified its signature as shown below: > > -extern int get_partition_for_tuple(PartitionDispatch *pd, > - TupleTableSlot *slot, > - EState *estate, > - PartitionDispatchData **failed_at, > - TupleTableSlot **failed_slot); > +extern int get_partition_for_tuple(Relation relation, Datum *values, > + bool *isnull); > > That way, we keep the core partition bound comparison logic inside > partition.c and move rest of the stuff to its caller ExecFindPartition(), > which includes navigating the enveloping PartitionDispatch's. > > Thoughts? > > PS: 0001 of the attached is the patch from [2] which is here to be applied > on HEAD before applying the main patch (0002) itself Since that 0001 patch was committed [1], here is the rebased patch. Will add this to the November commit-fest. Thanks, Amit [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=77b6b5e9c -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Amit Langote wrote: > Since that 0001 patch was committed [1], here is the rebased patch. Will > add this to the November commit-fest. Please rebase once more -- 1aba8e651ac3 seems to have broken things in this area pretty thoroughly. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2017/11/15 2:09, Alvaro Herrera wrote: > Amit Langote wrote: > >> Since that 0001 patch was committed [1], here is the rebased patch. Will >> add this to the November commit-fest. > > Please rebase once more -- 1aba8e651ac3 seems to have broken things > in this area pretty thoroughly. Thanks, done. Regards, Amit
Attachment
On Tue, Nov 14, 2017 at 7:59 PM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote: > On 2017/11/15 2:09, Alvaro Herrera wrote: >> Amit Langote wrote: >> >>> Since that 0001 patch was committed [1], here is the rebased patch. Will >>> add this to the November commit-fest. >> >> Please rebase once more -- 1aba8e651ac3 seems to have broken things >> in this area pretty thoroughly. > > Thanks, done. Committed. (Alvaro, hope that's not stepping your toes ...) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2017/11/16 0:29, Robert Haas wrote: > On Tue, Nov 14, 2017 at 7:59 PM, Amit Langote > <Langote_Amit_f8@lab.ntt.co.jp> wrote: >> On 2017/11/15 2:09, Alvaro Herrera wrote: >>> Amit Langote wrote: >>> >>>> Since that 0001 patch was committed [1], here is the rebased patch. Will >>>> add this to the November commit-fest. >>> >>> Please rebase once more -- 1aba8e651ac3 seems to have broken things >>> in this area pretty thoroughly. >> >> Thanks, done. > > Committed. Thank you. Regards, Amit