Multi column range partition table

On 2017/06/22 20:48, amul sul wrote:

Hi,

While working on the another patch, I came across the case where
I need an auto generated partition for a mutil-column range partitioned
table having following range bound:

PARTITION p1 FROM (UNBOUNDED, UNBOUNDED) TO (10, 10)
PARTITION p2 FROM (10, 10) TO (10, UNBOUNDED)
PARTITION p3 FROM (10, UNBOUNDED) TO (20, 10)
PARTITION p4 FROM (20, 10) TO (20, UNBOUNDED)
PARTITION p5 FROM (20, UNBOUNDED) TO (UNBOUNDED, UNBOUNDED)

In this, a lower bound of the partition is an upper bound of the
previous partition.

While trying to create p3 partition with (10, UNBOUNDED) to (20, 10) bound,
got an overlap partition error.

Here is the SQL to reproduced this error:

CREATE TABLE range_parted ( i1 int, i2 int ) PARTITION BY RANGE (i1, i2);
CREATE TABLE p1 PARTITION OF range_parted FOR VALUES FROM (UNBOUNDED,
UNBOUNDED) TO (10, 10);
CREATE TABLE p2 PARTITION OF range_parted FOR VALUES FROM (10, 10) TO
(10, UNBOUNDED);
CREATE TABLE p3 PARTITION OF tab1 FOR VALUES FROM (10, UNBOUNDED) TO (20, 10);

ERROR: partition "p3" would overlap partition "tab1_p_10_10"

This happened because of UNBOUNDED handling, where it is a negative infinite
if it is in FROM clause. Wondering can't we explicitly treat this as
a positive infinite value, can we?

No, we cannot. What would be greater than (or equal to) +infinite?
Nothing. So, even if you will want p3 to accept (10, 9890148), it won't
because 9890148 is not >= +infinite. It will accept only the rows where
the first column is > 10 (second column is not checked in that case).

You will have to define p3 as follows:

CREATE TABLE p3 PARTITION OF tab1 FOR VALUES FROM (11, UNBOUNDED) TO (20, 10);

It's fine to use the previous partition's upper bound as the lower bound
of the current partition, if the former does contain an UNBOUNDED value,
because whereas a finite value divides the range into two parts (assigned
to the two partitions respectively), an UNBOUNDED value does not. The
latter represents an abstract end of the range (either on the positive
side or the negative).

Does that make sense?

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 Fri, Jun 23, 2017 at 6:58 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2017/06/22 20:48, amul sul wrote:

Hi,

While working on the another patch, I came across the case where
I need an auto generated partition for a mutil-column range partitioned
table having following range bound:

PARTITION p1 FROM (UNBOUNDED, UNBOUNDED) TO (10, 10)
PARTITION p2 FROM (10, 10) TO (10, UNBOUNDED)
PARTITION p3 FROM (10, UNBOUNDED) TO (20, 10)
PARTITION p4 FROM (20, 10) TO (20, UNBOUNDED)
PARTITION p5 FROM (20, UNBOUNDED) TO (UNBOUNDED, UNBOUNDED)

In this, a lower bound of the partition is an upper bound of the
previous partition.

While trying to create p3 partition with (10, UNBOUNDED) to (20, 10) bound,
got an overlap partition error.

Here is the SQL to reproduced this error:

CREATE TABLE range_parted ( i1 int, i2 int ) PARTITION BY RANGE (i1, i2);
CREATE TABLE p1 PARTITION OF range_parted FOR VALUES FROM (UNBOUNDED,
UNBOUNDED) TO (10, 10);
CREATE TABLE p2 PARTITION OF range_parted FOR VALUES FROM (10, 10) TO
(10, UNBOUNDED);
CREATE TABLE p3 PARTITION OF tab1 FOR VALUES FROM (10, UNBOUNDED) TO (20, 10);

ERROR: partition "p3" would overlap partition "tab1_p_10_10"

This happened because of UNBOUNDED handling, where it is a negative infinite
if it is in FROM clause. Wondering can't we explicitly treat this as
a positive infinite value, can we?

No, we cannot. What would be greater than (or equal to) +infinite?
Nothing. So, even if you will want p3 to accept (10, 9890148), it won't
because 9890148 is not >= +infinite. It will accept only the rows where
the first column is > 10 (second column is not checked in that case).

You will have to define p3 as follows:

CREATE TABLE p3 PARTITION OF tab1 FOR VALUES FROM (11, UNBOUNDED) TO (20, 10);

What if the partition key column is FLOAT ?

Regards,
Amul

--
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/06/23 13:42, amul sul wrote:

On Fri, Jun 23, 2017 at 6:58 AM, Amit Langote wrote:

On 2017/06/22 20:48, amul sul wrote:

This happened because of UNBOUNDED handling, where it is a negative infinite
if it is in FROM clause. Wondering can't we explicitly treat this as
a positive infinite value, can we?

No, we cannot. What would be greater than (or equal to) +infinite?
Nothing. So, even if you will want p3 to accept (10, 9890148), it won't
because 9890148 is not >= +infinite. It will accept only the rows where
the first column is > 10 (second column is not checked in that case).

You will have to define p3 as follows:

CREATE TABLE p3 PARTITION OF tab1 FOR VALUES FROM (11, UNBOUNDED) TO (20, 10);

What if the partition key column is FLOAT ?

I would say use a value such that the btfloat4cmp (or btfloat8cmp) will
tell it to be greater than 10.

Of course, we can't write what I just said in the user-level
documentation, because the fact that we use system- or user-defined btree
comparison proc (btfloat4/8cmp) for partitioning may be irrelevant to the
users. Although, we do mention in the documentation that we use btree
operator class specified semantics for partitioning. In any case,
defining your partitioning or indexing on raw float type column(s) is
prone to semantic caveats, I'd think.

