On Fri, Apr 10, 2020 at 8:37 AM Ashutosh Bapat <
ashutosh.ba...@2ndquadrant.com> wrote:
> for a multi-key value the ^
> points to the first column and the reader may think that that's the
> problematci column. Should it instead point to ( ?
I attached a v2 of Amit's 0002 patch to also report the exact column
for the partition overlap errors.
From f7cf86fcf7c1a26895e68f357717244f776fe5cd Mon Sep 17 00:00:00 2001
From: Alexandra Wang <lew...@pivotal.io>
Date: Fri, 10 Apr 2020 13:51:53 -0700
Subject: [PATCH v2] Improve check new partition bound error position report
---
src/backend/parser/parse_utilcmd.c | 3 ++
src/backend/partitioning/partbounds.c | 60 +++++++++++++++-------
src/test/regress/expected/alter_table.out | 2 +-
src/test/regress/expected/create_table.out | 28 +++++-----
4 files changed, 59 insertions(+), 34 deletions(-)
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 75c122fe34..72113bb7ff 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -4170,5 +4170,8 @@ transformPartitionBoundValue(ParseState *pstate, Node *val,
if (!IsA(value, Const))
elog(ERROR, "could not evaluate partition bound expression");
+ /* Preserve parser location information. */
+ ((Const *) value)->location = exprLocation(val);
+
return (Const *) value;
}
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index dd56832efc..0ef0f6ed07 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -223,7 +223,8 @@ static int32 partition_rbound_cmp(int partnatts, FmgrInfo *partsupfunc,
static int partition_range_bsearch(int partnatts, FmgrInfo *partsupfunc,
Oid *partcollation,
PartitionBoundInfo boundinfo,
- PartitionRangeBound *probe, bool *is_equal);
+ PartitionRangeBound *probe, bool *is_equal,
+ int32 *cmpval);
static int get_partition_bound_num_indexes(PartitionBoundInfo b);
static Expr *make_partition_op_expr(PartitionKey key, int keynum,
uint16 strategy, Expr *arg1, Expr *arg2);
@@ -2810,6 +2811,7 @@ check_new_partition_bound(char *relname, Relation parent,
PartitionBoundInfo boundinfo = partdesc->boundinfo;
int with = -1;
bool overlap = false;
+ int overlap_location = 0;
if (spec->is_default)
{
@@ -2904,6 +2906,7 @@ check_new_partition_bound(char *relname, Relation parent,
if (boundinfo->indexes[remainder] != -1)
{
overlap = true;
+ overlap_location = spec->location;
with = boundinfo->indexes[remainder];
break;
}
@@ -2932,6 +2935,7 @@ check_new_partition_bound(char *relname, Relation parent,
{
Const *val = castNode(Const, lfirst(cell));
+ overlap_location = val->location;
if (!val->constisnull)
{
int offset;
@@ -2965,6 +2969,7 @@ check_new_partition_bound(char *relname, Relation parent,
{
PartitionRangeBound *lower,
*upper;
+ int cmpval;
Assert(spec->strategy == PARTITION_STRATEGY_RANGE);
lower = make_one_partition_rbound(key, -1, spec->lowerdatums, true);
@@ -2974,10 +2979,16 @@ check_new_partition_bound(char *relname, Relation parent,
* First check if the resulting range would be empty with
* specified lower and upper bounds
*/
- if (partition_rbound_cmp(key->partnatts, key->partsupfunc,
- key->partcollation, lower->datums,
- lower->kind, true, upper) >= 0)
+ cmpval = partition_rbound_cmp(key->partnatts,
+ key->partsupfunc,
+ key->partcollation, lower->datums,
+ lower->kind, true, upper);
+ if (cmpval >= 0)
{
+ /* Fetch the problem bound from lower datums list. */
+ PartitionRangeDatum *datum = list_nth(spec->lowerdatums,
+ cmpval - 1);
+
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("empty range bound specified for partition \"%s\"",
@@ -2985,7 +2996,7 @@ check_new_partition_bound(char *relname, Relation parent,
errdetail("Specified lower bound %s is greater than or equal to upper bound %s.",
get_range_partbound_string(spec->lowerdatums),
get_range_partbound_string(spec->upperdatums)),
- parser_errposition(pstate, spec->location)));
+ parser_errposition(pstate, datum->location)));
}
if (partdesc->nparts > 0)
@@ -3017,7 +3028,7 @@ check_new_partition_bound(char *relname, Relation parent,
key->partsupfunc,
key->partcollation,
boundinfo, lower,
- &equal);
+ &equal, &cmpval);
if (boundinfo->indexes[offset + 1] < 0)
{
@@ -3029,7 +3040,6 @@ check_new_partition_bound(char *relname, Relation parent,
*/
if (offset + 1 < boundinfo->ndatums)
{
- int32 cmpval;
Datum *datums;
PartitionRangeDatumKind *kind;
bool is_lower;
@@ -3051,6 +3061,8 @@ check_new_partition_bound(char *relname, Relation parent,
* offset + 2.
*/
overlap = true;
+ overlap_location = ((PartitionRangeDatum *)
+ list_nth(spec->upperdatums, -cmpval - 1))->location;
with = boundinfo->indexes[offset + 2];
}
}
@@ -3061,7 +3073,13 @@ check_new_partition_bound(char *relname, Relation parent,
* The new partition overlaps with the existing
* partition between offset and offset + 1.
*/
+ Datum *datum;
+
overlap = true;
+ Assert(cmpval >= 0);
+ datum = cmpval == 0 ? linitial(spec->lowerdatums):
+ list_nth(spec->lowerdatums, cmpval - 1);
+ overlap_location = ((PartitionRangeDatum *)datum)->location;
with = boundinfo->indexes[offset + 1];
}
}
@@ -3077,11 +3095,12 @@ check_new_partition_bound(char *relname, Relation parent,
if (overlap)
{
Assert(with >= 0);
+ Assert(overlap_location > 0);
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("partition \"%s\" would overlap partition \"%s\"",
relname, get_rel_name(partdesc->oids[with])),
- parser_errposition(pstate, spec->location)));
+ parser_errposition(pstate, overlap_location)));
}
}
@@ -3315,7 +3334,9 @@ make_one_partition_rbound(PartitionKey key, int index, List *datums, bool lower)
* partition_rbound_cmp
*
* Return for two range bounds whether the 1st one (specified in datums1,
- * kind1, and lower1) is <, =, or > the bound specified in *b2.
+ * kind1, and lower1) is <, =, or > the bound specified in *b2. 0 is returned if
+ * equal and the 1-based index of the first mismatching bound if unequal;
+ * multiplied by -1 if the 1st bound is smaller.
*
* partnatts, partsupfunc and partcollation give the number of attributes in the
* bounds to be compared, comparison function to be used and the collations of
@@ -3335,6 +3356,7 @@ partition_rbound_cmp(int partnatts, FmgrInfo *partsupfunc,
bool lower1, PartitionRangeBound *b2)
{
int32 cmpval = 0; /* placate compiler */
+ int result = 0;
int i;
Datum *datums2 = b2->datums;
PartitionRangeDatumKind *kind2 = b2->kind;
@@ -3342,6 +3364,8 @@ partition_rbound_cmp(int partnatts, FmgrInfo *partsupfunc,
for (i = 0; i < partnatts; i++)
{
+ result++;
+
/*
* First, handle cases where the column is unbounded, which should not
* invoke the comparison procedure, and should not consider any later
@@ -3349,9 +3373,9 @@ partition_rbound_cmp(int partnatts, FmgrInfo *partsupfunc,
* compare the same way as the values they represent.
*/
if (kind1[i] < kind2[i])
- return -1;
+ return -result;
else if (kind1[i] > kind2[i])
- return 1;
+ return result;
else if (kind1[i] != PARTITION_RANGE_DATUM_VALUE)
/*
@@ -3376,9 +3400,9 @@ partition_rbound_cmp(int partnatts, FmgrInfo *partsupfunc,
* two.
*/
if (cmpval == 0 && lower1 != lower2)
- cmpval = lower1 ? 1 : -1;
+ cmpval = lower1 ? result : -result;
- return cmpval;
+ return cmpval == 0 ? 0 : (cmpval < 0 ? -result : result);;
}
/*
@@ -3490,7 +3514,7 @@ static int
partition_range_bsearch(int partnatts, FmgrInfo *partsupfunc,
Oid *partcollation,
PartitionBoundInfo boundinfo,
- PartitionRangeBound *probe, bool *is_equal)
+ PartitionRangeBound *probe, bool *is_equal, int32 *cmpval)
{
int lo,
hi,
@@ -3500,19 +3524,17 @@ partition_range_bsearch(int partnatts, FmgrInfo *partsupfunc,
hi = boundinfo->ndatums - 1;
while (lo < hi)
{
- int32 cmpval;
-
mid = (lo + hi + 1) / 2;
- cmpval = partition_rbound_cmp(partnatts, partsupfunc,
+ *cmpval = partition_rbound_cmp(partnatts, partsupfunc,
partcollation,
boundinfo->datums[mid],
boundinfo->kind[mid],
(boundinfo->indexes[mid] == -1),
probe);
- if (cmpval <= 0)
+ if (*cmpval <= 0)
{
lo = mid;
- *is_equal = (cmpval == 0);
+ *is_equal = (*cmpval == 0);
if (*is_equal)
break;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 576f19b38e..b3174694e3 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3793,7 +3793,7 @@ CREATE TABLE fail_part (LIKE part_1 INCLUDING CONSTRAINTS);
ALTER TABLE list_parted ATTACH PARTITION fail_part FOR VALUES IN (1);
ERROR: partition "fail_part" would overlap partition "part_1"
LINE 1: ...LE list_parted ATTACH PARTITION fail_part FOR VALUES IN (1);
- ^
+ ^
DROP TABLE fail_part;
-- check that an existing table can be attached as a default partition
CREATE TABLE def_part (LIKE list_parted INCLUDING CONSTRAINTS);
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index b8de012536..41dce69cc4 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -705,7 +705,7 @@ CREATE TABLE bigintp_10 PARTITION OF bigintp FOR VALUES IN (10);
CREATE TABLE bigintp_10_2 PARTITION OF bigintp FOR VALUES IN ('10');
ERROR: partition "bigintp_10_2" would overlap partition "bigintp_10"
LINE 1: ...ABLE bigintp_10_2 PARTITION OF bigintp FOR VALUES IN ('10');
- ^
+ ^
DROP TABLE bigintp;
CREATE TABLE range_parted (
a date
@@ -828,10 +828,10 @@ CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT;
CREATE TABLE fail_part PARTITION OF list_parted2 FOR VALUES IN (null);
ERROR: partition "fail_part" would overlap partition "part_null_z"
LINE 1: ...LE fail_part PARTITION OF list_parted2 FOR VALUES IN (null);
- ^
+ ^
CREATE TABLE fail_part PARTITION OF list_parted2 FOR VALUES IN ('b', 'c');
ERROR: partition "fail_part" would overlap partition "part_ab"
-LINE 1: ...LE fail_part PARTITION OF list_parted2 FOR VALUES IN ('b', '...
+LINE 1: ...ail_part PARTITION OF list_parted2 FOR VALUES IN ('b', 'c');
^
-- check default partition overlap
INSERT INTO list_parted2 VALUES('X');
@@ -843,35 +843,35 @@ CREATE TABLE range_parted2 (
-- trying to create range partition with empty range
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (1) TO (0);
ERROR: empty range bound specified for partition "fail_part"
-LINE 1: ...E fail_part PARTITION OF range_parted2 FOR VALUES FROM (1) T...
+LINE 1: ..._part PARTITION OF range_parted2 FOR VALUES FROM (1) TO (0);
^
DETAIL: Specified lower bound (1) is greater than or equal to upper bound (0).
-- note that the range '[1, 1)' has no elements
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (1) TO (1);
ERROR: empty range bound specified for partition "fail_part"
-LINE 1: ...E fail_part PARTITION OF range_parted2 FOR VALUES FROM (1) T...
+LINE 1: ..._part PARTITION OF range_parted2 FOR VALUES FROM (1) TO (1);
^
DETAIL: Specified lower bound (1) is greater than or equal to upper bound (1).
CREATE TABLE part0 PARTITION OF range_parted2 FOR VALUES FROM (minvalue) TO (1);
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (minvalue) TO (2);
ERROR: partition "fail_part" would overlap partition "part0"
-LINE 1: ...E fail_part PARTITION OF range_parted2 FOR VALUES FROM (minv...
+LINE 1: ..._part PARTITION OF range_parted2 FOR VALUES FROM (minvalue) ...
^
CREATE TABLE part1 PARTITION OF range_parted2 FOR VALUES FROM (1) TO (10);
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (9) TO (maxvalue);
ERROR: partition "fail_part" would overlap partition "part1"
-LINE 1: ...E fail_part PARTITION OF range_parted2 FOR VALUES FROM (9) T...
+LINE 1: ..._part PARTITION OF range_parted2 FOR VALUES FROM (9) TO (max...
^
CREATE TABLE part2 PARTITION OF range_parted2 FOR VALUES FROM (20) TO (30);
CREATE TABLE part3 PARTITION OF range_parted2 FOR VALUES FROM (30) TO (40);
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (10) TO (30);
ERROR: partition "fail_part" would overlap partition "part2"
-LINE 1: ...E fail_part PARTITION OF range_parted2 FOR VALUES FROM (10) ...
- ^
+LINE 1: ...art PARTITION OF range_parted2 FOR VALUES FROM (10) TO (30);
+ ^
CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (10) TO (50);
ERROR: partition "fail_part" would overlap partition "part2"
-LINE 1: ...E fail_part PARTITION OF range_parted2 FOR VALUES FROM (10) ...
- ^
+LINE 1: ...art PARTITION OF range_parted2 FOR VALUES FROM (10) TO (50);
+ ^
-- Create a default partition for range partitioned table
CREATE TABLE range2_default PARTITION OF range_parted2 DEFAULT;
-- More than one default partition is not allowed, so this should give error
@@ -892,14 +892,14 @@ CREATE TABLE range_parted3 (
CREATE TABLE part00 PARTITION OF range_parted3 FOR VALUES FROM (0, minvalue) TO (0, maxvalue);
CREATE TABLE fail_part PARTITION OF range_parted3 FOR VALUES FROM (0, minvalue) TO (0, 1);
ERROR: partition "fail_part" would overlap partition "part00"
-LINE 1: ...E fail_part PARTITION OF range_parted3 FOR VALUES FROM (0, m...
+LINE 1: ..._part PARTITION OF range_parted3 FOR VALUES FROM (0, minvalu...
^
CREATE TABLE part10 PARTITION OF range_parted3 FOR VALUES FROM (1, minvalue) TO (1, 1);
CREATE TABLE part11 PARTITION OF range_parted3 FOR VALUES FROM (1, 1) TO (1, 10);
CREATE TABLE part12 PARTITION OF range_parted3 FOR VALUES FROM (1, 10) TO (1, maxvalue);
CREATE TABLE fail_part PARTITION OF range_parted3 FOR VALUES FROM (1, 10) TO (1, 20);
ERROR: partition "fail_part" would overlap partition "part12"
-LINE 1: ...E fail_part PARTITION OF range_parted3 FOR VALUES FROM (1, 1...
+LINE 1: ...rt PARTITION OF range_parted3 FOR VALUES FROM (1, 10) TO (1,...
^
CREATE TABLE range3_default PARTITION OF range_parted3 DEFAULT;
-- cannot create a partition that says column b is allowed to range
@@ -907,7 +907,7 @@ CREATE TABLE range3_default PARTITION OF range_parted3 DEFAULT;
-- more specific ranges
CREATE TABLE fail_part PARTITION OF range_parted3 FOR VALUES FROM (1, minvalue) TO (1, maxvalue);
ERROR: partition "fail_part" would overlap partition "part10"
-LINE 1: ...E fail_part PARTITION OF range_parted3 FOR VALUES FROM (1, m...
+LINE 1: ..._part PARTITION OF range_parted3 FOR VALUES FROM (1, minvalu...
^
-- check for partition bound overlap and other invalid specifications for the hash partition
CREATE TABLE hash_parted2 (
--
2.26.0
