Re: reorganizing partitioning code
On Wed, Mar 28, 2018 at 12:07 AM, Amit Langote wrote: > On 2018/03/22 11:45, Amit Langote wrote: >> FWIW, I did manage to rebase it this morning and posting it here. > > Rebased again. > > I started wondering if we should separate out stuff related to partition > bounds. That is create a utils/partbound.h and put PartitionBoundInfo and > related comparison and search functions into a utils/adt/partbound.c. I > had started thinking about that when I looked at the code added by the > patch submitted on the "advanced partition matching algorithm for > partition-wise join" thread [1]. I haven't done anything about that though. adt = Abstract Data Type, which I think we've interpreted up until now to mean an SQL-visible data type, so that seems like an odd choice. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Re: Re: reorganizing partitioning code
Robert Haas wrote: > On Wed, Mar 21, 2018 at 10:20 AM, Alvaro Herrera > wrote: > > Let's keep this entry open till the last minute. > > Ugh, I don't like keeping things open till the last minute all that > much, especially if they're not being updated. But since this has > been updated I guess it's somewhat moot. > > Are you going to pick this up RSN? If during next week qualifies as RSN, then yes. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Re: Re: reorganizing partitioning code
On Wed, Mar 21, 2018 at 10:20 AM, Alvaro Herrera wrote: > Let's keep this entry open till the last minute. Ugh, I don't like keeping things open till the last minute all that much, especially if they're not being updated. But since this has been updated I guess it's somewhat moot. Are you going to pick this up RSN? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: reorganizing partitioning code
Alvaro Herrera writes: > David Steele wrote: >> Are you planning to update this patch? If not, I think it should be >> marked as Returned with Feedback and submitted to the next CF once it >> has been updated. > This is no new development, only code movement. I think it would be > worse to have three different branches of partitioning code, v10 > "basic", v11 "powerful but not reorganized", v12 "reorganized". I'd > rather have only v10 "basic" and v11+ "powerful". > Let's keep this entry open till the last minute. Nonetheless, it's March 21. David's point is that it's time to get a move on. regards, tom lane
Re: Re: Re: reorganizing partitioning code
David Steele wrote: > > Sorry about the confusing comment. It could be sometime later half of > > the next week. > > We are now three weeks into the CF with no new patch. > > Are you planning to update this patch? If not, I think it should be > marked as Returned with Feedback and submitted to the next CF once it > has been updated. This is no new development, only code movement. I think it would be worse to have three different branches of partitioning code, v10 "basic", v11 "powerful but not reorganized", v12 "reorganized". I'd rather have only v10 "basic" and v11+ "powerful". Let's keep this entry open till the last minute. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Re: Re: reorganizing partitioning code
Hi Amit, On 3/2/18 11:17 AM, Amit Langote wrote: > > On Fri, Mar 2, 2018 at 11:53 PM, David Steele wrote: >> Hi Amit, >> >> On 2/16/18 3:36 AM, Amit Langote wrote: >>> >>> Attached updated version. >> >> This patch no longer applies and the conflicts do not appear to be trivial. >> >> I'm a bit confused about your comment in [1]: >> >>> I gave up on rebasing this patch yesterday as I couldn't finish it in >>> 5 minutes, but maybe I will try later this month. Gotta focus on >>> thefaster pruning stuff for now... >> >> How much later are we talking about? > > Sorry about the confusing comment. It could be sometime later half of > the next week. We are now three weeks into the CF with no new patch. Are you planning to update this patch? If not, I think it should be marked as Returned with Feedback and submitted to the next CF once it has been updated. Regards, -- -David da...@pgmasters.net
Re: Re: reorganizing partitioning code
Hi David. On Fri, Mar 2, 2018 at 11:53 PM, David Steele wrote: > Hi Amit, > > On 2/16/18 3:36 AM, Amit Langote wrote: >> >> Attached updated version. > > This patch no longer applies and the conflicts do not appear to be trivial. > > I'm a bit confused about your comment in [1]: > >> I gave up on rebasing this patch yesterday as I couldn't finish it in >> 5 minutes, but maybe I will try later this month. Gotta focus on >> thefaster pruning stuff for now... > > How much later are we talking about? Sorry about the confusing comment. It could be sometime later half of the next week. Thanks, Amit
Re: Re: reorganizing partitioning code
Hi Amit, On 2/16/18 3:36 AM, Amit Langote wrote: > > Attached updated version. This patch no longer applies and the conflicts do not appear to be trivial. I'm a bit confused about your comment in [1]: > I gave up on rebasing this patch yesterday as I couldn't finish it in > 5 minutes, but maybe I will try later this month. Gotta focus on > thefaster pruning stuff for now... How much later are we talking about? Marked Waiting on Author. -- -David da...@pgmasters.net [1] https://www.postgresql.org/message-id/33098109-9ef1-9594-e7d5-0977a50f8cfa%40lab.ntt.co.jp
Re: reorganizing partitioning code
Hello. I'd like to make a humble comment. At Thu, 15 Feb 2018 19:31:47 +0900, Amit Langote wrote in <8906c861-ea47-caee-c860-eff8d7e1d...@lab.ntt.co.jp> > Added to CF here: https://commitfest.postgresql.org/17/1520/ The reorganization adds/removes header files to/from .[ch] files. That can easily leave useless includes and they're hardly noticed. Following files touched in this patch have such useless includes after this patch. On the other hand, naive decision of this kind of cleanup can lead to curruption. [1] So, I don't insisit that the all of the following *should* amended, especially for rel.h. [1] https://www.postgresql.org/message-id/6748.1518711...@sss.pgh.pa.us nodeModifyTable.c: > +#include "rewrite/rewriteManip.h" rewriteManip.h is changed to include rel.h so rel.h is no longer need to be included in the file. (It is also included in lmgr.h so it was needless since before this patch, though.) hba.c: > +#include "catalog/objectaddress.h" objectaddress.h includes acl.h so acl.h is no longer useful. joinrels.c: > +#include "utils/partcache.h" partcache.h includes lsyscache.h. prepunion.c: > +#include "utils/partcache.h" partcache.h includes lsyscache.h and partcache.h is included in rel.h. So partcache.h and lsyscache.h are not required. utility.c: > +#include "utils/rel.h" rel.h includes xlog.h (and xlog.h includes fd.h). The last two are removable. partcache.c: parsenodes.h is included from some other files. rel.h is included from rewriteManip.h. partcache.h is included from rel.h As the result, parsenodes.h, rel.h, partcache.h are not required. relcache.c: > +#include "utils/partcache.h" lsyscache.h is included by partcache.h. rel.h: > +#include "utils/partcache.h" partcache.h includes fmgr.h and relcache.h so the last two are no longer useful. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: reorganizing partitioning code
Added to CF here: https://commitfest.postgresql.org/17/1520/ Thanks, Amit
Re: reorganizing partitioning code
On 2018/02/15 5:30, Alvaro Herrera wrote: > BTW may I suggest that > > git config diff.algorithm=histogram > > results in better diffs? Aha! That's much better. Thanks, Amit
Re: reorganizing partitioning code (was: Re: [HACKERS] path toward faster partition pruning)
BTW may I suggest that git config diff.algorithm=histogram results in better diffs? -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: reorganizing partitioning code (was: Re: [HACKERS] path toward faster partition pruning)
This is looking attractive. Please don't #include postgres.h in partcache.h. That's per policy. Why do you need to #include parsenodes.h in partcache.h? I think rewriteManip.h can do with just relcache.h rather than rel.h (probably partition.h can do likewise) thanks -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: reorganizing partitioning code (was: Re: [HACKERS] path toward faster partition pruning)
On 2018/02/13 23:08, Ashutosh Bapat wrote: > On Tue, Feb 13, 2018 at 2:17 PM, Amit Langote > wrote: >> >> Agree with the proposed reorganizing and adding a partcache.c, which I >> tried to do in the attached patch. >> >> * The new src/backend/utils/cache/partcache.c contains functions that >> initialize relcache's partitioning related fields. Various partition >> bound comparison and search functions (and then some) that work off of the >> cached information are moved. > > Are you moving partition bound comparison functions to partcache.c? > They will also used by optimizer, so may be leave them out of > partcache.c? Yes, I moved the partition bound comparison and search functions, because I thought that they should live with the other code that manages the cached information. So, I moved not only the code that reads the catalogs and builds the partition key, partition (bound) descriptor, and partition qual (for individual partitions), but also the code that uses those structures. So, with the new arrangement, optimizer will include utils/partcache.h, instead of catalog/partition.h. Thanks, Amit
Re: reorganizing partitioning code
On 2018/02/13 22:23, Alvaro Herrera wrote: > Thanks for working on this. May I suggest to open a completely new > thread? Done. Thanks, Amit
Re: reorganizing partitioning code (was: Re: [HACKERS] path toward faster partition pruning)
On Tue, Feb 13, 2018 at 2:17 PM, Amit Langote wrote: > > Agree with the proposed reorganizing and adding a partcache.c, which I > tried to do in the attached patch. > > * The new src/backend/utils/cache/partcache.c contains functions that > initialize relcache's partitioning related fields. Various partition > bound comparison and search functions (and then some) that work off of the > cached information are moved. Are you moving partition bound comparison functions to partcache.c? They will also used by optimizer, so may be leave them out of partcache.c? -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Re: reorganizing partitioning code (was: Re: [HACKERS] path toward faster partition pruning)
Thanks for working on this. May I suggest to open a completely new thread? -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