Also, there was interesting exchange on this topic during the patch
development [1]/messages/by-id/CA+TgmoaucSqQ=dJFhaojSpb1706MQYo1Tfn_3tWv6CVWhAOdrQ@mail.gmail.com. Excerpt:

"Same for ranges of floating-point numbers, which are also probably an
unlikely candidate for a partitioning key anyway."

Thanks,
Amit

[1]: /messages/by-id/CA+TgmoaucSqQ=dJFhaojSpb1706MQYo1Tfn_3tWv6CVWhAOdrQ@mail.gmail.com
/messages/by-id/CA+TgmoaucSqQ=dJFhaojSpb1706MQYo1Tfn_3tWv6CVWhAOdrQ@mail.gmail.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Jun 23, 2017 at 6:58 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2017/06/22 20:48, amul sul wrote:

Hi,

While working on the another patch, I came across the case where
I need an auto generated partition for a mutil-column range partitioned
table having following range bound:

PARTITION p1 FROM (UNBOUNDED, UNBOUNDED) TO (10, 10)
PARTITION p2 FROM (10, 10) TO (10, UNBOUNDED)
PARTITION p3 FROM (10, UNBOUNDED) TO (20, 10)
PARTITION p4 FROM (20, 10) TO (20, UNBOUNDED)
PARTITION p5 FROM (20, UNBOUNDED) TO (UNBOUNDED, UNBOUNDED)

In this, a lower bound of the partition is an upper bound of the
previous partition.

While trying to create p3 partition with (10, UNBOUNDED) to (20, 10) bound,
got an overlap partition error.

Here is the SQL to reproduced this error:

CREATE TABLE range_parted ( i1 int, i2 int ) PARTITION BY RANGE (i1, i2);
CREATE TABLE p1 PARTITION OF range_parted FOR VALUES FROM (UNBOUNDED,
UNBOUNDED) TO (10, 10);
CREATE TABLE p2 PARTITION OF range_parted FOR VALUES FROM (10, 10) TO
(10, UNBOUNDED);
CREATE TABLE p3 PARTITION OF tab1 FOR VALUES FROM (10, UNBOUNDED) TO (20, 10);

ERROR: partition "p3" would overlap partition "tab1_p_10_10"

This happened because of UNBOUNDED handling, where it is a negative infinite
if it is in FROM clause. Wondering can't we explicitly treat this as
a positive infinite value, can we?

The way we have designed our syntax, we don't have a way to tell that
p3 comes after p2 and they have no gap between those. But I don't
think that's your question. What you are struggling with is a way to
specify a lower bound (10, +infinity) so that anything with i1 > 10
would go to partition 3.

No, we cannot. What would be greater than (or equal to) +infinite?
Nothing. So, even if you will want p3 to accept (10, 9890148), it won't
because 9890148 is not >= +infinite. It will accept only the rows where
the first column is > 10 (second column is not checked in that case).

You will have to define p3 as follows:

CREATE TABLE p3 PARTITION OF tab1 FOR VALUES FROM (11, UNBOUNDED) TO (20, 10);

That's not exactly same as specifying (10, +infinity) in case i1 is a
float. A user can not precisely tell what would be the acceptable
value just greater than 10.

An UNBOUNDED in the lower bound is always considered as -infinity for
that data type. There is no way to specify a lower bound which has
+infinity in it. +infinite as a lower bounds for the first key may not
make sense (although that means that the partition will always be
empty), but it does make sense for keys after the first as Amul has
explained below.

The question is do we have support for that and if not, will we
consider it for v10 or v11 and how.

It's fine to use the previous partition's upper bound as the lower bound
of the current partition, if the former does contain an UNBOUNDED value,
because whereas a finite value divides the range into two parts (assigned
to the two partitions respectively), an UNBOUNDED value does not. The
latter represents an abstract end of the range (either on the positive
side or the negative).

Not exactly for second key onwards.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 23 June 2017 at 08:01, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:

The way we have designed our syntax, we don't have a way to tell that
p3 comes after p2 and they have no gap between those. But I don't
think that's your question. What you are struggling with is a way to
specify a lower bound (10, +infinity) so that anything with i1 > 10
would go to partition 3.

I think actually there is a fundamental problem here, which arises
because UNBOUNDED has 2 different meanings depending on context, and
thus it is not possible in general to specify the start of one range
to be equal to the end of the previous range, as is necessary to get
contiguous non-overlapping ranges.

Note that this isn't just a problem for floating point datatypes
either, it also applies to other types such as strings. For example,
given a partition over (text, int) types defined with the following
values:

FROM ('a', UNBOUNDED) TO ('b', UNBOUNDED)

which is equivalent to

FROM ('a', -INFINITY) TO ('b', +INFINITY)

where should the next range start?

Even if you were to find a way to specify "the next string after 'b'",
it wouldn't exactly be pretty. The problem is that the above partition
corresponds to "all the strings starting with 'a', plus the string
'b', which is pretty ugly. A neater way to define the pair of ranges
in this case would be:

FROM ('a', -INFINITY) TO ('b', -INFINITY)
FROM ('b', -INFINITY) TO ('c', -INFINITY)

since then all strings starting with 'a' would fall into the first
partition and all the strings starting with 'b' would fall into the
second one.

Currently, when there are 2 partition columns, the partition
constraint is defined as

(a is not null) and (b is not null)
and
(a > al or (a = al and b >= bl))
and
(a < au or (a = au and b < bu))

if the upper bound bu were allowed to be -INFINITY (something that
should probably be forbidden unless the previous column's upper bound
were finite), then this would simplify to

(a is not null) and (b is not null)
and
(a > al or (a = al and b >= bl))
and
(a < au)

