On 3/3/20 10:08 AM, Alvaro Herrera wrote:
+\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
This won't work well, because people would be forced to update the .out
file whenever the execPartition.c file changed to add or remove lines
before the one with the error call.
I agree. I expected that and should have made it more clear that I
didn't intend for those tests to be committed. Others in the thread
suggested I include some form of test, even if it didn't live past
review. That being said...
Maybe if you want to verify the
schema/table names, use a plpgsql function to extract them, using
GET STACKED DIAGNOSTICS TABLE_NAME = ...
in an exception block?
This is a great idea and the result looks much cleaner than I expected.
I have no reservations about committing the attached tests.
I'm not sure that this *needs* to be tested, though. Don't we already
verify that errtable() works, elsewhere?
I looked for tests that might target errtable() or errtableconstraint()
but found none. Perhaps someone who knows the tests better could answer
this question.
I don't suppose you mean to
test that every single ereport() call that includes errtable() contains
a TABLE NAME item.
Correct. I intend only to test the few calls I'm touching in this
thread. It might be worthwhile for someone to perform a more thorough
review of existing errors, however. The documentation seems to say that
every error in SQLSTATE class 23 has one of these fields filled[1]. The
errors in these patches are in that class but lacked any fields.
[1] https://www.postgresql.org/docs/current/errcodes-appendix.html
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 1/2] 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 19a259beeab29ca4a1398641c8e2dbdf5a3d548e 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 2/2] Tests for partition error fields
---
src/test/regress/expected/partition_errors.out | 70 ++++++++++++++++++++++++++
src/test/regress/parallel_schedule | 2 +-
src/test/regress/sql/partition_errors.sql | 45 +++++++++++++++++
3 files changed, 116 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..cbe77fd107
--- /dev/null
+++ src/test/regress/expected/partition_errors.out
@@ -0,0 +1,70 @@
+--
+-- Tests for partition error fields
+--
+\pset expanded on
+CREATE FUNCTION partition_error_record(
+ dml text,
+ OUT err_sqlstate text,
+ OUT err_message text,
+ OUT err_detail text,
+ OUT err_schema text,
+ OUT err_table text)
+AS $$
+BEGIN
+ EXECUTE $1;
+EXCEPTION
+ WHEN OTHERS THEN GET STACKED DIAGNOSTICS
+ err_sqlstate := RETURNED_SQLSTATE,
+ err_message := MESSAGE_TEXT,
+ err_detail := PG_EXCEPTION_DETAIL,
+ err_schema := SCHEMA_NAME,
+ err_table := TABLE_NAME;
+END;
+$$ LANGUAGE plpgsql;
+-- no partitions
+CREATE TABLE pterr1 (x int, y int, PRIMARY KEY (x, y)) PARTITION BY RANGE (y);
+SELECT * FROM partition_error_record($$
+ INSERT INTO pterr1 VALUES (10, 10);
+$$);
+-[ RECORD 1 ]+------------------------------------------------------
+err_sqlstate | 23514
+err_message | no partition of relation "pterr1" found for row
+err_detail | Partition key of the failing row contains (y) = (10).
+err_schema | public
+err_table | pterr1
+
+-- outside the only partition
+CREATE TABLE pterr1_p1 PARTITION OF pterr1 FOR VALUES FROM (1) TO (5);
+SELECT * FROM partition_error_record($$
+ INSERT INTO pterr1 VALUES (10, 10);
+$$);
+-[ RECORD 1 ]+------------------------------------------------------
+err_sqlstate | 23514
+err_message | no partition of relation "pterr1" found for row
+err_detail | Partition key of the failing row contains (y) = (10).
+err_schema | public
+err_table | pterr1
+
+SELECT * FROM partition_error_record($$
+ INSERT INTO pterr1_p1 VALUES (10, 10);
+$$);
+-[ RECORD 1 ]+---------------------------------------------------------------
+err_sqlstate | 23514
+err_message | new row for relation "pterr1_p1" violates partition constraint
+err_detail | Failing row contains (10, 10).
+err_schema | public
+err_table | pterr1_p1
+
+-- conflict with default
+CREATE TABLE pterr1_default PARTITION OF pterr1 DEFAULT;
+INSERT INTO pterr1 VALUES (10, 10);
+SELECT * FROM partition_error_record($$
+ CREATE TABLE pterr1_p2 PARTITION OF pterr1 FOR VALUES FROM (6) TO (20);
+$$);
+-[ RECORD 1 ]+--------------------------------------------------------------------------------------------------
+err_sqlstate | 23514
+err_message | updated partition constraint for default partition "pterr1_default" would be violated by some row
+err_detail |
+err_schema | public
+err_table | pterr1_default
+
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..420d75994f
--- /dev/null
+++ src/test/regress/sql/partition_errors.sql
@@ -0,0 +1,45 @@
+--
+-- Tests for partition error fields
+--
+\pset expanded on
+CREATE FUNCTION partition_error_record(
+ dml text,
+ OUT err_sqlstate text,
+ OUT err_message text,
+ OUT err_detail text,
+ OUT err_schema text,
+ OUT err_table text)
+AS $$
+BEGIN
+ EXECUTE $1;
+EXCEPTION
+ WHEN OTHERS THEN GET STACKED DIAGNOSTICS
+ err_sqlstate := RETURNED_SQLSTATE,
+ err_message := MESSAGE_TEXT,
+ err_detail := PG_EXCEPTION_DETAIL,
+ err_schema := SCHEMA_NAME,
+ err_table := TABLE_NAME;
+END;
+$$ LANGUAGE plpgsql;
+
+-- no partitions
+CREATE TABLE pterr1 (x int, y int, PRIMARY KEY (x, y)) PARTITION BY RANGE (y);
+SELECT * FROM partition_error_record($$
+ INSERT INTO pterr1 VALUES (10, 10);
+$$);
+
+-- outside the only partition
+CREATE TABLE pterr1_p1 PARTITION OF pterr1 FOR VALUES FROM (1) TO (5);
+SELECT * FROM partition_error_record($$
+ INSERT INTO pterr1 VALUES (10, 10);
+$$);
+SELECT * FROM partition_error_record($$
+ INSERT INTO pterr1_p1 VALUES (10, 10);
+$$);
+
+-- conflict with default
+CREATE TABLE pterr1_default PARTITION OF pterr1 DEFAULT;
+INSERT INTO pterr1 VALUES (10, 10);
+SELECT * FROM partition_error_record($$
+ CREATE TABLE pterr1_p2 PARTITION OF pterr1 FOR VALUES FROM (6) TO (20);
+$$);
--
2.11.0
