Skip to content

Commit

Permalink
sqlbase: link to issue #42508 in sequence-related errors
Browse files Browse the repository at this point in the history
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:

```
[email protected]: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: #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: #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: #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.
  • Loading branch information
knz committed Nov 17, 2019
1 parent c2b2cd1 commit a234188
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 5 deletions.
27 changes: 26 additions & 1 deletion pkg/sql/logictest/testdata/logic_test/schema_change_in_txn
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion pkg/sql/sqlbase/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions pkg/sql/sqlbase/evalctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down

0 comments on commit a234188

Please sign in to comment.