and in the example above, where al is -INFINITY, it would further simplify to

(a is not null) and (b is not null)
and
(a >= al)
and
(a < au)

There would also be a similar simplification possible if the lower
bound of a partition column were allowed to be +INFINITY.

So, I think that having UNBOUNDED represent both -INFINITY and
+INFINITY depending on context is a design flaw, and that we need to
allow both -INFINITY and +INFINITY as upper and lower bounds (provided
they are preceded by a column with a finite bound). I think that, in
general, that's the only way to allow contiguous non-overlapping
partitions to be defined on multiple columns.

Regards,
Dean

--
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/06/23 17:00, Dean Rasheed wrote:

On 23 June 2017 at 08:01, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:

The way we have designed our syntax, we don't have a way to tell that
p3 comes after p2 and they have no gap between those. But I don't
think that's your question. What you are struggling with is a way to
specify a lower bound (10, +infinity) so that anything with i1 > 10
would go to partition 3.

I think actually there is a fundamental problem here, which arises
because UNBOUNDED has 2 different meanings depending on context, and
thus it is not possible in general to specify the start of one range
to be equal to the end of the previous range, as is necessary to get
contiguous non-overlapping ranges.

Okay, I thought about this a bit more and I think I realize that this
arbitrary-sounding restriction of allowing only -infinity in FROM and
+infinity in TO limits the usefulness of the feature to specify infinite
bounds at all.

Note that this isn't just a problem for floating point datatypes
either, it also applies to other types such as strings. For example,
given a partition over (text, int) types defined with the following
values:

FROM ('a', UNBOUNDED) TO ('b', UNBOUNDED)

which is equivalent to

FROM ('a', -INFINITY) TO ('b', +INFINITY)

where should the next range start?

Even if you were to find a way to specify "the next string after 'b'",
it wouldn't exactly be pretty. The problem is that the above partition
corresponds to "all the strings starting with 'a', plus the string
'b', which is pretty ugly. A neater way to define the pair of ranges
in this case would be:

FROM ('a', -INFINITY) TO ('b', -INFINITY)
FROM ('b', -INFINITY) TO ('c', -INFINITY)

since then all strings starting with 'a' would fall into the first
partition and all the strings starting with 'b' would fall into the
second one.

I agree that a valid use case like the one above is awkward to express
currently.

Currently, when there are 2 partition columns, the partition
constraint is defined as

(a is not null) and (b is not null)
and
(a > al or (a = al and b >= bl))
and
(a < au or (a = au and b < bu))

if the upper bound bu were allowed to be -INFINITY (something that
should probably be forbidden unless the previous column's upper bound
were finite), then this would simplify to

(a is not null) and (b is not null)
and
(a > al or (a = al and b >= bl))
and
(a < au)

and in the example above, where al is -INFINITY, it would further simplify to

(a is not null) and (b is not null)
and
(a >= al)
and
(a < au)

There would also be a similar simplification possible if the lower
bound of a partition column were allowed to be +INFINITY.

Yep.

So, I think that having UNBOUNDED represent both -INFINITY and
+INFINITY depending on context is a design flaw, and that we need to
allow both -INFINITY and +INFINITY as upper and lower bounds (provided
they are preceded by a column with a finite bound). I think that, in
general, that's the only way to allow contiguous non-overlapping
partitions to be defined on multiple columns.

Alright, I spent some time implementing a patch to allow specifying
-infinity and +infinity in arbitrary ways. Of course, it prevents
nonsensical inputs with appropriate error messages.

When implementing the same, I initially thought that the only grammar
modification required is to allow specifying a sign before the unbounded
keyword, but thought it sounded strange to call the actual bound values
-unbounded and +unbounded.  While the keyword "unbounded" describes the
property of being unbounded, actual values are really -infinity and
+infinity.  So, I decided to instead modify the grammar to accept
-infinity and +infinity in the FROM and TO lists.  The sign is optional
and in its absence, infinity in FROM means -infinity and vice versa.  This
decision may be seen as controversial, now that we are actually in beta,
if we decide to go with this patch at all.

Some adjustments were required in the logic in partition.c that depended
on the old assumption that all infinite values in the lower bound meant
-infinity and vice versa. That includes get_qual_for_range() being able
to simplify the partition constraint as Dean mentioned in his email.

When testing the patch, I realized that the current code in
check_new_partition_bound() that checks for range partition overlap had a
latent bug that resulted in false positives for the new cases that the new
less restrictive syntax allowed. I spent some time simplifying that code
while also fixing the aforementioned bug. It's implemented in the
attached patch 0001.

0002 is the patch that implements the new syntax.

It's possible that this won't be considered a PG 10 open item but a new
feature and so PG 11 material, as Ashutosh also wondered.

Thanks,
Amit

On Fri, Jun 30, 2017 at 1:36 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Alright, I spent some time implementing a patch to allow specifying
-infinity and +infinity in arbitrary ways. Of course, it prevents
nonsensical inputs with appropriate error messages.

I don't think -infinity and +infinity are the right terms. For a
string or character data type there is no -infinity and +infinity.
Similarly for enums. We need to extend UNBOUNDED somehow to indicate
the end of a given type in the given direction. I thought about
UNBOUNDED LEFT/RIGHT but then whether LEFT indicates -ve side or +side
would cause confusion. Also LEFT/RIGHT may work for a single
dimensional datatype but not for multi-dimensional spaces. How about
MINIMUM/MAXIMUM or UNBOUNDED MIN/MAX to indicate the extremities.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 30 June 2017 at 09:06, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

When testing the patch, I realized that the current code in
check_new_partition_bound() that checks for range partition overlap had a
latent bug that resulted in false positives for the new cases that the new
less restrictive syntax allowed. I spent some time simplifying that code
while also fixing the aforementioned bug. It's implemented in the
attached patch 0001.

