Horton

Thread

Commits

  1. After a MINVALUE/MAXVALUE bound, allow only more of the same.

  2. Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition bounds.

  1. pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2017-07-21T08:24:37Z

    Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition bounds.

Previously, UNBOUNDED meant no lower bound when used in the FROM list,
and no upper bound when used in the TO list, which was OK for
single-column range partitioning, but problematic with multiple
columns. For example, an upper bound of (10.0, UNBOUNDED) would not be
collocated with a lower bound of (10.0, UNBOUNDED), thus making it
difficult or impossible to define contiguous multi-column range
partitions in some cases.

Fix this by using MINVALUE and MAXVALUE instead of UNBOUNDED to
represent a partition column that is unbounded below or above
respectively. This syntax removes any ambiguity, and ensures that if
one partition's lower bound equals another partition's upper bound,
then the partitions are contiguous.

Also drop the constraint prohibiting finite values after an unbounded
column, and just document the fact that any values after MINVALUE or
MAXVALUE are ignored. Previously it was necessary to repeat UNBOUNDED
multiple times, which was needlessly verbose.

Note: Forces a post-PG 10 beta2 initdb.

Report by Amul Sul, original patch by Amit Langote with some
additional hacking by me.

Discussion: https://postgr.es/m/CAAJ_b947mowpLdxL3jo3YLKngRjrq9+Ej4ymduQTfYR+8=YAYQ@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/d363d42bb9a4399a0207bd3b371c966e22e06bd3

Modified Files
--------------
doc/src/sgml/ref/create_table.sgml         |  61 +++++++--
src/backend/catalog/partition.c            | 211 ++++++++++++++---------------
src/backend/nodes/copyfuncs.c              |   2 +-
src/backend/nodes/equalfuncs.c             |   2 +-
src/backend/nodes/outfuncs.c               |   2 +-
src/backend/nodes/readfuncs.c              |   2 +-
src/backend/parser/gram.y                  |  16 ++-
src/backend/parser/parse_utilcmd.c         |  34 -----
src/backend/utils/adt/ruleutils.c          |  12 +-
src/include/catalog/catversion.h           |   2 +-
src/include/nodes/parsenodes.h             |  16 ++-
src/test/regress/expected/create_table.out |  41 +++---
src/test/regress/expected/inherit.out      |   6 +-
src/test/regress/expected/insert.out       | 130 +++++++++++++++++-
src/test/regress/sql/create_table.sql      |  31 ++---
src/test/regress/sql/inherit.sql           |   6 +-
src/test/regress/sql/insert.sql            |  39 +++++-
src/tools/pgindent/typedefs.list           |   1 +
18 files changed, 377 insertions(+), 237 deletions(-)

  2. Re: pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2017-07-21T08:36:36Z

    On 21 July 2017 at 09:24, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition bounds.
>

Hmm, looks like the buildfarm doesn't like this.

It looks like the order of partitions listed by \d+ isn't entirely
predictable ... looking into it now.

Regards,
Dean

  3. Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Robert Haas <robertmhaas@gmail.com> — 2017-08-08T18:22:36Z

    On Fri, Jul 21, 2017 at 4:24 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> Also drop the constraint prohibiting finite values after an unbounded
> column, and just document the fact that any values after MINVALUE or
> MAXVALUE are ignored. Previously it was necessary to repeat UNBOUNDED
> multiple times, which was needlessly verbose.

I would like to (belatedly) complain about this part of this commit.
Now you can do stuff like this:

rhaas=# create table foo (a int, b text) partition by range (a, b);
CREATE TABLE
rhaas=# create table foo1 partition of foo for values from (minvalue,
0) to (maxvalue, 0);
CREATE TABLE

The inclusion of 0 has no effect, but it is still stored in the
catalog, shows up in \d foo1, and somebody might think it does
something.  I think we should go back to requiring bounds after
minvalue or maxvalue to be minvalue or maxvalue.  I thought maybe the
idea here was that you were going to allow something like this to
work, which actually would have saved some typing:

