From 6ff4cd9c1614b089f01cb633220d37305719f46d Mon Sep 17 00:00:00 2001 From: amitlan Date: Fri, 10 Apr 2020 16:38:21 +0900 Subject: [PATCH v3 2/2] Improve check_new_partition_bound error position reporting --- src/backend/parser/parse_utilcmd.c | 3 +++ src/backend/partitioning/partbounds.c | 40 ++++++++++++++++++++++-------- src/test/regress/expected/alter_table.out | 2 +- src/test/regress/expected/create_table.out | 28 ++++++++++----------- 4 files changed, 48 insertions(+), 25 deletions(-) diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 75c122f..72113bb 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 dd56832..feb3357 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -2810,6 +2810,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 +2905,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 +2934,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 +2968,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 +2978,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 +2995,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) @@ -3051,6 +3061,8 @@ check_new_partition_bound(char *relname, Relation parent, * offset + 2. */ overlap = true; + overlap_location = ((PartitionRangeDatum *) + linitial(spec->upperdatums))->location; with = boundinfo->indexes[offset + 2]; } } @@ -3062,6 +3074,8 @@ check_new_partition_bound(char *relname, Relation parent, * partition between offset and offset + 1. */ overlap = true; + overlap_location = ((PartitionRangeDatum *) + linitial(spec->lowerdatums))->location; with = boundinfo->indexes[offset + 1]; } } @@ -3077,11 +3091,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 +3330,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 +3352,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 +3360,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 +3369,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 +3396,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); } /* diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 576f19b..b317469 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 b8de012..bc5a660 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: ..._part PARTITION OF range_parted3 FOR VALUES FROM (1, 10) TO ... ^ 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 ( -- 1.8.3.1