I haven't had time to look at 0002 yet, but looking at 0001, I'm not
convinced that this really represents much of a simplification, but I
do prefer the way it now consistently reports the first overlapping
partition in the error message.

I'm not entirely convinced by this change either:

-                        if (equal || off1 != off2)
+                        if (off2 > off1 + 1 || ((off2 == off1 + 1) && !equal))

Passing probe_is_bound = true to partition_bound_bsearch() will I
think cause it to return equal = false when the upper bound of one
partition equals the lower bound of another, so relying on the "equal"
flag here seems a bit dubious. I think I can just about convince
myself that it works, but not for the reasons stated in the comments.

It also seems unnecessary for this code to be doing 2 binary searches.
I think a better simplification would be to just do one binary search
to find the gap that the lower bound fits in, and then test the upper
bound of the new partition against the lower bound of the next
partition (if there is one), as in the attached patch.

Regards,
Dean

On 30 June 2017 at 10:04, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:

On Fri, Jun 30, 2017 at 1:36 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Alright, I spent some time implementing a patch to allow specifying
-infinity and +infinity in arbitrary ways. Of course, it prevents
nonsensical inputs with appropriate error messages.

I don't think -infinity and +infinity are the right terms. For a
string or character data type there is no -infinity and +infinity.
Similarly for enums. We need to extend UNBOUNDED somehow to indicate
the end of a given type in the given direction. I thought about
UNBOUNDED LEFT/RIGHT but then whether LEFT indicates -ve side or +side
would cause confusion. Also LEFT/RIGHT may work for a single
dimensional datatype but not for multi-dimensional spaces. How about
MINIMUM/MAXIMUM or UNBOUNDED MIN/MAX to indicate the extremities.

Yes, I think you're right. Also, some datatypes include values that
are equal to +/-infinity, which would then behave differently from
unbounded as range bounds, so it wouldn't be a good idea to overload
that term.

My first thought was UNBOUNDED ABOVE/BELOW, because that matches the
terminology already in use of upper and lower bounds.

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Dean,

Thanks a lot for the review.

On 2017/07/03 1:59, Dean Rasheed wrote:

On 30 June 2017 at 09:06, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

When testing the patch, I realized that the current code in
check_new_partition_bound() that checks for range partition overlap had a
latent bug that resulted in false positives for the new cases that the new
less restrictive syntax allowed. I spent some time simplifying that code
while also fixing the aforementioned bug. It's implemented in the
attached patch 0001.

I haven't had time to look at 0002 yet, but looking at 0001, I'm not
convinced that this really represents much of a simplification, but I
do prefer the way it now consistently reports the first overlapping
partition in the error message.

I'm not entirely convinced by this change either:

-                        if (equal || off1 != off2)
+                        if (off2 > off1 + 1 || ((off2 == off1 + 1) && !equal))

Passing probe_is_bound = true to partition_bound_bsearch() will I
think cause it to return equal = false when the upper bound of one
partition equals the lower bound of another, so relying on the "equal"
flag here seems a bit dubious. I think I can just about convince
myself that it works, but not for the reasons stated in the comments.

You are right. What's actually happening in the case where I was thinking
equal would be set to true is that off2 ends up being equal to off1, so
the second arm of that || is not checked at all.

It also seems unnecessary for this code to be doing 2 binary searches.
I think a better simplification would be to just do one binary search
to find the gap that the lower bound fits in, and then test the upper
bound of the new partition against the lower bound of the next
partition (if there is one), as in the attached patch.

I agree. The patch looks good to me.

Thanks again.

Regards,
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/07/03 2:15, Dean Rasheed wrote:

On 30 June 2017 at 10:04, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:

On Fri, Jun 30, 2017 at 1:36 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Alright, I spent some time implementing a patch to allow specifying
-infinity and +infinity in arbitrary ways. Of course, it prevents
nonsensical inputs with appropriate error messages.

I don't think -infinity and +infinity are the right terms. For a
string or character data type there is no -infinity and +infinity.
Similarly for enums. We need to extend UNBOUNDED somehow to indicate
the end of a given type in the given direction. I thought about
UNBOUNDED LEFT/RIGHT but then whether LEFT indicates -ve side or +side
would cause confusion. Also LEFT/RIGHT may work for a single
dimensional datatype but not for multi-dimensional spaces. How about
MINIMUM/MAXIMUM or UNBOUNDED MIN/MAX to indicate the extremities.

Yes, I think you're right. Also, some datatypes include values that
are equal to +/-infinity, which would then behave differently from
unbounded as range bounds, so it wouldn't be a good idea to overload
that term.

Agree with you both that using (+/-) infinity may not be a good idea after
all.

My first thought was UNBOUNDED ABOVE/BELOW, because that matches the
terminology already in use of upper and lower bounds.

I was starting to like the Ashutosh's suggested UNBOUNDED MIN/MAX syntax,
but could you clarify your comment that ABOVE/BELOW is the terminology
already in use of upper and lower bounds? I couldn't find ABOVE/BELOW in
our existing syntax anywhere that uses the upper/lower bound notion, so
was confused a little bit.

Also, I assume UNBOUNDED ABOVE signifies positive infinity and vice versa.

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/07/03 14:00, Amit Langote wrote:

On 2017/07/03 2:15, Dean Rasheed wrote:

On 30 June 2017 at 10:04, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:

On Fri, Jun 30, 2017 at 1:36 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Alright, I spent some time implementing a patch to allow specifying
-infinity and +infinity in arbitrary ways. Of course, it prevents
nonsensical inputs with appropriate error messages.