create table foo2 partition of foo for values from (minvalue) to (maxvalue);

But no:

ERROR:  FROM must specify exactly one value per partitioning column

So the only way that this has made things less verbose is by letting
you put in a meaningless value of the data type which takes fewer than
8 characters to type.  That doesn't seem to me to be a defensible way
of reducing verbosity.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  4. Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2017-08-08T23:33:10Z

    On 8 August 2017 at 19:22, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Jul 21, 2017 at 4:24 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>> Also drop the constraint prohibiting finite values after an unbounded
>> column, and just document the fact that any values after MINVALUE or
>> MAXVALUE are ignored. Previously it was necessary to repeat UNBOUNDED
>> multiple times, which was needlessly verbose.
>
> I would like to (belatedly) complain about this part of this commit.
> Now you can do stuff like this:
>
> rhaas=# create table foo (a int, b text) partition by range (a, b);
> CREATE TABLE
> rhaas=# create table foo1 partition of foo for values from (minvalue,
> 0) to (maxvalue, 0);
> CREATE TABLE
>
> The inclusion of 0 has no effect, but it is still stored in the
> catalog, shows up in \d foo1, and somebody might think it does
> something.  I think we should go back to requiring bounds after
> minvalue or maxvalue to be minvalue or maxvalue.  I thought maybe the
> idea here was that you were going to allow something like this to
> work, which actually would have saved some typing:
>
> create table foo2 partition of foo for values from (minvalue) to (maxvalue);
>
> But no:
>
> ERROR:  FROM must specify exactly one value per partitioning column
>
> So the only way that this has made things less verbose is by letting
> you put in a meaningless value of the data type which takes fewer than
> 8 characters to type.  That doesn't seem to me to be a defensible way
> of reducing verbosity.
>

Well perhaps verbosity-reduction isn't sufficient justification but I
still think this is correct because logically any values in the bound
after MINVALUE/MAXVALUE are irrelevant, so it seems overly restrictive
to force all later values to be MINVALUE/MAXVALUE when the code will
just ignore those values.

I don't think we should allow values after MINVALUE/MAXVALUE to be
omitted altogether because we document multi-column ranges as being
most easily understood according to the rules of row-wise comparisons,
and they require equal numbers of values in each row.

It's also worth noting that this choice is consistent with other
databases, so at least some people will be used to being able to do
this.

Regards,
Dean

  5. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    David G. Johnston <david.g.johnston@gmail.com> — 2017-08-09T00:03:08Z

    On Tue, Aug 8, 2017 at 4:33 PM, Dean Rasheed <dean.a.rasheed@gmail.com>
wrote:

> On 8 August 2017 at 19:22, Robert Haas <robertmhaas@gmail.com> wrote:
> > On Fri, Jul 21, 2017 at 4:24 AM, Dean Rasheed <dean.a.rasheed@gmail.com>
> wrote:
> >> Also drop the constraint prohibiting finite values after an unbounded
> >> column, and just document the fact that any values after MINVALUE or
> >> MAXVALUE are ignored. Previously it was necessary to repeat UNBOUNDED
> >> multiple times, which was needlessly verbose.
> >
> > I would like to (belatedly) complain about this part of this commit.
> > Now you can do stuff like this:
> >
> > rhaas=# create table foo (a int, b text) partition by range (a, b);
> > CREATE TABLE
> > rhaas=# create table foo1 partition of foo for values from (minvalue,
> > 0) to (maxvalue, 0);
> > CREATE TABLE
> >
> > The inclusion of 0 has no effect, but it is still stored in the
> > catalog, shows up in \d foo1, and somebody might think it does
> > something.  I think we should go back to requiring bounds after
> > minvalue or maxvalue to be minvalue or maxvalue.
>

​The commit message indicates one would just specify "unbounded", not
min/maxvalue​, which indeed seems to be a better word for it.

