On Fri, Dec 8, 2017 at 12:25 AM, Robert Haas <robertmh...@gmail.com> wrote:
> On Tue, Dec 5, 2017 at 1:03 AM, Ashutosh Bapat
> <ashutosh.ba...@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
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index dd4a8d3..ef156e4 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -72,6 +72,13 @@
  * of datum-tuples with 2 datums, modulus and remainder, corresponding to a
  * given partition.
  *
+ * The datums in datums array are arranged in increasing order as defined by
+ * functions qsort_partition_rbound_cmp(), qsort_partition_list_value_cmp() and
+ * qsort_partition_hbound_cmp() for range, list and hash partitioned tables
+ * respectively. For range and list partitions this simply means that the
+ * datums in the datums array are arranged in increasing order as defined by
+ * the partition key's operator classes and collations.
+ *
  * In the case of list partitioning, the indexes array stores one entry for
  * every datum, which is the index of the partition that accepts a given datum.
  * In case of range partitioning, it stores one entry per distinct range

Reply via email to