I don't think -infinity and +infinity are the right terms. For a
string or character data type there is no -infinity and +infinity.
Similarly for enums. We need to extend UNBOUNDED somehow to indicate
the end of a given type in the given direction. I thought about
UNBOUNDED LEFT/RIGHT but then whether LEFT indicates -ve side or +side
would cause confusion. Also LEFT/RIGHT may work for a single
dimensional datatype but not for multi-dimensional spaces. How about
MINIMUM/MAXIMUM or UNBOUNDED MIN/MAX to indicate the extremities.

Yes, I think you're right. Also, some datatypes include values that
are equal to +/-infinity, which would then behave differently from
unbounded as range bounds, so it wouldn't be a good idea to overload
that term.

Agree with you both that using (+/-) infinity may not be a good idea after
all.

My first thought was UNBOUNDED ABOVE/BELOW, because that matches the
terminology already in use of upper and lower bounds.

I was starting to like the Ashutosh's suggested UNBOUNDED MIN/MAX syntax,
but could you clarify your comment that ABOVE/BELOW is the terminology
already in use of upper and lower bounds? I couldn't find ABOVE/BELOW in
our existing syntax anywhere that uses the upper/lower bound notion, so
was confused a little bit.

Also, I assume UNBOUNDED ABOVE signifies positive infinity and vice versa.

Anyway, here's the revised version of the syntax patch that implements
ABOVE/BELOW extension to UNBOUNDED specification.

0001 is the patch that Dean posted [1]/messages/by-id/CAEZATCVcBCBZsMcHj37TF+dcsjCtKZdZ_FAaJjaFMvfoXRqZMg@mail.gmail.com as a replacement for what I earlier
posted for simplifying range partition overlap check.

0002 is the UNBOUNDED syntax extension patch.

Thanks,
Amit

[1]: /messages/by-id/CAEZATCVcBCBZsMcHj37TF+dcsjCtKZdZ_FAaJjaFMvfoXRqZMg@mail.gmail.com
/messages/by-id/CAEZATCVcBCBZsMcHj37TF+dcsjCtKZdZ_FAaJjaFMvfoXRqZMg@mail.gmail.com

On 3 July 2017 at 06:00, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2017/07/03 2:15, Dean Rasheed wrote:

My first thought was UNBOUNDED ABOVE/BELOW, because that matches the
terminology already in use of upper and lower bounds.

I was starting to like the Ashutosh's suggested UNBOUNDED MIN/MAX syntax,
but could you clarify your comment that ABOVE/BELOW is the terminology
already in use of upper and lower bounds? I couldn't find ABOVE/BELOW in
our existing syntax anywhere that uses the upper/lower bound notion, so
was confused a little bit.

I just meant that the words "above" and "below" more closely match the
already-used terms "upper" and "lower" for the bounds, so that
terminology seemed more consistent, e.g. "UNBOUNDED ABOVE" => no upper
bound.

Also, I assume UNBOUNDED ABOVE signifies positive infinity and vice versa.

Right.

I'm not particularly wedded to that terminology. I always find naming
things hard, so if anyone can think of anything better, let's hear it.

The bigger question is do we want this for PG10? If so, time is
getting tight. My feeling is that we do, because otherwise we'd be
changing the syntax in PG11 of a feature only just released in PG10,
and I think the current syntax is flawed, so it would be better not to
have it in any public release. I'd feel better hearing from the
original committer though.

Meanwhile, I'll continue trying to review the latest patches...

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Jul 3, 2017 at 2:06 PM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

On 3 July 2017 at 06:00, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2017/07/03 2:15, Dean Rasheed wrote:

My first thought was UNBOUNDED ABOVE/BELOW, because that matches the
terminology already in use of upper and lower bounds.

I was starting to like the Ashutosh's suggested UNBOUNDED MIN/MAX syntax,
but could you clarify your comment that ABOVE/BELOW is the terminology
already in use of upper and lower bounds? I couldn't find ABOVE/BELOW in
our existing syntax anywhere that uses the upper/lower bound notion, so
was confused a little bit.

I just meant that the words "above" and "below" more closely match the
already-used terms "upper" and "lower" for the bounds, so that
terminology seemed more consistent, e.g. "UNBOUNDED ABOVE" => no upper
bound.

Also, I assume UNBOUNDED ABOVE signifies positive infinity and vice versa.

Right.

I'm not particularly wedded to that terminology. I always find naming
things hard, so if anyone can think of anything better, let's hear it.

Yet another option: UNBOUNDED UPPER/LOWER.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Dean,

On 2017/07/03 17:36, Dean Rasheed wrote:

On 3 July 2017 at 06:00, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2017/07/03 2:15, Dean Rasheed wrote:

My first thought was UNBOUNDED ABOVE/BELOW, because that matches the
terminology already in use of upper and lower bounds.

I was starting to like the Ashutosh's suggested UNBOUNDED MIN/MAX syntax,
but could you clarify your comment that ABOVE/BELOW is the terminology
already in use of upper and lower bounds? I couldn't find ABOVE/BELOW in
our existing syntax anywhere that uses the upper/lower bound notion, so
was confused a little bit.

I just meant that the words "above" and "below" more closely match the
already-used terms "upper" and "lower" for the bounds, so that
terminology seemed more consistent, e.g. "UNBOUNDED ABOVE" => no upper
bound.

Also, I assume UNBOUNDED ABOVE signifies positive infinity and vice versa.

Right.

I see, thanks for clarifying.

I'm not particularly wedded to that terminology. I always find naming
things hard, so if anyone can think of anything better, let's hear it.

The bigger question is do we want this for PG10? If so, time is
getting tight. My feeling is that we do, because otherwise we'd be
changing the syntax in PG11 of a feature only just released in PG10,
and I think the current syntax is flawed, so it would be better not to
have it in any public release. I'd feel better hearing from the
original committer though.

The way I have extended the syntax in the posted patch, ABOVE/BELOW (or
whatever we decide instead) are optional. UNBOUNDED without the
ABOVE/BELOW specifications implicitly means UNBOUNDED ABOVE if in FROM and
vice versa, which seems to me like sensible default behavior and what's
already present in PG 10.

Do you think ABOVE/BELOW shouldn't really be optional?

Meanwhile, I'll continue trying to review the latest patches...

I had forgotten to update the CREATE TABLE documentation in 0002 to
reflect the syntax extension. Fixed in the attached latest patch.

Thanks,
Amit

On 3 July 2017 at 10:32, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2017/07/03 17:36, Dean Rasheed wrote:

The bigger question is do we want this for PG10? If so, time is
getting tight. My feeling is that we do, because otherwise we'd be
changing the syntax in PG11 of a feature only just released in PG10,
and I think the current syntax is flawed, so it would be better not to
have it in any public release. I'd feel better hearing from the
original committer though.

The way I have extended the syntax in the posted patch, ABOVE/BELOW (or
whatever we decide instead) are optional. UNBOUNDED without the
ABOVE/BELOW specifications implicitly means UNBOUNDED ABOVE if in FROM and
vice versa, which seems to me like sensible default behavior and what's
already present in PG 10.

Do you think ABOVE/BELOW shouldn't really be optional?

Hmm, I'm not so sure about that.

The more I think about this, the more I think that the current design
is broken, and that introducing UNBOUNDED ABOVE/BELOW is just a
sticking plaster to cover that up. Yes, it allows nicer multi-column
ranges to be defined, as demonstrated upthread. But, it also allows
some pretty counterintuitive things like making the lower bound
exclusive and the upper bound inclusive.

I think that's actually the real problem with the current design. If I
have a single-column partition like

(col) FROM (x) TO (y)

it's pretty clear that's a simple range, inclusive at the lower end
and exclusive at the upper end:

(x) <= (col) < (y)

If I now make that a 2-column partition, but leave the second column
unbounded:

(col1,col2) FROM (x,UNBOUNDED) TO (y,UNBOUNDED)

my initial expectation would have been for that to mean the same
thing, i.e.,

(x) <= (col1) < (y)

but that only happens if "UNBOUNDED" means negative infinity in both
places. That then starts to give the sort of desirable properties
you'd expect, like using the same expression for the lower bound of
one partition as the upper bound of another makes the two partitions
contiguous.

But of course, that's not exactly a pretty design either, because then
you'd be saying that UNBOUNDED means positive infinity if it's the
upper bound of the first column, and negative infinity if it's the
lower bound of the first column or either bound of any other column.

Another aspect of the current design I don't like is that you have to
keep repeating UNBOUNDED [ABOVE/BELOW], for each of the rest of the
columns in the bound, and anything else is an error. That's a pretty
verbose way of saying "the rest of the columns are unbounded".

So the more I think about this, the more I think that a cleaner design
would be as follows:

1). Don't allow UNBOUNDED, except in the first column, where it can
keep it's current meaning.

2). Allow the partition bounds to have fewer columns than the
partition definition, and have that mean the same as it would have
meant if you were partitioning by that many columns. So, for
example, if you were partitioning by (col1,col2), you'd be allowed
to define a partition like so:

FROM (x) TO (y)

and it would mean

x <= col1 < y

Or you'd be able to define a partition like

FROM (x1,x2) TO (y)

which would mean

(col1 > x1) OR (col1 = x1 AND col2 >= x2) AND col1 < y

3). Don't allow any value after UNBOUNDED (i.e., only specify
UNBOUNDED once in a partition bound).

This design has a few neat properties:

- Lower bounds are always inclusive and upper bounds are always
exclusive.

- If the expression for the lower bound of one partition is the same
as the expression for the upper bound of another, the 2 partitions
are contiguous, making it easy to define a covering set of
partitions.

- It's much easier to understand what a bound of "(x)" means than
"(x,UNBOUNDED [ABOVE/BELOW])"

- It's much less verbose, and there's no needless repetition.

Of course, it's pretty late in the day to be proposing this kind of
redesign, but I fear that if we don't tackle it now, it will just be
harder to deal with in the future.

Actually, a quick, simple hacky implementation might be to just fill
in any omitted values in a partition bound with negative infinity
internally, and when printing a bound, omit any values after an
infinite value. But really, I think we'd want to tidy up the
implementation, and I think a number of things would actually get much
simpler. For example, get_qual_for_range() could simply stop when it
reached the end of the list of values for the bound, and it wouldn't
need to worry about an unbounded value following a bounded one.

Thoughts?

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Dean,

On 2017/07/04 16:49, Dean Rasheed wrote:

On 3 July 2017 at 10:32, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2017/07/03 17:36, Dean Rasheed wrote:

The bigger question is do we want this for PG10? If so, time is
getting tight. My feeling is that we do, because otherwise we'd be
changing the syntax in PG11 of a feature only just released in PG10,
and I think the current syntax is flawed, so it would be better not to
have it in any public release. I'd feel better hearing from the
original committer though.

The way I have extended the syntax in the posted patch, ABOVE/BELOW (or
whatever we decide instead) are optional. UNBOUNDED without the
ABOVE/BELOW specifications implicitly means UNBOUNDED ABOVE if in FROM and
vice versa, which seems to me like sensible default behavior and what's
already present in PG 10.

Do you think ABOVE/BELOW shouldn't really be optional?

Hmm, I'm not so sure about that.