Can we, presently, stick "null" in place of the "0"?

Well perhaps verbosity-reduction isn't sufficient justification but I
> still think this is correct because logically any values in the bound
> after MINVALUE/MAXVALUE are irrelevant, so it seems overly restrictive
> to force all later values to be MINVALUE/MAXVALUE when the code will
> just ignore those values.
>
>
But semantically using
​unbounded
 is correct - and referencing the principle of "write once, read many"
people are probably going to care a lot more about the \d display than the
original SQL - and \d showing some randomly chosen value and mentally doing
the gymnastics to think "oh, wait, it doesn't actually mater what this
value is)" compared to it showing "unbounded" and it being obvious that
"any value" will work, seems like a win.


> I don't think we should allow values after MINVALUE/MAXVALUE to be
> omitted altogether because we document multi-column ranges as being
> most easily understood according to the rules of row-wise comparisons,
> and they require equal numbers of values in each row.
>

I wouldn't change the underlying representation but from a UX perspective
being able to ?omit the explicit specification and let the system default
appropriately would be worth considering - though probably not worth the
effort.

​The complaint regarding \d could be solved by figuring out on-the-fly
whether the particular column is presently bounded or not and diplaying
"unbounded" for the later ones regardless of what value is stored in the
catalog.  "Unbounded (0)" would communicate both aspects.​  In the "be
liberal in what you accept" department that would seem to be a win.

​David J.​

  6. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2017-08-09T01:32:19Z

    On 2017/08/09 9:03, David G. Johnston wrote:
> On Tue, Aug 8, 2017 at 4:33 PM, Dean Rasheed wrote:
>> Well perhaps verbosity-reduction isn't sufficient justification but I
>> still think this is correct because logically any values in the bound
>> after MINVALUE/MAXVALUE are irrelevant, so it seems overly restrictive
>> to force all later values to be MINVALUE/MAXVALUE when the code will
>> just ignore those values.
>>
>>
> But semantically using ​unbounded
> is correct - and referencing the principle of "write once, read many"
> people are probably going to care a lot more about the \d display than the
> original SQL - and \d showing some randomly chosen value and mentally doing
> the gymnastics to think "oh, wait, it doesn't actually mater what this
> value is)" compared to it showing "unbounded" and it being obvious that
> "any value" will work, seems like a win.
> 
>> I don't think we should allow values after MINVALUE/MAXVALUE to be
>> omitted altogether because we document multi-column ranges as being
>> most easily understood according to the rules of row-wise comparisons,
>> and they require equal numbers of values in each row.
>>
> 
> I wouldn't change the underlying representation but from a UX perspective
> being able to ?omit the explicit specification and let the system default
> appropriately would be worth considering - though probably not worth the
> effort.
> 
> ​The complaint regarding \d could be solved by figuring out on-the-fly
> whether the particular column is presently bounded or not and diplaying
> "unbounded" for the later ones regardless of what value is stored in the
> catalog.  "Unbounded (0)" would communicate both aspects.​  In the "be
> liberal in what you accept" department that would seem to be a win.

One of the patches posted on the thread where this development occurred
[1] implemented more or less this behavior.  That is, we would let the
users omit inconsequential values in the FROM and TO lists, but internally
store minvalue in the columns for which no value was specified. \d could
show those values as the catalog had it (minvalue).

Consider following example with the current syntax:

create table rp (a int, b int) partition by range (a, b);
create table rp0 partition of rp for values from (minvalue, minvalue) to
(1, minvalue);

\d+ rp0 shows:

Partition of: rp FOR VALUES FROM (MINVALUE, MINVALUE) TO (1, MINVALUE)
Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND (a < 1))

With the aforementioned patch, the same partition could be created as:

create table rp0 partition of rp for values from (minvalue) to (1, minvalue);

or:

create table rp0 partition of rp for values from (minvalue) to (1);

Consider one more partition:

create table rp1 partition of rp for values from (1, minvalue) to (1, 1);

