From a234188fa75c85260d53c349f7b4b9f410026eaa Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Fri, 15 Nov 2019 10:46:46 +0100 Subject: [PATCH] sqlbase: link to issue #42508 in sequence-related errors This patch fixes two things. 1) When the backfiller encounters a sequence operation it fails with a hard error. This and consequences is described further in issue #42508. To inform the user better, this patch links the error message to that issue. Additionally, this also ensures that telemetry usage is reported for the unimplemented feature. For example: ``` root@127.0.0.1:39861/movr> alter table t add column y int default nextval('s'); pq: nextval(): unimplemented: cannot backfill such sequence operation HINT: You have attempted to use a feature that is not yet implemented. See: https://github.com/cockroachdb/cockroach/issues/42508 ``` and ``` > begin; alter table t add column y int default nextval('s'); commit; pq: transaction committed but schema change aborted with error: (0A000): nextval(): unimplemented: cannot backfill such sequence operation HINT: You have attempted to use a feature that is not yet implemented. See: https://github.com/cockroachdb/cockroach/issues/42508 -- Some of the non-DDL statements may have committed successfully, but some of the DDL statement(s) failed. Manual inspection may be required to determine the actual state of the database. -- See: https://github.com/cockroachdb/cockroach/issues/42061 ``` 2) The other thing. Prior to this patch, an error "under" a backfill would be flattened into error code 42P15 (invalid schema), even if the schema was valid and the error was really about something encountered during the backfill. This patch fixes it. For example, ``` ALTER TABLE shopping ADD COLUMN c int AS (quantity::int // 0) STORED ``` Would previously error out with code 42P15, whereas the true error is 22012 (division by zero) which is reported now. Release note (sql change): CockroachDB will now properly refer to issue #42508 in the error message hint when a client attempts to add a sequence-based column to an existing table, which is an unimplemented feature. Release note (sql change): CockroachDB will now report an more accurate error message, hint and error code if/when an error is encountered while adding a new column. --- .../testdata/logic_test/schema_change_in_txn | 27 ++++++++++++++++++- .../schema_change_in_txn-mixed-19.1-19.2 | 27 ++++++++++++++++++- pkg/sql/sqlbase/errors.go | 2 +- pkg/sql/sqlbase/evalctx.go | 5 ++-- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn b/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn index 048458b55c76..dcfb766957f7 100644 --- a/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn +++ b/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn @@ -520,7 +520,7 @@ CREATE TABLE shopping (item, quantity) AS VALUES ('cups', 10), ('plates', 30), ( statement count 3 SELECT * FROM shopping -statement error pgcode 42P15 division by zero +statement error pgcode 22012 division by zero ALTER TABLE shopping ADD COLUMN c int AS (quantity::int // 0) STORED statement ok @@ -1746,3 +1746,28 @@ DROP TABLE people; statement ok COMMIT; + +subtest add_column_default_sequence_op + +# This is a current known limitation (#42508). This test ensures that +# the error message is properly reported, with issue hint. +# Once the limitation is lifted, this entire test can be removed +# (and replaced by test for the feature). + +statement ok +CREATE TABLE t42508(x INT); INSERT INTO t42508(x) VALUES (1); + +statement ok +CREATE SEQUENCE s42508 + +statement error pgcode 0A000 unimplemented: cannot backfill such sequence operation.*\nHINT.*\n.*42508 +ALTER TABLE t42508 ADD COLUMN y INT DEFAULT nextval('s42508') + +statement ok +BEGIN + +statement ok +ALTER TABLE t42508 ADD COLUMN y INT DEFAULT nextval('s42508') + +statement error pgcode XXA00 unimplemented: cannot backfill such sequence operation.*\nHINT.*\n.*42508 +COMMIT diff --git a/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn-mixed-19.1-19.2 b/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn-mixed-19.1-19.2 index d9b78f36bcd1..da68e12382ba 100644 --- a/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn-mixed-19.1-19.2 +++ b/pkg/sql/logictest/testdata/logic_test/schema_change_in_txn-mixed-19.1-19.2 @@ -495,7 +495,7 @@ CREATE TABLE shopping (item, quantity) AS VALUES ('cups', 10), ('plates', 30), ( statement count 3 SELECT * FROM shopping -statement error pgcode 42P15 division by zero +statement error pgcode 22012 division by zero ALTER TABLE shopping ADD COLUMN c int AS (quantity::int // 0) STORED statement ok @@ -1340,3 +1340,28 @@ COMMIT statement ok DROP TABLE t + +subtest add_column_default_sequence_op + +# This is a current known limitation (#42508). This test ensures that +# the error message is properly reported, with issue hint. +# Once the limitation is lifted, this entire test can be removed +# (and replaced by test for the feature). + +statement ok +CREATE TABLE t42508(x INT); INSERT INTO t42508(x) VALUES (1); + +statement ok +CREATE SEQUENCE s42508 + +statement error pgcode 0A000 unimplemented: cannot backfill such sequence operation.*\nHINT.*\n.*42508 +ALTER TABLE t42508 ADD COLUMN y INT DEFAULT nextval('s42508') + +statement ok +BEGIN + +statement ok +ALTER TABLE t42508 ADD COLUMN y INT DEFAULT nextval('s42508') + +statement error pgcode XXA00 unimplemented: cannot backfill such sequence operation.*\nHINT.*\n.*42508 +COMMIT diff --git a/pkg/sql/sqlbase/errors.go b/pkg/sql/sqlbase/errors.go index 4ccbdd26009c..7829ee22c351 100644 --- a/pkg/sql/sqlbase/errors.go +++ b/pkg/sql/sqlbase/errors.go @@ -49,7 +49,7 @@ func NewNonNullViolationError(columnName string) error { // NewInvalidSchemaDefinitionError creates an error for an invalid schema // definition such as a schema definition that doesn't parse. func NewInvalidSchemaDefinitionError(err error) error { - return pgerror.New(pgcode.InvalidSchemaDefinition, err.Error()) + return pgerror.WithCandidateCode(err, pgcode.InvalidSchemaDefinition) } // NewUnsupportedSchemaUsageError creates an error for an invalid diff --git a/pkg/sql/sqlbase/evalctx.go b/pkg/sql/sqlbase/evalctx.go index d892aae215db..6ac61297e5dc 100644 --- a/pkg/sql/sqlbase/evalctx.go +++ b/pkg/sql/sqlbase/evalctx.go @@ -15,7 +15,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/types" - "github.com/pkg/errors" + "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" + "github.com/cockroachdb/errors" ) // DummySequenceOperators implements the tree.SequenceOperators interface by @@ -24,7 +25,7 @@ type DummySequenceOperators struct{} var _ tree.EvalDatabase = &DummySequenceOperators{} -var errSequenceOperators = errors.New("cannot backfill such sequence operation") +var errSequenceOperators = unimplemented.NewWithIssue(42508, "cannot backfill such sequence operation") // ParseQualifiedTableName is part of the tree.EvalDatabase interface. func (so *DummySequenceOperators) ParseQualifiedTableName(