The more I think about this, the more I think that the current design
is broken, and that introducing UNBOUNDED ABOVE/BELOW is just a
sticking plaster to cover that up. Yes, it allows nicer multi-column
ranges to be defined, as demonstrated upthread. But, it also allows
some pretty counterintuitive things like making the lower bound
exclusive and the upper bound inclusive.

Yes, I kind of got that impression from the example, but wasn't able to
reach the same conclusion as yours that it stems from the underlying
design issues; I thought we'd just have to document them as caveats, but
that doesn't really sound nice. Thanks for pointing that out.

I think that's actually the real problem with the current design. If I
have a single-column partition like

(col) FROM (x) TO (y)

it's pretty clear that's a simple range, inclusive at the lower end
and exclusive at the upper end:

(x) <= (col) < (y)

If I now make that a 2-column partition, but leave the second column
unbounded:

(col1,col2) FROM (x,UNBOUNDED) TO (y,UNBOUNDED)

my initial expectation would have been for that to mean the same
thing, i.e.,

(x) <= (col1) < (y)

but that only happens if "UNBOUNDED" means negative infinity in both
places. That then starts to give the sort of desirable properties
you'd expect, like using the same expression for the lower bound of
one partition as the upper bound of another makes the two partitions
contiguous.

But of course, that's not exactly a pretty design either, because then
you'd be saying that UNBOUNDED means positive infinity if it's the
upper bound of the first column, and negative infinity if it's the
lower bound of the first column or either bound of any other column.

Initially, I didn't understand the part where you said FROM (x, UNBOUNDED)
TO (y, UNBOUNDED) would mean the same thing as (x) <= (col1) < (y),
because row comparison logic that underlying multi-column range partition
key comparisons appears to me to contradict the same. But, maybe it's
thinking about the implementation details like this that's clouding my
judgement about the correctness or the intuitiveness of the current design.

Another aspect of the current design I don't like is that you have to
keep repeating UNBOUNDED [ABOVE/BELOW], for each of the rest of the
columns in the bound, and anything else is an error. That's a pretty
verbose way of saying "the rest of the columns are unbounded".

So the more I think about this, the more I think that a cleaner design
would be as follows:

1). Don't allow UNBOUNDED, except in the first column, where it can
keep it's current meaning.

2). Allow the partition bounds to have fewer columns than the
partition definition, and have that mean the same as it would have
meant if you were partitioning by that many columns. So, for
example, if you were partitioning by (col1,col2), you'd be allowed
to define a partition like so:

FROM (x) TO (y)

and it would mean

x <= col1 < y

Or you'd be able to define a partition like

FROM (x1,x2) TO (y)

which would mean

(col1 > x1) OR (col1 = x1 AND col2 >= x2) AND col1 < y

3). Don't allow any value after UNBOUNDED (i.e., only specify
UNBOUNDED once in a partition bound).

I assume we don't need the ability of specifying ABOVE/BELOW in this design.

In retrospect, that sounds like something that was implemented in the
earlier versions of the patch, whereby there was no ability to specify
UNBOUNDED on a per-column basis. So the syntax was:

FROM { (x [, ...]) | UNBOUNDED } TO { (y [, ...]) | UNBOUNDED }

But, it was pointed out to me [1]/messages/by-id/CA+TgmoYJcUTcN7vVgg54GHtffH11JJWYZnfF4KiRxjV-iaACQg@mail.gmail.com that that doesn't address the use case,
for example, where part1 goes up to (10, 10) and part2 goes from (10, 10)
up to (10, unbounded).

The new design will limit the usage of unbounded range partitions at the
tail ends.

This design has a few neat properties:

- Lower bounds are always inclusive and upper bounds are always
exclusive.

- If the expression for the lower bound of one partition is the same
as the expression for the upper bound of another, the 2 partitions
are contiguous, making it easy to define a covering set of
partitions.

- It's much easier to understand what a bound of "(x)" means than
"(x,UNBOUNDED [ABOVE/BELOW])"

- It's much less verbose, and there's no needless repetition.

They all sound good to me.

Of course, it's pretty late in the day to be proposing this kind of
redesign, but I fear that if we don't tackle it now, it will just be
harder to deal with in the future.

Actually, a quick, simple hacky implementation might be to just fill
in any omitted values in a partition bound with negative infinity
internally, and when printing a bound, omit any values after an
infinite value. But really, I think we'd want to tidy up the
implementation, and I think a number of things would actually get much
simpler. For example, get_qual_for_range() could simply stop when it
reached the end of the list of values for the bound, and it wouldn't
need to worry about an unbounded value following a bounded one.

Thoughts?

I cooked up a patch for the "hacky" implementation for now, just as you
described in the above paragraph. Will you be willing to give it a look?
I will also think about the non-hacky way of implementing this.

0001 is your patch to tidy up check_new_partition_bound() (must be
applied before 0002)

0002 is the patch to implement the range partition syntax redesign that
you outlined above

Thanks again.

Regards,
Amit

[1]: /messages/by-id/CA+TgmoYJcUTcN7vVgg54GHtffH11JJWYZnfF4KiRxjV-iaACQg@mail.gmail.com
/messages/by-id/CA+TgmoYJcUTcN7vVgg54GHtffH11JJWYZnfF4KiRxjV-iaACQg@mail.gmail.com

On 5 July 2017 at 10:43, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

So the more I think about this, the more I think that a cleaner design
would be as follows:

1). Don't allow UNBOUNDED, except in the first column, where it can
keep it's current meaning.

2). Allow the partition bounds to have fewer columns than the
partition definition, and have that mean the same as it would have
meant if you were partitioning by that many columns. So, for
example, if you were partitioning by (col1,col2), you'd be allowed
to define a partition like so:

FROM (x) TO (y)

and it would mean

x <= col1 < y

Or you'd be able to define a partition like