\d+ rp1 shows:

Partition of: rp FOR VALUES FROM (1, MINVALUE) TO (1, 1)
Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND (a = 1) AND
(b < 1))

Same could be created with following alternative command:

create table rp1 partition of rp for values from (1) to (1, 1);

Regarding Dean's argument that it's hard to make sense of that syntax when
one considers that row-comparison logic is used which requires equal
number of columns to be present on both sides, since users are always able
to reveal what ROW expression looks like internally by describing a
partition, they can see what values are being used in the row comparisons,
including those of the columns for which they didn't specify any value
when creating the table.

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/a6d6b752-3d08-ded8-3c8f-5cc9f090ec20%40lab.ntt.co.jp

  7. Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Robert Haas <robertmhaas@gmail.com> — 2017-08-09T12:14:23Z

    On Tue, Aug 8, 2017 at 7:33 PM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> Well perhaps verbosity-reduction isn't sufficient justification but I
> still think this is correct because logically any values in the bound
> after MINVALUE/MAXVALUE are irrelevant, so it seems overly restrictive
> to force all later values to be MINVALUE/MAXVALUE when the code will
> just ignore those values.

I just don't understand why you think there should be multiple
spellings of the same bound.  Generally, canonicalization is good.
One of my fears here is that at least some people will get confused
and think a bound from (minvalue, 0) to (maxvalue, 10) allows any
value for the first column and a 0-9 value for the second column which
is wrong.

My other fear is that, since you've not only allowed this into the
syntax but into the catalog, this will become a source of bugs for
years to come.  Every future patch that deals with partition bounds
will now have to worry about testing
unbounded-followed-by-non-unbounded.  If we're not going to put back
those error checks completely - and I think we should - we should at
least canonicalize what actually gets stored.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  8. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2017-09-12T13:58:31Z

    Robert Haas wrote:

> I just don't understand why you think there should be multiple
> spellings of the same bound.  Generally, canonicalization is good.
> One of my fears here is that at least some people will get confused
> and think a bound from (minvalue, 0) to (maxvalue, 10) allows any
> value for the first column and a 0-9 value for the second column which
> is wrong.
> 
> My other fear is that, since you've not only allowed this into the
> syntax but into the catalog, this will become a source of bugs for
> years to come.  Every future patch that deals with partition bounds
> will now have to worry about testing
> unbounded-followed-by-non-unbounded.  If we're not going to put back
> those error checks completely - and I think we should - we should at
> least canonicalize what actually gets stored.

Did anything happen on this, or did we just forget it completely?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  9. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Robert Haas <robertmhaas@gmail.com> — 2017-09-12T14:16:22Z

    On Tue, Sep 12, 2017 at 9:58 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Robert Haas wrote:
>> I just don't understand why you think there should be multiple
>> spellings of the same bound.  Generally, canonicalization is good.
>> One of my fears here is that at least some people will get confused
>> and think a bound from (minvalue, 0) to (maxvalue, 10) allows any
>> value for the first column and a 0-9 value for the second column which
>> is wrong.
>>
>> My other fear is that, since you've not only allowed this into the
>> syntax but into the catalog, this will become a source of bugs for
>> years to come.  Every future patch that deals with partition bounds
>> will now have to worry about testing
>> unbounded-followed-by-non-unbounded.  If we're not going to put back
>> those error checks completely - and I think we should - we should at
>> least canonicalize what actually gets stored.
>
> Did anything happen on this, or did we just forget it completely?

