Обсуждение: Mention ordered datums in PartitionBoundInfoData comment
Hi, Julien Rouhaund, who has proposed a patch for partition-wise ordering mentioned to me offlist that the comments for PartitionBoundInfoData do not mention the fact that the datums in datums array are ordered. I think that's important to mention there. So here's patch to do that. The comment I have added refers to the functions which order the datums, since every partition kind has different method of ordering datums and I think the prologue is not a suitable place to explain that ordering. I have added a sentence for range and list partitioning since the ordering is easier to explain in those cases. Also added a sentence about canonical PartitionBoundInfoData without using that word. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Вложения
On Tue, Dec 5, 2017 at 5:48 AM, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote: > Hi, > Hi, > Julien Rouhaund, who has proposed a patch for partition-wise ordering > mentioned to me offlist that the comments for PartitionBoundInfoData > do not mention the fact that the datums in datums array are ordered. I > think that's important to mention there. So here's patch to do that. > Thanks for the patch! I agree this should mentioned in the comment. small typo: + * PartitionBoundInfoData structures for two partitioned table with exactly same should be "tables".
On Tue, Dec 5, 2017 at 11:20 AM, Julien Rouhaud <rjuju123@gmail.com> wrote: > + * PartitionBoundInfoData structures for two partitioned table with > exactly same > > should be "tables". Sorry. Thanks for pointing it out. fixed in the attached patch. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Вложения
On Tue, Dec 5, 2017 at 1:03 AM, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote: > Sorry. Thanks for pointing it out. fixed in the attached patch. + * The datums in datums array are arranged in the increasing order defined by Suggest: in increasing order as defined There's a second place where the same change is needed. + * resp. For range and list partitions this simply means that the datums in the I think you should spell out "respectively" instead of abbreviating to "resp". + * datums array are arranged in the increasing order defined by the partition + * key collation. It's not just the collation but also, and I think more importantly, the operator class. And there can be multiple columns, and thus multiple opclases/collations. Maybe "defined by the partition key's operator classes and collations". + * PartitionBoundInfoData structures for two partitioned tables with exactly + * same bounds look exactly same. This doesn't seem to me to add much. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Dec 8, 2017 at 12:25 AM, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Dec 5, 2017 at 1:03 AM, Ashutosh Bapat > <ashutosh.bapat@enterprisedb.com> wrote: >> Sorry. Thanks for pointing it out. fixed in the attached patch. > > + * The datums in datums array are arranged in the increasing order defined by > > Suggest: in increasing order as defined > Done. > There's a second place where the same change is needed. Done. > > + * resp. For range and list partitions this simply means that the datums in the > > I think you should spell out "respectively" instead of abbreviating to "resp". Done. > > + * datums array are arranged in the increasing order defined by the partition > + * key collation. > > It's not just the collation but also, and I think more importantly, > the operator class. And there can be multiple columns, and thus > multiple opclases/collations. Maybe "defined by the partition key's > operator classes and collations". I had forgot about the operator class. Sorry. Done. > > + * PartitionBoundInfoData structures for two partitioned tables with exactly > + * same bounds look exactly same. > > This doesn't seem to me to add much. > We have a comment in partition_bounds_equal()'s prologue "PartitionBoundInfo is a canonical representation of partition bounds.". But we may use that property in other places also, so having it in prologue of PartitionBoundsInfoData makes sense. For now, I have removed those lines. PFA updated patch. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Вложения
On Mon, Dec 11, 2017 at 7:28 AM, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote: > PFA updated patch. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company