FROM (x1,x2) TO (y)

which would mean

(col1 > x1) OR (col1 = x1 AND col2 >= x2) AND col1 < y

3). Don't allow any value after UNBOUNDED (i.e., only specify
UNBOUNDED once in a partition bound).

I assume we don't need the ability of specifying ABOVE/BELOW in this design.

Yes that's right.

In retrospect, that sounds like something that was implemented in the
earlier versions of the patch, whereby there was no ability to specify
UNBOUNDED on a per-column basis. So the syntax was:

FROM { (x [, ...]) | UNBOUNDED } TO { (y [, ...]) | UNBOUNDED }

Yes, that's where I ended up too.

But, it was pointed out to me [1] that that doesn't address the use case,
for example, where part1 goes up to (10, 10) and part2 goes from (10, 10)
up to (10, unbounded).

The new design will limit the usage of unbounded range partitions at the
tail ends.

True, but I don't think that's really a problem. When the first column
is a discrete type, an upper bound of (10, unbounded) can be rewritten
as (11) in the new design. When it's a continuous type, e.g. floating
point, it can no longer be represented, because (10.0, unbounded)
really means (col1 <= 10.0). But we've already decided not to support
anything other than inclusive lower bounds and exclusive upper bounds,
so allowing this upper bound goes against that design choice.

Of course, it's pretty late in the day to be proposing this kind of
redesign, but I fear that if we don't tackle it now, it will just be
harder to deal with in the future.

Actually, a quick, simple hacky implementation might be to just fill
in any omitted values in a partition bound with negative infinity
internally, and when printing a bound, omit any values after an
infinite value. But really, I think we'd want to tidy up the
implementation, and I think a number of things would actually get much
simpler. For example, get_qual_for_range() could simply stop when it
reached the end of the list of values for the bound, and it wouldn't
need to worry about an unbounded value following a bounded one.

Thoughts?

I cooked up a patch for the "hacky" implementation for now, just as you
described in the above paragraph. Will you be willing to give it a look?
I will also think about the non-hacky way of implementing this.

OK, I'll take a look.

Meanwhile, I already had a go at the "non-hacky" implementation (WIP
patch attached). The more I worked on it, the simpler things got,
which I think is a good sign.

Part-way through, I realised that the PartitionRangeDatum Node type is
no longer needed, because each bound value is now necessarily finite,
so the lowerdatums and upperdatums lists in a PartitionBoundSpec can
now be made into lists of Const nodes, making them match the
listdatums field used for LIST partitioning, and then a whole lot of
related code gets simplified.

It needed a little bit more code in partition.c to track individual
bound sizes, but there were a number of other places that could be
simplified, so overall this represents a reduction in the code size
and complexity.

It's not complete (e.g., no doc updates yet), but it passes all the
tests, and so far seems to work as I would expect.

Regards,
Dean

On 5 July 2017 at 10:43, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

In retrospect, that sounds like something that was implemented in the
earlier versions of the patch, whereby there was no ability to specify
UNBOUNDED on a per-column basis. So the syntax was:

FROM { (x [, ...]) | UNBOUNDED } TO { (y [, ...]) | UNBOUNDED }

But, it was pointed out to me [1] that that doesn't address the use case,
for example, where part1 goes up to (10, 10) and part2 goes from (10, 10)
up to (10, unbounded).

[Reading that other thread]

It's a reasonable point that our syntax is quite different from
Oracle's, and doing this takes it even further away, and removes
support for things that they do support.

For the record, Oracle allows things like the following:

DROP TABLE t1;
CREATE TABLE t1 (a NUMBER, b NUMBER, c NUMBER)
PARTITION BY RANGE (a,b,c)
(PARTITION t1p1 VALUES LESS THAN (1,2,3),
PARTITION t1p2 VALUES LESS THAN (2,3,4),
PARTITION t1p3 VALUES LESS THAN (3,MAXVALUE,5),
PARTITION t1p4 VALUES LESS THAN (4,MAXVALUE,6)
);

INSERT INTO t1 VALUES(1,2,3);
INSERT INTO t1 VALUES(2,3,4);
INSERT INTO t1 VALUES(3,4,5);
INSERT INTO t1 VALUES(3.01,4,5);
INSERT INTO t1 VALUES(4,5,10);

COLUMN subobject_name FORMAT a20;
SELECT a, b, c, subobject_name
FROM t1, user_objects o
WHERE o.data_object_id = dbms_rowid.rowid_object(t1.ROWID)
ORDER BY a,b,c;

A B C SUBOBJECT_NAME
---------- ---------- ---------- --------------------
1 2 3 T1P2
2 3 4 T1P3
3 4 5 T1P3
3.01 4 5 T1P4
4 5 10 T1P4

So they use MAXVALUE instead of UNBOUNDED for an upper bound, which is
more explicit. They don't have an equivalent MINVALUE, but it's
arguably not necessary, since the first partition's lower bound is
implicitly unbounded.

With this syntax they don't need to worry about gaps or overlaps
between partitions, which is nice, but arguably less flexible.

They're also more lax about allowing finite values after MAXVALUE, and
they document the fact that any value after a MAXVALUE is ignored.

I don't think their scheme provides any way to define a partition of
the above table that would hold all rows for which a < some value.

So if we were to go for maximum flexibility and compatibility with
Oracle, then perhaps what we would do is more like the original idea
of UNBOUNDED ABOVE/BELOW, except call them MINVALUE and MAXVALUE,
which conveniently are already unreserved keywords, as well as being
much shorter. Plus, we would also relax the constraint about having
finite values after MINVALUE/MAXVALUE.

I think I'll go play around with that idea to see what it looks like
in practice. Your previous patch already does much of that, and is far
less invasive.

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers