On 2017/09/14 16:53, Dean Rasheed wrote:
> On 13 September 2017 at 10:05, Amit Langote
> <langote_amit...@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
From 61bfaeb46623572f5adba0b624d00dfecb6ed495 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 | 20 +++++++++++++++++---
src/test/regress/expected/insert.out | 8 ++++----
src/test/regress/sql/create_table.sql | 6 ++++++
5 files changed, 60 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..ca983105cc 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -3381,6 +3381,8 @@ transformPartitionBound(ParseState *pstate, Relation
parent,
*cell2;
int i,
j;
+ PartitionRangeDatumKind lower_kind =
PARTITION_RANGE_DATUM_VALUE,
+
upper_kind = PARTITION_RANGE_DATUM_VALUE;
if (spec->strategy != PARTITION_STRATEGY_RANGE)
ereport(ERROR,
@@ -3426,7 +3428,17 @@ transformPartitionBound(ParseState *pstate, Relation
parent,
coltype = get_partition_col_typid(key, i);
coltypmod = get_partition_col_typmod(key, i);
- if (ldatum->value)
+ /*
+ * If we've found the first infinite bound, use the
same for
+ * subsequent columns.
+ */
+ if (lower_kind != PARTITION_RANGE_DATUM_VALUE)
+ {
+ ldatum = copyObject(ldatum); /* don't
scribble on input */
+ ldatum->kind = lower_kind;
+ ldatum->value = NULL;
+ }
+ else if (ldatum->value)
{
con = castNode(A_Const, ldatum->value);
value = transformPartitionBoundValue(pstate,
con,
@@ -3439,8 +3451,20 @@ transformPartitionBound(ParseState *pstate, Relation
parent,
ldatum = copyObject(ldatum); /* don't
scribble on input */
ldatum->value = (Node *) value;
}
+ else
+ lower_kind = ldatum->kind;
- if (rdatum->value)
+ /*
+ * If we've found the first infinite bound, use the
same for
+ * subsequent columns.
+ */
+ if (upper_kind != PARTITION_RANGE_DATUM_VALUE)
+ {
+ rdatum = copyObject(rdatum); /* don't
scribble on input */
+ rdatum->kind = upper_kind;
+ rdatum->value = NULL;
+ }
+ else if (rdatum->value)
{
con = castNode(A_Const, rdatum->value);
value = transformPartitionBoundValue(pstate,
con,
@@ -3453,6 +3477,8 @@ transformPartitionBound(ParseState *pstate, Relation
parent,
rdatum = copyObject(rdatum); /* don't
scribble on input */
rdatum->value = (Node *) value;
}
+ else
+ upper_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..10eff793c9 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;
@@ -790,3 +790,17 @@ SELECT obj_description('parted_col_comment'::regclass);
Partition key: LIST (a)
DROP TABLE parted_col_comment;
+-- Test canonicalization of range partition bounds
+create table mcr (a int, b int, c int) partition by range (a, b, c);
+create table mcr1 partition of mcr for values from (minvalue, 1, maxvalue) to
(1, maxvalue, 0);
+\d mcr1
+ Table "public.mcr1"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+ b | integer | | |
+ c | integer | | |
+Partition of: mcr FOR VALUES FROM (MINVALUE, MINVALUE, MINVALUE) TO (1,
MAXVALUE, MAXVALUE)
+No partition constraint
+
+drop table mcr;
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),
diff --git a/src/test/regress/sql/create_table.sql
b/src/test/regress/sql/create_table.sql
index eeab5d91ff..d333b7183a 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -662,3 +662,9 @@ COMMENT ON COLUMN parted_col_comment.a IS 'Partition key';
SELECT obj_description('parted_col_comment'::regclass);
\d+ parted_col_comment
DROP TABLE parted_col_comment;
+
+-- Test canonicalization of range partition bounds
+create table mcr (a int, b int, c int) partition by range (a, b, c);
+create table mcr1 partition of mcr for values from (minvalue, 1, maxvalue) to
(1, maxvalue, 0);
+\d mcr1
+drop table mcr;
--
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
