Hi Dean, On 2017/09/13 17:51, Dean Rasheed wrote: > Robert Haas <robertmh...@gmail.com> writes: >> On Tue, Sep 12, 2017 at 9:58 AM, Alvaro Herrera <alvhe...@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
From e5fc04c915cb98b0c56b234372ece4b9b1043033 Mon Sep 17 00:00:00 2001 From: amit <amitlangot...@gmail.com> Date: Wed, 13 Sep 2017 17:51:38 +0900 Subject: [PATCH] Canonicalize catalog representation of range partition bounds Since the system effectively ignores values of the bound for partition key columns following the first unbounded key, it's pointless to accurately remember them in the system catalog. Instead store minvalue or maxvalue for all such columns, matching what the first unbounded key is. This makes \d output of range partitions look more canonical. --- doc/src/sgml/ref/create_table.sgml | 6 +++++- src/backend/parser/parse_utilcmd.c | 30 ++++++++++++++++++++++++++++-- src/test/regress/expected/create_table.out | 6 +++--- src/test/regress/expected/insert.out | 8 ++++---- 4 files changed, 40 insertions(+), 10 deletions(-) diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 824253de40..7d2eb30722 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -328,7 +328,11 @@ FROM ( { <replaceable class="PARAMETER">numeric_literal</replaceable> | <replace <literal>MAXVALUE</> in a partition bound are ignored; so the bound <literal>(10, MINVALUE, 0)</> is equivalent to <literal>(10, MINVALUE, 10)</> and <literal>(10, MINVALUE, MINVALUE)</> - and <literal>(10, MINVALUE, MAXVALUE)</>. + and <literal>(10, MINVALUE, MAXVALUE)</>. Morever, the system will + store <literal>(10, MINVALUE, MINVALUE)</> into the system catalog if + the user specifies <literal>(10, MINVALUE, 0)</>, and + <literal>(10, MAXVALUE, MAXVALUE)</> if the user specifies + <literal>(10, MAXVALUE, 0). </para> <para> diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 655da02c10..9086ac90ef 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -3381,6 +3381,10 @@ transformPartitionBound(ParseState *pstate, Relation parent, *cell2; int i, j; + PartitionRangeDatumKind first_lower_unbounded_kind, + first_upper_unbounded_kind; + bool lower_unbounded_found = false, + upper_unbounded_found = false; if (spec->strategy != PARTITION_STRATEGY_RANGE) ereport(ERROR, @@ -3426,7 +3430,13 @@ transformPartitionBound(ParseState *pstate, Relation parent, coltype = get_partition_col_typid(key, i); coltypmod = get_partition_col_typmod(key, i); - if (ldatum->value) + if (lower_unbounded_found) + { + ldatum = copyObject(ldatum); + ldatum->kind = first_lower_unbounded_kind; + ldatum->value = NULL; + } + else if (ldatum->value) { con = castNode(A_Const, ldatum->value); value = transformPartitionBoundValue(pstate, con, @@ -3439,8 +3449,19 @@ transformPartitionBound(ParseState *pstate, Relation parent, ldatum = copyObject(ldatum); /* don't scribble on input */ ldatum->value = (Node *) value; } + else + { + lower_unbounded_found = true; + first_lower_unbounded_kind = ldatum->kind; + } - if (rdatum->value) + if (upper_unbounded_found) + { + rdatum = copyObject(rdatum); + rdatum->kind = first_upper_unbounded_kind; + rdatum->value = NULL; + } + else if (rdatum->value) { con = castNode(A_Const, rdatum->value); value = transformPartitionBoundValue(pstate, con, @@ -3453,6 +3474,11 @@ transformPartitionBound(ParseState *pstate, Relation parent, rdatum = copyObject(rdatum); /* don't scribble on input */ rdatum->value = (Node *) value; } + else + { + upper_unbounded_found = true; + first_upper_unbounded_kind = rdatum->kind; + } result_spec->lowerdatums = lappend(result_spec->lowerdatums, ldatum); diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 58c755be50..e194ed00ad 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -731,7 +731,7 @@ CREATE TABLE unbounded_range_part PARTITION OF range_parted4 FOR VALUES FROM (MI a | integer | | | | plain | | b | integer | | | | plain | | c | integer | | | | plain | | -Partition of: range_parted4 FOR VALUES FROM (MINVALUE, 0, 0) TO (MAXVALUE, 0, 0) +Partition of: range_parted4 FOR VALUES FROM (MINVALUE, MINVALUE, MINVALUE) TO (MAXVALUE, MAXVALUE, MAXVALUE) Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (c IS NOT NULL)) DROP TABLE unbounded_range_part; @@ -743,7 +743,7 @@ CREATE TABLE range_parted4_1 PARTITION OF range_parted4 FOR VALUES FROM (MINVALU a | integer | | | | plain | | b | integer | | | | plain | | c | integer | | | | plain | | -Partition of: range_parted4 FOR VALUES FROM (MINVALUE, 0, 0) TO (1, MAXVALUE, 0) +Partition of: range_parted4 FOR VALUES FROM (MINVALUE, MINVALUE, MINVALUE) TO (1, MAXVALUE, MAXVALUE) Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (c IS NOT NULL) AND (abs(a) <= 1)) CREATE TABLE range_parted4_2 PARTITION OF range_parted4 FOR VALUES FROM (3, 4, 5) TO (6, 7, MAXVALUE); @@ -765,7 +765,7 @@ CREATE TABLE range_parted4_3 PARTITION OF range_parted4 FOR VALUES FROM (6, 8, M a | integer | | | | plain | | b | integer | | | | plain | | c | integer | | | | plain | | -Partition of: range_parted4 FOR VALUES FROM (6, 8, MINVALUE) TO (9, MAXVALUE, 0) +Partition of: range_parted4 FOR VALUES FROM (6, 8, MINVALUE) TO (9, MAXVALUE, MAXVALUE) Partition constraint: ((abs(a) IS NOT NULL) AND (abs(b) IS NOT NULL) AND (c IS NOT NULL) AND ((abs(a) > 6) OR ((abs(a) = 6) AND (abs(b) >= 8))) AND (abs(a) <= 9)) DROP TABLE range_parted4; diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out index 73a5600f19..bd9f7f5b40 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -681,14 +681,14 @@ create table mcrparted8_ge_d partition of mcrparted for values from ('d', minval a | text | | | | extended | | b | integer | | | | plain | | Partition key: RANGE (a, b) -Partitions: mcrparted1_lt_b FOR VALUES FROM (MINVALUE, 0) TO ('b', MINVALUE), +Partitions: mcrparted1_lt_b FOR VALUES FROM (MINVALUE, MINVALUE) TO ('b', MINVALUE), mcrparted2_b FOR VALUES FROM ('b', MINVALUE) TO ('c', MINVALUE), mcrparted3_c_to_common FOR VALUES FROM ('c', MINVALUE) TO ('common', MINVALUE), mcrparted4_common_lt_0 FOR VALUES FROM ('common', MINVALUE) TO ('common', 0), mcrparted5_common_0_to_10 FOR VALUES FROM ('common', 0) TO ('common', 10), mcrparted6_common_ge_10 FOR VALUES FROM ('common', 10) TO ('common', MAXVALUE), mcrparted7_gt_common_lt_d FOR VALUES FROM ('common', MAXVALUE) TO ('d', MINVALUE), - mcrparted8_ge_d FOR VALUES FROM ('d', MINVALUE) TO (MAXVALUE, 0) + mcrparted8_ge_d FOR VALUES FROM ('d', MINVALUE) TO (MAXVALUE, MAXVALUE) \d+ mcrparted1_lt_b Table "public.mcrparted1_lt_b" @@ -696,7 +696,7 @@ Partitions: mcrparted1_lt_b FOR VALUES FROM (MINVALUE, 0) TO ('b', MINVALUE), --------+---------+-----------+----------+---------+----------+--------------+------------- a | text | | | | extended | | b | integer | | | | plain | | -Partition of: mcrparted FOR VALUES FROM (MINVALUE, 0) TO ('b', MINVALUE) +Partition of: mcrparted FOR VALUES FROM (MINVALUE, MINVALUE) TO ('b', MINVALUE) Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND (a < 'b'::text)) \d+ mcrparted2_b @@ -759,7 +759,7 @@ Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND (a > 'common'::te --------+---------+-----------+----------+---------+----------+--------------+------------- a | text | | | | extended | | b | integer | | | | plain | | -Partition of: mcrparted FOR VALUES FROM ('d', MINVALUE) TO (MAXVALUE, 0) +Partition of: mcrparted FOR VALUES FROM ('d', MINVALUE) TO (MAXVALUE, MAXVALUE) Partition constraint: ((a IS NOT NULL) AND (b IS NOT NULL) AND (a >= 'd'::text)) insert into mcrparted values ('aaa', 0), ('b', 0), ('bz', 10), ('c', -10), -- 2.11.0
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers