Horton

Thread

Commits

  1. Improve comment about PartitionBoundInfoData.

  1. Mention ordered datums in PartitionBoundInfoData comment

    Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> — 2017-12-05T04:48:43Z

    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

  2. Re: Mention ordered datums in PartitionBoundInfoData comment

    Julien Rouhaud <rjuju123@gmail.com> — 2017-12-05T05:50:23Z

    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".

  3. Re: Mention ordered datums in PartitionBoundInfoData comment

    Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> — 2017-12-05T06:03:01Z

    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

  4. Re: Mention ordered datums in PartitionBoundInfoData comment

    Robert Haas <robertmhaas@gmail.com> — 2017-12-07T18:55:52Z

    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

  5. Re: Mention ordered datums in PartitionBoundInfoData comment

    Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> — 2017-12-11T12:28:10Z

    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

  6. Re: Mention ordered datums in PartitionBoundInfoData comment

    Robert Haas <robertmhaas@gmail.com> — 2017-12-11T17:52:24Z

    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