I forgot it.  :-(

I really think we should fix this.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  10. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-12T15:37:59Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Sep 12, 2017 at 9:58 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> Did anything happen on this, or did we just forget it completely?

> I forgot it.  :-(

> I really think we should fix this.

+1.  You've got the rest of the week ...

			regards, tom lane

  11. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2017-09-13T08:51:12Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Sep 12, 2017 at 9:58 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> Did anything happen on this, or did we just forget it completely?
>
> I forgot it.  :-(
>
> I really think we should fix this.

Ah, sorry. This was for me to follow up, and I dropped the ball.

Here's a patch restoring the original error checks (actually not the
same coding as used in previous versions of the patch, because that
would have allowed a MINVALUE after a MAXVALUE and vice versa).

A drawback to doing this is that we lose compatibility with syntaxes
supported by other databases, which was part of the reason for
choosing the terms MINVALUE and MAXVALUE in the first place.

So thinking about this afresh, my preference would actually be to just
canonicalise the values stored rather than erroring out.

Thoughts?

Regards,
Dean

  12. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2017-09-13T09:05:30Z

    Hi Dean,

On 2017/09/13 17:51, Dean Rasheed wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Tue, Sep 12, 2017 at 9:58 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>>> Did anything happen on this, or did we just forget it completely?
>>
>> I forgot it.  :-(
>>
>> I really think we should fix this.
> 
> Ah, sorry. This was for me to follow up, and I dropped the ball.
> 
> Here's a patch restoring the original error checks (actually not the
> same coding as used in previous versions of the patch, because that
> would have allowed a MINVALUE after a MAXVALUE and vice versa).
> 
> A drawback to doing this is that we lose compatibility with syntaxes
> supported by other databases, which was part of the reason for
> choosing the terms MINVALUE and MAXVALUE in the first place.
> 
> So thinking about this afresh, my preference would actually be to just
> canonicalise the values stored rather than erroring out.
> 
> Thoughts?

Coincidentally, I just wrote the patch for canonicalizing stored values,
instead of erroring out.  Please see attached if that's what you were
thinking too.

Thanks,
Amit

  13. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Robert Haas <robertmhaas@gmail.com> — 2017-09-13T13:51:19Z

    On Wed, Sep 13, 2017 at 5:05 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> So thinking about this afresh, my preference would actually be to just
>> canonicalise the values stored rather than erroring out.
>
> Coincidentally, I just wrote the patch for canonicalizing stored values,
> instead of erroring out.  Please see attached if that's what you were
> thinking too.

Coincidentally, I wrote a patch for this too, but mine goes back to
rejecting MINVALUE or MAXVALUE followed by anything else.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  14. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Robert Haas <robertmhaas@gmail.com> — 2017-09-13T13:53:10Z

    On Wed, Sep 13, 2017 at 4:51 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> A drawback to doing this is that we lose compatibility with syntaxes
> supported by other databases, which was part of the reason for
> choosing the terms MINVALUE and MAXVALUE in the first place.
>
> So thinking about this afresh, my preference would actually be to just
> canonicalise the values stored rather than erroring out.

Can you be more specific about what other databases do here?  Which
other systems support MINVALUE/MAXVALUE, and what are their respective
behaviors in this situation?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  15. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2017-09-13T14:54:48Z

    On 13 September 2017 at 14:53, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Sep 13, 2017 at 4:51 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>> A drawback to doing this is that we lose compatibility with syntaxes
>> supported by other databases, which was part of the reason for
>> choosing the terms MINVALUE and MAXVALUE in the first place.
>>
>> So thinking about this afresh, my preference would actually be to just
>> canonicalise the values stored rather than erroring out.
>
> Can you be more specific about what other databases do here?  Which
> other systems support MINVALUE/MAXVALUE, and what are their respective
> behaviors in this situation?
>

Oracle, MySQL and DB2 all use MINVALUE/MAXVALUE. Actually, Oracle and
MySQL only use MAXVALUE, not MINVALUE, because they don't allow gaps
between partitions and the first partition implicitly starts at
MINVALUE, so the bounds that we currently support are a strict
superset of those supported by Oracle and MySQL.

Both Oracle and MySQL allow finite values after MAXVALUE (usually
listed as "0" in code examples, e.g. see [1]). Oracle explicitly
documents the fact that values after MAXVALUE are irrelevant in [1].
I'm not sure if MySQL explicitly documents that, but it does behave
the same.

Also, both Oracle and MySQL store what the user entered (they do not
canonicalise), as can be seen by looking at ALL_TAB_PARTITIONS in
Oracle, or "show create table" in MySQL.

I have not used DB2.

Regards,
Dean

[1] https://docs.oracle.com/cd/E18283_01/server.112/e16541/part_admin001.htm

  16. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Robert Haas <robertmhaas@gmail.com> — 2017-09-13T16:06:40Z

    On Wed, Sep 13, 2017 at 10:54 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> Oracle, MySQL and DB2 all use MINVALUE/MAXVALUE. Actually, Oracle and
> MySQL only use MAXVALUE, not MINVALUE, because they don't allow gaps
> between partitions and the first partition implicitly starts at
> MINVALUE, so the bounds that we currently support are a strict
> superset of those supported by Oracle and MySQL.
>
> Both Oracle and MySQL allow finite values after MAXVALUE (usually
> listed as "0" in code examples, e.g. see [1]). Oracle explicitly
> documents the fact that values after MAXVALUE are irrelevant in [1].
> I'm not sure if MySQL explicitly documents that, but it does behave
> the same.
>
> Also, both Oracle and MySQL store what the user entered (they do not
> canonicalise), as can be seen by looking at ALL_TAB_PARTITIONS in
> Oracle, or "show create table" in MySQL.

OK, thanks.  I still don't really like allowing this, but I can see
that compatibility with other systems has some value here, and if
nobody else is rejecting these cases, maybe we shouldn't either.  So
I'll hold my nose and change my vote to canonicalizing rather than
rejecting outright.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  17. Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Noah Misch <noah@leadboat.com> — 2017-09-14T02:49:54Z

    On Wed, Sep 13, 2017 at 12:06:40PM -0400, Robert Haas wrote:
> On Wed, Sep 13, 2017 at 10:54 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> > Oracle, MySQL and DB2 all use MINVALUE/MAXVALUE. Actually, Oracle and
> > MySQL only use MAXVALUE, not MINVALUE, because they don't allow gaps
> > between partitions and the first partition implicitly starts at
> > MINVALUE, so the bounds that we currently support are a strict
> > superset of those supported by Oracle and MySQL.
> >
> > Both Oracle and MySQL allow finite values after MAXVALUE (usually
> > listed as "0" in code examples, e.g. see [1]). Oracle explicitly
> > documents the fact that values after MAXVALUE are irrelevant in [1].
> > I'm not sure if MySQL explicitly documents that, but it does behave
> > the same.
> >
> > Also, both Oracle and MySQL store what the user entered (they do not
> > canonicalise), as can be seen by looking at ALL_TAB_PARTITIONS in
> > Oracle, or "show create table" in MySQL.
> 
> OK, thanks.  I still don't really like allowing this, but I can see
> that compatibility with other systems has some value here, and if
> nobody else is rejecting these cases, maybe we shouldn't either.  So
> I'll hold my nose and change my vote to canonicalizing rather than
> rejecting outright.

I vote for rejecting it.  DDL compatibility is less valuable than other
compatibility.  The hypothetical affected application can change its DDL to
placate PostgreSQL and use that modified DDL for all other databases, too.

  18. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2017-09-14T07:53:49Z

    On 13 September 2017 at 10:05, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Coincidentally, I just wrote the patch for canonicalizing stored values,
> instead of erroring out.  Please see attached if that's what you were
> thinking too.
>

Looks reasonable to me, if we decide to go this way.

One minor review comment -- it isn't really necessary to have the
separate new boolean local variables as well as the datum kind
variables. Just tracking the datum kind is sufficient and slightly
simpler. That would also address a slight worry I have that your
coding might result in a compiler warning about the kind variables
being used uninitialised with some less intelligent compilers, not
smart enough to work out that it can't actually happen.

Regards,
Dean

  19. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2017-09-14T07:54:34Z

    On 13 September 2017 at 14:51, Robert Haas <robertmhaas@gmail.com> wrote:
> Coincidentally, I wrote a patch for this too, but mine goes back to
> rejecting MINVALUE or MAXVALUE followed by anything else.
>

LGTM, if we decide to go this way.

One minor review comment -- you missed an example code snippet using
(MINVALUE, 0) further down the page in create_table.sgml, which needs
updating.

Regards,
Dean

  20. Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2017-09-14T08:01:46Z

    On 14 September 2017 at 03:49, Noah Misch <noah@leadboat.com> wrote:
> On Wed, Sep 13, 2017 at 12:06:40PM -0400, Robert Haas wrote:
>> OK, thanks.  I still don't really like allowing this, but I can see
>> that compatibility with other systems has some value here, and if
>> nobody else is rejecting these cases, maybe we shouldn't either.  So
>> I'll hold my nose and change my vote to canonicalizing rather than
>> rejecting outright.
>
> I vote for rejecting it.  DDL compatibility is less valuable than other
> compatibility.  The hypothetical affected application can change its DDL to
> placate PostgreSQL and use that modified DDL for all other databases, too.

So we have 3 options:

1. Do nothing, allowing and keeping any values after a MINVALUE/MAXVALUE.

2. Allow the such values, but canonicalise what we store.

3. Reject anything other than MINVALUE/MAXVALUE after MINVALUE/MAXVALUE.


My order of preference is still (1), (2) then (3) because:

- Compatibility.
- Less verbose / easier to type.
- We allow multiple syntaxes for equivalent constraints in other places,
  without canonicalising what the user enters.
- Regarding Robert's coding argument, code in general should not be
  looking at and basing decisions on any values it sees after a
  MINVALUE/MAXVALUE. If it does, at the very least it's doing more work
  than it needs to, and at worst, there's a potential bug there which
  would be more readily exposed by allowing arbitrary values there. Code
  that only worked because because of earlier canonicalisation would
  strike me as being somewhat fragile.

However, it seems that there is a clear consensus towards (2) or (3)
and we have viable patches for each, although I'm not sure which of
those the consensus is really leaning towards.

Robert, since partitioning was originally your commit, and you know
the wider codebase better, I'm happy to go with whatever you decide.

Regards,
Dean

  21. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2017-09-14T09:01:36Z

    On 2017/09/14 16:53, Dean Rasheed wrote:
> On 13 September 2017 at 10:05, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> Coincidentally, I just wrote the patch for canonicalizing stored values,
>> instead of erroring out.  Please see attached if that's what you were
>> thinking too.
>>
> 
> Looks reasonable to me, if we decide to go this way.>
> One minor review comment --

Thanks for the review.

> it isn't really necessary to have the
> separate new boolean local variables as well as the datum kind
> variables. Just tracking the datum kind is sufficient and slightly
> simpler. That would also address a slight worry I have that your
> coding might result in a compiler warning about the kind variables
> being used uninitialised with some less intelligent compilers, not
> smart enough to work out that it can't actually happen.

Got rid of the boolean variables in the updated patch.

Thanks,
Amit

  22. Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Robert Haas <robertmhaas@gmail.com> — 2017-09-14T15:10:14Z

    On Wed, Sep 13, 2017 at 10:49 PM, Noah Misch <noah@leadboat.com> wrote:
>> > Both Oracle and MySQL allow finite values after MAXVALUE (usually
>> > listed as "0" in code examples, e.g. see [1]). Oracle explicitly
>> > documents the fact that values after MAXVALUE are irrelevant in [1].
>> > I'm not sure if MySQL explicitly documents that, but it does behave
>> > the same.
>> >
>> > Also, both Oracle and MySQL store what the user entered (they do not
>> > canonicalise), as can be seen by looking at ALL_TAB_PARTITIONS in
>> > Oracle, or "show create table" in MySQL.
>>
>> OK, thanks.  I still don't really like allowing this, but I can see
>> that compatibility with other systems has some value here, and if
>> nobody else is rejecting these cases, maybe we shouldn't either.  So
>> I'll hold my nose and change my vote to canonicalizing rather than
>> rejecting outright.
>
> I vote for rejecting it.  DDL compatibility is less valuable than other
> compatibility.  The hypothetical affected application can change its DDL to
> placate PostgreSQL and use that modified DDL for all other databases, too.

OK.  Any other votes?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  23. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Stephen Frost <sfrost@snowman.net> — 2017-09-14T15:41:17Z

    Robert, all,

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, Sep 13, 2017 at 10:49 PM, Noah Misch <noah@leadboat.com> wrote:
> >> > Both Oracle and MySQL allow finite values after MAXVALUE (usually
> >> > listed as "0" in code examples, e.g. see [1]). Oracle explicitly
> >> > documents the fact that values after MAXVALUE are irrelevant in [1].
> >> > I'm not sure if MySQL explicitly documents that, but it does behave
> >> > the same.
> >> >
> >> > Also, both Oracle and MySQL store what the user entered (they do not
> >> > canonicalise), as can be seen by looking at ALL_TAB_PARTITIONS in
> >> > Oracle, or "show create table" in MySQL.
> >>
> >> OK, thanks.  I still don't really like allowing this, but I can see
> >> that compatibility with other systems has some value here, and if
> >> nobody else is rejecting these cases, maybe we shouldn't either.  So
> >> I'll hold my nose and change my vote to canonicalizing rather than
> >> rejecting outright.
> >
> > I vote for rejecting it.  DDL compatibility is less valuable than other
> > compatibility.  The hypothetical affected application can change its DDL to
> > placate PostgreSQL and use that modified DDL for all other databases, too.
> 
> OK.  Any other votes?

I think I side with Noah on this one.  I agree that DDL compatibility is
less valuable than other compatibility.  I had also been thinking that
perhaps it would be good to still be compatible for the sake of DBAs not
being confused if they got an error, but this seems straight-forward
enough that it wouldn't take much for a DBA who is building such
partitions to understand their mistake and to fix it.

I haven't been as close to this as others, so perhaps my vote is only
0.5 on this specific case, but that's my feeling on it.

Also, I don't think we should be adding compatibility for the sake of
compatibility alone.  If there's more than one way to do something and
they're all correct and reasonable, then I could see us choosing the
route that matches what others in the industry do, but I don't see
simply ignoring user input in this specific case as really correct and
therefore it's better to reject it.

Basically, for my 2c, the reason Oracle does this is something more of a
historical artifact than because it was deemed sensible, and everyone
else just followed suit, but I don't believe we need to or should.

Thanks!

Stephen

  24. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    David G. Johnston <david.g.johnston@gmail.com> — 2017-09-14T16:59:46Z

    On Thu, Sep 14, 2017 at 8:41 AM, Stephen Frost <sfrost@snowman.net> wrote:

> Robert, all,
>
> * Robert Haas (robertmhaas@gmail.com) wrote:
>
> > >
> > > I vote for rejecting it.  DDL compatibility is less valuable than other
> > > compatibility.  The hypothetical affected application can change its
> DDL to
> > > placate PostgreSQL and use that modified DDL for all other databases,
> too.
> >
> > OK.  Any other votes?
>
> I haven't been as close to this as others, so perhaps my vote is only
> 0.5 on this specific case, but that's my feeling on it.
>

I think we are being consistent as a project by enforcing strictness of
input in this situation so I'll toss my +0.5/+1​ here as well.

David J.

  25. Re: Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b

    Robert Haas <robertmhaas@gmail.com> — 2017-09-16T01:35:43Z

    On Thu, Sep 14, 2017 at 12:59 PM, David G. Johnston
<david.g.johnston@gmail.com> wrote:
> I think we are being consistent as a project by enforcing strictness of
> input in this situation so I'll toss my +0.5/+1 here as well.

All right, since all three new votes are going the same direction with
this, committed the patch for that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company