pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b
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: /messages/by-id/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(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
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
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 TABLEThe 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 TABLEThe 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.
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]: /messages/by-id/a6d6b752-3d08-ded8-3c8f-5cc9f090ec20@lab.ntt.co.jp
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]: /messages/by-id/a6d6b752-3d08-ded8-3c8f-5cc9f090ec20@lab.ntt.co.jp
/messages/by-id/a6d6b752-3d08-ded8-3c8f-5cc9f090ec20@lab.ntt.co.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
Attachments:
restore-minvalue-maxvalue-error-checks.patchapplication/octet-stream; name=restore-minvalue-maxvalue-error-checks.patchDownload+105-34
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
Attachments:
0001-Canonicalize-catalog-representation-of-range-partiti.patchtext/plain; charset=UTF-8; name=0001-Canonicalize-catalog-representation-of-range-partiti.patchDownload+40-11
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
Attachments:
enforce-minmaxvalue-consistency.patchapplication/octet-stream; name=enforce-minmaxvalue-consistency.patchDownload+101-36
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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]https://docs.oracle.com/cd/E18283_01/server.112/e16541/part_admin001.htm). Oracle explicitly
documents the fact that values after MAXVALUE are irrelevant in [1]https://docs.oracle.com/cd/E18283_01/server.112/e16541/part_admin001.htm.
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers