On 3/1/20 10:09 PM, Amit Langote wrote:
Hi Chris,
On Mon, Mar 2, 2020 at 8:51 AM Chris Bandy <bandy.ch...@gmail.com> wrote:
On 3/1/20 5:14 AM, Amit Kapila wrote:
On Sun, Mar 1, 2020 at 10:10 AM Amit Langote <amitlangot...@gmail.com> wrote:
There are couple more instances in src/backend/command/tablecmds.c
where partition constraint is checked:
Maybe, better fix these too for completeness.
Another thing we might need to see is which of these can be
back-patched. We should also try to write the tests for cases we are
changing even if we don't want to commit those.
I don't have any opinion on back-patching. Existing tests pass. I wasn't
able to find another test that checks the constraint field of errors.
There's a little bit in the tests for psql, but that is about the the
\errverbose functionality rather than specific errors and their fields.
Actually, it's not a bad idea to use \errverbose to test this.
I've added a second patch with tests that cover three of the five errors
touched by the first patch. Rather than \errverbose, I simply \set
VERBOSITY verbose. I could not find a way to exclude the location field
from the output, so those lines will be likely be out of date soon--if
not already.
I couldn't find a way to exercise the errors in tablecmds.c. Does anyone
know how to instigate a table rewrite that would violate partition
constraints? I tried:
ALTER TABLE pterr1 ALTER y TYPE bigint USING (y - 5);
ERROR: 42P16: cannot alter column "y" because it is part of the
partition key of relation "pterr1"
LOCATION: ATPrepAlterColumnType, tablecmds.c:10812
Thanks,
Chris
>From c6b39accf9f51f9c08a2fc62e848144776e23ffb Mon Sep 17 00:00:00 2001
From: Chris Bandy <bandy.ch...@gmail.com>
Date: Sat, 29 Feb 2020 12:47:56 -0600
Subject: [PATCH] Add schema and table names to partition errors
---
src/backend/commands/tablecmds.c | 6 ++++--
src/backend/executor/execMain.c | 3 ++-
src/backend/executor/execPartition.c | 3 ++-
src/backend/partitioning/partbounds.c | 3 ++-
4 files changed, 10 insertions(+), 5 deletions(-)
diff --git src/backend/commands/tablecmds.c src/backend/commands/tablecmds.c
index 02a7c04fdb..6a32812a1f 100644
--- src/backend/commands/tablecmds.c
+++ src/backend/commands/tablecmds.c
@@ -5342,12 +5342,14 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("updated partition constraint for default partition \"%s\" would be violated by some row",
- RelationGetRelationName(oldrel))));
+ RelationGetRelationName(oldrel)),
+ errtable(oldrel)));
else
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("partition constraint of relation \"%s\" is violated by some row",
- RelationGetRelationName(oldrel))));
+ RelationGetRelationName(oldrel)),
+ errtable(oldrel)));
}
/* Write the tuple out to the new relation */
diff --git src/backend/executor/execMain.c src/backend/executor/execMain.c
index ee5c3a60ff..7cb486b211 100644
--- src/backend/executor/execMain.c
+++ src/backend/executor/execMain.c
@@ -1878,7 +1878,8 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates partition constraint",
RelationGetRelationName(resultRelInfo->ri_RelationDesc)),
- val_desc ? errdetail("Failing row contains %s.", val_desc) : 0));
+ val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
+ errtable(resultRelInfo->ri_RelationDesc)));
}
/*
diff --git src/backend/executor/execPartition.c src/backend/executor/execPartition.c
index c13b1d3501..a5542b92c7 100644
--- src/backend/executor/execPartition.c
+++ src/backend/executor/execPartition.c
@@ -345,7 +345,8 @@ ExecFindPartition(ModifyTableState *mtstate,
RelationGetRelationName(rel)),
val_desc ?
errdetail("Partition key of the failing row contains %s.",
- val_desc) : 0));
+ val_desc) : 0,
+ errtable(rel)));
}
if (partdesc->is_leaf[partidx])
diff --git src/backend/partitioning/partbounds.c src/backend/partitioning/partbounds.c
index 35953f23fa..4c47f54a57 100644
--- src/backend/partitioning/partbounds.c
+++ src/backend/partitioning/partbounds.c
@@ -1366,7 +1366,8 @@ check_default_partition_contents(Relation parent, Relation default_rel,
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("updated partition constraint for default partition \"%s\" would be violated by some row",
- RelationGetRelationName(default_rel))));
+ RelationGetRelationName(default_rel)),
+ errtable(default_rel)));
ResetExprContext(econtext);
CHECK_FOR_INTERRUPTS();
--
2.11.0
>From 2d2ab39bcbc7ea13cd04986b0c8aad6f14449ebd Mon Sep 17 00:00:00 2001
From: Chris Bandy <bandy.ch...@gmail.com>
Date: Mon, 2 Mar 2020 22:13:28 -0600
Subject: [PATCH] Tests for partition error fields
---
src/test/regress/expected/partition_errors.out | 37 ++++++++++++++++++++++++++
src/test/regress/parallel_schedule | 2 +-
src/test/regress/sql/partition_errors.sql | 23 ++++++++++++++++
3 files changed, 61 insertions(+), 1 deletion(-)
create mode 100644 src/test/regress/expected/partition_errors.out
create mode 100644 src/test/regress/sql/partition_errors.sql
diff --git src/test/regress/expected/partition_errors.out src/test/regress/expected/partition_errors.out
new file mode 100644
index 0000000000..c5a3b8c815
--- /dev/null
+++ src/test/regress/expected/partition_errors.out
@@ -0,0 +1,37 @@
+--
+-- Tests for partition error fields
+--
+\set VERBOSITY verbose
+-- no partitions
+CREATE TABLE pterr1 (x int, y int, PRIMARY KEY (x, y)) PARTITION BY RANGE (y);
+INSERT INTO pterr1 VALUES (10, 10);
+ERROR: 23514: no partition of relation "pterr1" found for row
+DETAIL: Partition key of the failing row contains (y) = (10).
+SCHEMA NAME: public
+TABLE NAME: pterr1
+LOCATION: ExecFindPartition, execPartition.c:349
+-- outside the only partition
+CREATE TABLE pterr1_p1 PARTITION OF pterr1 FOR VALUES FROM (1) TO (5);
+INSERT INTO pterr1 VALUES (10, 10);
+ERROR: 23514: no partition of relation "pterr1" found for row
+DETAIL: Partition key of the failing row contains (y) = (10).
+SCHEMA NAME: public
+TABLE NAME: pterr1
+LOCATION: ExecFindPartition, execPartition.c:349
+INSERT INTO pterr1_p1 VALUES (10, 10);
+ERROR: 23514: new row for relation "pterr1_p1" violates partition constraint
+DETAIL: Failing row contains (10, 10).
+SCHEMA NAME: public
+TABLE NAME: pterr1_p1
+LOCATION: ExecPartitionCheckEmitError, execMain.c:1882
+-- conflict with default
+CREATE TABLE pterr1_default PARTITION OF pterr1 DEFAULT;
+INSERT INTO pterr1 VALUES (10, 10);
+CREATE TABLE pterr1_p2 PARTITION OF pterr1 FOR VALUES FROM (6) TO (20);
+ERROR: 23514: updated partition constraint for default partition "pterr1_default" would be violated by some row
+SCHEMA NAME: public
+TABLE NAME: pterr1_default
+LOCATION: check_default_partition_contents, partbounds.c:1370
+-- cleanup
+\set VERBOSITY default
+DROP TABLE pterr1, pterr1_default, pterr1_p1;
diff --git src/test/regress/parallel_schedule src/test/regress/parallel_schedule
index d2b17dd3ea..e1708c87ec 100644
--- src/test/regress/parallel_schedule
+++ src/test/regress/parallel_schedule
@@ -112,7 +112,7 @@ test: plancache limit plpgsql copy2 temp domain rangefuncs prepare conversion tr
# ----------
# Another group of parallel tests
# ----------
-test: partition_join partition_prune reloptions hash_part indexing partition_aggregate partition_info tuplesort explain
+test: partition_errors partition_join partition_prune reloptions hash_part indexing partition_aggregate partition_info tuplesort explain
# event triggers cannot run concurrently with any test that runs DDL
test: event_trigger
diff --git src/test/regress/sql/partition_errors.sql src/test/regress/sql/partition_errors.sql
new file mode 100644
index 0000000000..7b9197d507
--- /dev/null
+++ src/test/regress/sql/partition_errors.sql
@@ -0,0 +1,23 @@
+--
+-- Tests for partition error fields
+--
+
+\set VERBOSITY verbose
+
+-- no partitions
+CREATE TABLE pterr1 (x int, y int, PRIMARY KEY (x, y)) PARTITION BY RANGE (y);
+INSERT INTO pterr1 VALUES (10, 10);
+
+-- outside the only partition
+CREATE TABLE pterr1_p1 PARTITION OF pterr1 FOR VALUES FROM (1) TO (5);
+INSERT INTO pterr1 VALUES (10, 10);
+INSERT INTO pterr1_p1 VALUES (10, 10);
+
+-- conflict with default
+CREATE TABLE pterr1_default PARTITION OF pterr1 DEFAULT;
+INSERT INTO pterr1 VALUES (10, 10);
+CREATE TABLE pterr1_p2 PARTITION OF pterr1 FOR VALUES FROM (6) TO (20);
+
+-- cleanup
+\set VERBOSITY default
+DROP TABLE pterr1, pterr1_default, pterr1_p1;
--
2.11.0
