Hi,
On 2019/03/06 17:27, Michael Paquier wrote:
> On Wed, Mar 06, 2019 at 04:00:42PM +0900, Amit Langote wrote:
>> Thanks for looking at this. Your patch seems better, because it allows us
>> to keep the error message consistent with the message one would get with
>> list-partitioned syntax.
>
> Thanks for confirming. I think that it would be nice as well to add
> more test coverage for such error patterns with all the strategies.
> It would be good to fix that first, so I can take care of that.
I've added some tests to your patch. Also improved the comments a bit.
I noticed another issue with the code -- it's using strcmp() to compare
specified string against "minvalue" and "maxvalue", which causes the
following silly error:
create table q2 partition of q for values from ("MINVALUE") to (maxvalue);
ERROR: column "MINVALUE" does not exist
LINE 1: create table q2 partition of q for values from ("MINVALUE") ...
It should be using pg_strncasecmp().
> Now I don't really find the error "missing FROM-clause entry for
> table" quite convincing when this is applied to a partition bound when
> using a column defined in the relation. Adding more error classes in
> the set of CRERR_NO_RTE would be perhaps nice, still I am not sure how
> elegant it could be made when looking at expressions for partition
> bounds.
Note that this is not just a problem for partition bounds. You can see it
with default expressions too.
create table foo (a int default (aa.a));
ERROR: missing FROM-clause entry for table "aa"
LINE 1: create table foo (a int default (aa.a));
create table foo (a int default (a.a.aa.a.a.a.a.aa));
ERROR: improper qualified name (too many dotted names): a.a.aa.a.a.a.a.aa
LINE 1: create table foo (a int default (a.a.aa.a.a.a.a.aa));
We could make the error message more meaningful depending on the context,
but maybe it'd better be pursue it as a separate project.
Thanks,
Amit
diff --git a/src/backend/parser/parse_utilcmd.c
b/src/backend/parser/parse_utilcmd.c
index a37d1f18be..67b05b8039 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -3746,18 +3746,28 @@ transformPartitionRangeBounds(ParseState *pstate, List
*blist,
ColumnRef *cref = (ColumnRef *) expr;
char *cname = NULL;
+ /*
+ * There should be a single field named "minvalue" or
"maxvalue".
+ */
if (list_length(cref->fields) == 1 &&
IsA(linitial(cref->fields), String))
cname = strVal(linitial(cref->fields));
- Assert(cname != NULL);
- if (strcmp("minvalue", cname) == 0)
+ if (cname == NULL)
+ {
+ /*
+ * ColumnRef is not in the desired
single-field-name form; for
+ * consistency, let transformExpr() report the
error rather
+ * than doing it ourselves.
+ */
+ }
+ else if (pg_strncasecmp(cname, "minvalue", 6) == 0)
{
prd = makeNode(PartitionRangeDatum);
prd->kind = PARTITION_RANGE_DATUM_MINVALUE;
prd->value = NULL;
}
- else if (strcmp("maxvalue", cname) == 0)
+ else if (pg_strncasecmp(cname, "maxvalue", 6) == 0)
{
prd = makeNode(PartitionRangeDatum);
prd->kind = PARTITION_RANGE_DATUM_MAXVALUE;
diff --git a/src/test/regress/expected/create_table.out
b/src/test/regress/expected/create_table.out
index d51e547278..503dca5866 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -709,6 +709,66 @@ ERROR: modulus for hash partition must be a positive
integer
-- remainder must be greater than or equal to zero and less than modulus
CREATE TABLE fail_part PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 8,
REMAINDER 8);
ERROR: remainder for hash partition must be less than modulus
+-- more tests for generalized expression syntax for list/range partition bounds
+CREATE TABLE list_parted_bound_exprs(a int) PARTITION BY LIST (a);
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN ("UNKNOWN");
+ERROR: column "UNKNOWN" does not exist
+LINE 1: ...RTITION OF list_parted_bound_exprs FOR VALUES IN ("UNKNOWN")...
+ ^
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN (a);
+ERROR: cannot use column references in partition bound expression
+LINE 1: ...prs1 PARTITION OF list_parted_bound_exprs FOR VALUES IN (a);
+ ^
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN (foo.a);
+ERROR: missing FROM-clause entry for table "foo"
+LINE 1: ... PARTITION OF list_parted_bound_exprs FOR VALUES IN (foo.a);
+ ^
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN (list_parted_bound_exprs.a);
+ERROR: missing FROM-clause entry for table "list_parted_bound_exprs"
+LINE 1: ...RTITION OF list_parted_bound_exprs FOR VALUES IN (list_parte...
+ ^
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN (list_parted_bound_exprs1.a.a.a.a);
+ERROR: improper qualified name (too many dotted names):
list_parted_bound_exprs1.a.a.a.a
+LINE 1: ...RTITION OF list_parted_bound_exprs FOR VALUES IN (list_parte...
+ ^
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN (list_parted_bound_exprs1.a);
+ERROR: cannot use column references in partition bound expression
+LINE 1: ...RTITION OF list_parted_bound_exprs FOR VALUES IN (list_parte...
+ ^
+CREATE TABLE range_parted_bound_exprs(a int) PARTITION BY RANGE (a);
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO (unknown);
+ERROR: column "unknown" does not exist
+LINE 1: ...F range_parted_bound_exprs FOR VALUES FROM (1) TO (unknown);
+ ^
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO ("a".unknown);
+ERROR: missing FROM-clause entry for table "a"
+LINE 1: ... range_parted_bound_exprs FOR VALUES FROM (1) TO ("a".unknow...
+ ^
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO ("a"."a");
+ERROR: missing FROM-clause entry for table "a"
+LINE 1: ...F range_parted_bound_exprs FOR VALUES FROM (1) TO ("a"."a");
+ ^
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO ("range_parted_bound_exprs1"."a");
+ERROR: cannot use column references in partition bound expression
+LINE 1: ... range_parted_bound_exprs FOR VALUES FROM (1) TO ("range_par...
+ ^
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO ("range_parted_bound_exprs1"."a".a.a);
+ERROR: cross-database references are not implemented:
range_parted_bound_exprs1.a.a.a
+LINE 1: ... range_parted_bound_exprs FOR VALUES FROM (1) TO ("range_par...
+ ^
+-- the following two should work
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM ("minValue") TO (1);
+CREATE TABLE range_parted_bound_exprs2 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO (MAXVALUE);
+\d+ range_parted_bound_exprs
+ Partitioned table "public.range_parted_bound_exprs"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target |
Description
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a | integer | | | | plain | |
+Partition key: RANGE (a)
+Partitions: range_parted_bound_exprs1 FOR VALUES FROM (MINVALUE) TO (1),
+ range_parted_bound_exprs2 FOR VALUES FROM (1) TO (MAXVALUE)
+
+drop table list_parted_bound_exprs, range_parted_bound_exprs;
-- check schema propagation from parent
CREATE TABLE parted (
a text,
diff --git a/src/test/regress/sql/create_table.sql
b/src/test/regress/sql/create_table.sql
index 4091c19cf0..399a882430 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -627,6 +627,28 @@ CREATE TABLE fail_part PARTITION OF hash_parted2 FOR
VALUES WITH (MODULUS 0, REM
-- remainder must be greater than or equal to zero and less than modulus
CREATE TABLE fail_part PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 8,
REMAINDER 8);
+-- more tests for generalized expression syntax for list/range partition bounds
+CREATE TABLE list_parted_bound_exprs(a int) PARTITION BY LIST (a);
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN ("UNKNOWN");
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN (a);
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN (foo.a);
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN (list_parted_bound_exprs.a);
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN (list_parted_bound_exprs1.a.a.a.a);
+CREATE TABLE list_parted_bound_exprs1 PARTITION OF list_parted_bound_exprs FOR
VALUES IN (list_parted_bound_exprs1.a);
+
+CREATE TABLE range_parted_bound_exprs(a int) PARTITION BY RANGE (a);
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO (unknown);
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO ("a".unknown);
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO ("a"."a");
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO ("range_parted_bound_exprs1"."a");
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO ("range_parted_bound_exprs1"."a".a.a);
+-- the following two should work
+CREATE TABLE range_parted_bound_exprs1 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM ("minValue") TO (1);
+CREATE TABLE range_parted_bound_exprs2 PARTITION OF range_parted_bound_exprs
FOR VALUES FROM (1) TO (MAXVALUE);
+\d+ range_parted_bound_exprs
+
+drop table list_parted_bound_exprs, range_parted_bound_exprs;
+
-- check schema propagation from parent
CREATE TABLE parted (
