Skip to content

Commit

Permalink
sql: only qualify sequences with database names
Browse files Browse the repository at this point in the history 
if sequence is in a different database

Fixes #58783.

Previously, when we were converting sequence IDs back
into names, we were fully qualifying them (with
their database and schema). This differs from Postgres,
which does not include database qualification since
cross database references are disallowed.
This resulted in `pk_and_sequence_for` in the
ruby/rails activerecord-cockroachdb-adapter
being unable to grab the correct sequence,
causing roachtests to fail.
This patch changes the sequence decoding
to only include the database name if the
sequence does not live in the current database.

Release justification: bug fix for new functionality
Release note: None
  • Loading branch information
@the-ericwang35
the-ericwang35 committed Mar 8, 2021
1 parent a00fa66 commit 2f9f77b
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 50 deletions.
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/import_stmt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5668,7 +5668,7 @@ func TestImportPgDump(t *testing.T) {
if c.expected == expectAll {
sqlDB.CheckQueryResults(t, `SHOW CREATE TABLE seqtable`, [][]string{{
"seqtable", `CREATE TABLE public.seqtable (
a INT8 NULL DEFAULT nextval('foo.public.a_seq':::STRING::REGCLASS),
a INT8 NULL DEFAULT nextval('public.a_seq':::STRING::REGCLASS),
b INT8 NULL,
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
Expand Down
10 changes: 9 additions & 1 deletion pkg/sql/catalog/schemaexpr/column.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,11 +302,19 @@ func replaceIDsWithFQNames(
if err != nil {
return true, expr, nil //nolint:returnerrcheck
}

// Omit the database qualification if the sequence lives in the current database.
currDb := semaCtx.TableNameResolver.CurrentDatabase()
if seqName.Catalog() == currDb {
seqName.CatalogName = ""
seqName.ExplicitCatalog = false
}

// Swap out this node to use the qualified table name for the sequence.
return false, &tree.CastExpr{
Type: types.RegClass,
SyntaxMode: tree.CastShort,
Expr: tree.NewStrVal(seqName.FQString()),
Expr: tree.NewStrVal(seqName.String()),
}, nil
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/sql/logictest/testdata/logic_test/drop_sequence
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ query TT
SHOW CREATE TABLE t1
----
t1 CREATE TABLE public.t1 (
i INT8 NOT NULL DEFAULT nextval('test.public.drop_test':::STRING::REGCLASS),
i INT8 NOT NULL DEFAULT nextval('public.drop_test':::STRING::REGCLASS),
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
FAMILY "primary" (i, rowid)
Expand Down Expand Up @@ -93,7 +93,7 @@ SHOW CREATE TABLE foo
----
foo CREATE TABLE public.foo (
i INT8 NOT NULL DEFAULT nextval('other_db.public.s':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('test.public.s':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('public.s':::STRING::REGCLASS),
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
FAMILY fam_0_i_j_rowid (i, j, rowid)
Expand All @@ -115,7 +115,7 @@ SHOW CREATE TABLE foo
----
foo CREATE TABLE public.foo (
i INT8 NOT NULL,
j INT8 NOT NULL DEFAULT nextval('test.public.s':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('public.s':::STRING::REGCLASS),
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
FAMILY fam_0_i_j_rowid (i, j, rowid)
Expand Down Expand Up @@ -151,8 +151,8 @@ query TT
SHOW CREATE TABLE bar
----
bar CREATE TABLE public.bar (
i INT8 NOT NULL DEFAULT nextval('test.other_sc.s':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('test.public.s':::STRING::REGCLASS),
i INT8 NOT NULL DEFAULT nextval('other_sc.s':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('public.s':::STRING::REGCLASS),
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
FAMILY fam_0_i_j_rowid (i, j, rowid)
Expand All @@ -174,7 +174,7 @@ SHOW CREATE TABLE bar
----
bar CREATE TABLE public.bar (
i INT8 NOT NULL,
j INT8 NOT NULL DEFAULT nextval('test.public.s':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('public.s':::STRING::REGCLASS),
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
FAMILY fam_0_i_j_rowid (i, j, rowid)
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/logictest/testdata/logic_test/int_size
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ query TT
SHOW CREATE TABLE i4_sql_sequence
----
i4_sql_sequence CREATE TABLE public.i4_sql_sequence (
a INT4 NOT NULL DEFAULT nextval('test.public.i4_sql_sequence_a_seq':::STRING::REGCLASS),
a INT4 NOT NULL DEFAULT nextval('public.i4_sql_sequence_a_seq':::STRING::REGCLASS),
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
FAMILY "primary" (a, rowid)
Expand All @@ -150,7 +150,7 @@ query TT
SHOW CREATE TABLE i8_sql_sequence
----
i8_sql_sequence CREATE TABLE public.i8_sql_sequence (
a INT8 NOT NULL DEFAULT nextval('test.public.i8_sql_sequence_a_seq':::STRING::REGCLASS),
a INT8 NOT NULL DEFAULT nextval('public.i8_sql_sequence_a_seq':::STRING::REGCLASS),
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
FAMILY "primary" (a, rowid)
Expand All @@ -170,7 +170,7 @@ query TT
SHOW CREATE TABLE i4_virtual_sequence
----
i4_virtual_sequence CREATE TABLE public.i4_virtual_sequence (
a INT8 NOT NULL DEFAULT nextval('test.public.i4_virtual_sequence_a_seq':::STRING::REGCLASS),
a INT8 NOT NULL DEFAULT nextval('public.i4_virtual_sequence_a_seq':::STRING::REGCLASS),
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
FAMILY "primary" (a, rowid)
Expand All @@ -186,7 +186,7 @@ query TT
SHOW CREATE TABLE i8_virtual_sequence
----
i8_virtual_sequence CREATE TABLE public.i8_virtual_sequence (
a INT8 NOT NULL DEFAULT nextval('test.public.i8_virtual_sequence_a_seq':::STRING::REGCLASS),
a INT8 NOT NULL DEFAULT nextval('public.i8_virtual_sequence_a_seq':::STRING::REGCLASS),
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
FAMILY "primary" (a, rowid)
Expand Down
38 changes: 19 additions & 19 deletions pkg/sql/logictest/testdata/logic_test/sequences_regclass
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ statement ok
CREATE SEQUENCE test_seq

statement ok
CREATE SEQUENCE test_seq2
CREATE DATABASE diff_db

statement ok
CREATE SEQUENCE diff_db.test_seq

let $test_seq_id
SELECT 'test_seq'::regclass::int

let $test_seq2_id
SELECT 'test_seq2'::regclass::int

statement ok
CREATE TABLE foo (i SERIAL PRIMARY KEY)

Expand All @@ -29,19 +29,19 @@ statement ok
ALTER TABLE foo ADD COLUMN l INT NOT NULL

statement ok
ALTER TABLE FOO ALTER COLUMN l SET DEFAULT currval('test_seq2')
ALTER TABLE FOO ALTER COLUMN l SET DEFAULT currval('diff_db.test_seq')

statement ok
SELECT nextval('test_seq2')
SELECT nextval('diff_db.test_seq')

query TT
SHOW CREATE TABLE foo
----
foo CREATE TABLE public.foo (
i INT8 NOT NULL DEFAULT nextval('test.public.foo_i_seq':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('test.public.test_seq':::STRING::REGCLASS),
k INT8 NOT NULL DEFAULT nextval('test.public.foo_k_seq':::STRING::REGCLASS),
l INT8 NOT NULL DEFAULT currval('test.public.test_seq2':::STRING::REGCLASS),
i INT8 NOT NULL DEFAULT nextval('public.foo_i_seq':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('public.test_seq':::STRING::REGCLASS),
k INT8 NOT NULL DEFAULT nextval('public.foo_k_seq':::STRING::REGCLASS),
l INT8 NOT NULL DEFAULT currval('diff_db.public.test_seq':::STRING::REGCLASS),
CONSTRAINT "primary" PRIMARY KEY (i ASC),
FAMILY "primary" (i, j, k, l)
)
Expand Down Expand Up @@ -130,9 +130,9 @@ query TT
SHOW CREATE TABLE bar
----
bar CREATE TABLE public.bar (
i INT8 NOT NULL DEFAULT nextval('test.public.new_bar_i_seq':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT currval('test.public.new_s1':::STRING::REGCLASS),
k INT8 NOT NULL DEFAULT nextval('test.public.new_s2':::STRING::REGCLASS),
i INT8 NOT NULL DEFAULT nextval('public.new_bar_i_seq':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT currval('public.new_s1':::STRING::REGCLASS),
k INT8 NOT NULL DEFAULT nextval('public.new_s2':::STRING::REGCLASS),
CONSTRAINT "primary" PRIMARY KEY (i ASC),
FAMILY fam_0_i_j_k (i, j, k)
)
Expand Down Expand Up @@ -251,9 +251,9 @@ query TT
SHOW CREATE TABLE tb
----
tb CREATE TABLE public.tb (
i INT8 NOT NULL DEFAULT nextval('test.test_schema.tb_i_seq':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('test.test_schema.sc_s1':::STRING::REGCLASS),
k INT8 NOT NULL DEFAULT currval('test.test_schema.sc_s2':::STRING::REGCLASS),
i INT8 NOT NULL DEFAULT nextval('test_schema.tb_i_seq':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('test_schema.sc_s1':::STRING::REGCLASS),
k INT8 NOT NULL DEFAULT currval('test_schema.sc_s2':::STRING::REGCLASS),
CONSTRAINT "primary" PRIMARY KEY (i ASC),
FAMILY fam_0_i_j_k (i, j, k)
)
Expand Down Expand Up @@ -306,9 +306,9 @@ query TT
SHOW CREATE TABLE new_test_schema.foo
----
new_test_schema.foo CREATE TABLE new_test_schema.foo (
i INT8 NOT NULL DEFAULT nextval('test.new_test_schema.foo_i_seq':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('test.new_test_schema.s3':::STRING::REGCLASS),
k INT8 NOT NULL DEFAULT currval('test.new_test_schema.s4':::STRING::REGCLASS),
i INT8 NOT NULL DEFAULT nextval('new_test_schema.foo_i_seq':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('new_test_schema.s3':::STRING::REGCLASS),
k INT8 NOT NULL DEFAULT currval('new_test_schema.s4':::STRING::REGCLASS),
CONSTRAINT "primary" PRIMARY KEY (i ASC),
FAMILY fam_0_i_j_k (i, j, k)
)
Expand Down
32 changes: 16 additions & 16 deletions pkg/sql/logictest/testdata/logic_test/serial
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ query TT
SHOW CREATE TABLE serial
----
serial CREATE TABLE public.serial (
a INT8 NOT NULL DEFAULT nextval('test.public.serial_a_seq':::STRING::REGCLASS),
a INT8 NOT NULL DEFAULT nextval('public.serial_a_seq':::STRING::REGCLASS),
b INT8 NULL DEFAULT 7:::INT8,
c INT8 NOT NULL DEFAULT nextval('test.public.serial_c_seq2':::STRING::REGCLASS),
c INT8 NOT NULL DEFAULT nextval('public.serial_c_seq2':::STRING::REGCLASS),
CONSTRAINT "primary" PRIMARY KEY (a ASC),
UNIQUE INDEX serial_c_key (c ASC),
FAMILY "primary" (a, b, c)
Expand Down Expand Up @@ -160,8 +160,8 @@ query TT
SHOW CREATE TABLE smallbig
----
smallbig CREATE TABLE public.smallbig (
a INT8 NOT NULL DEFAULT nextval('test.public.smallbig_a_seq':::STRING::REGCLASS),
b INT8 NOT NULL DEFAULT nextval('test.public.smallbig_b_seq':::STRING::REGCLASS),
a INT8 NOT NULL DEFAULT nextval('public.smallbig_a_seq':::STRING::REGCLASS),
b INT8 NOT NULL DEFAULT nextval('public.smallbig_b_seq':::STRING::REGCLASS),
c INT8 NULL,
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
Expand All @@ -183,9 +183,9 @@ query TT
SHOW CREATE TABLE serials
----
serials CREATE TABLE public.serials (
a INT8 NOT NULL DEFAULT nextval('test.public.serials_a_seq':::STRING::REGCLASS),
b INT8 NOT NULL DEFAULT nextval('test.public.serials_b_seq':::STRING::REGCLASS),
c INT8 NOT NULL DEFAULT nextval('test.public.serials_c_seq':::STRING::REGCLASS),
a INT8 NOT NULL DEFAULT nextval('public.serials_a_seq':::STRING::REGCLASS),
b INT8 NOT NULL DEFAULT nextval('public.serials_b_seq':::STRING::REGCLASS),
c INT8 NOT NULL DEFAULT nextval('public.serials_c_seq':::STRING::REGCLASS),
d INT8 NULL,
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
Expand Down Expand Up @@ -222,9 +222,9 @@ query TT
SHOW CREATE TABLE serial
----
serial CREATE TABLE public.serial (
a INT8 NOT NULL DEFAULT nextval('test.public.serial_a_seq1':::STRING::REGCLASS),
a INT8 NOT NULL DEFAULT nextval('public.serial_a_seq1':::STRING::REGCLASS),
b INT8 NULL DEFAULT 7:::INT8,
c INT8 NOT NULL DEFAULT nextval('test.public.serial_c_seq3':::STRING::REGCLASS),
c INT8 NOT NULL DEFAULT nextval('public.serial_c_seq3':::STRING::REGCLASS),
CONSTRAINT "primary" PRIMARY KEY (a ASC),
UNIQUE INDEX serial_c_key (c ASC),
FAMILY "primary" (a, b, c)
Expand Down Expand Up @@ -278,9 +278,9 @@ query TT
SHOW CREATE TABLE "serial_MixedCase"
----
"serial_MixedCase" CREATE TABLE public."serial_MixedCase" (
a INT8 NOT NULL DEFAULT nextval('test.public."serial_MixedCase_a_seq"':::STRING::REGCLASS),
a INT8 NOT NULL DEFAULT nextval('public."serial_MixedCase_a_seq"':::STRING::REGCLASS),
b INT8 NULL DEFAULT 7:::INT8,
c INT8 NOT NULL DEFAULT nextval('test.public."serial_MixedCase_c_seq"':::STRING::REGCLASS),
c INT8 NOT NULL DEFAULT nextval('public."serial_MixedCase_c_seq"':::STRING::REGCLASS),
CONSTRAINT "primary" PRIMARY KEY (a ASC),
UNIQUE INDEX "serial_MixedCase_c_key" (c ASC),
FAMILY "primary" (a, b, c)
Expand All @@ -305,8 +305,8 @@ query TT
SHOW CREATE TABLE smallbig
----
smallbig CREATE TABLE public.smallbig (
a INT2 NOT NULL DEFAULT nextval('test.public.smallbig_a_seq1':::STRING::REGCLASS),
b INT8 NOT NULL DEFAULT nextval('test.public.smallbig_b_seq1':::STRING::REGCLASS),
a INT2 NOT NULL DEFAULT nextval('public.smallbig_a_seq1':::STRING::REGCLASS),
b INT8 NOT NULL DEFAULT nextval('public.smallbig_b_seq1':::STRING::REGCLASS),
c INT8 NULL,
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
Expand All @@ -328,9 +328,9 @@ query TT
SHOW CREATE TABLE serials
----
serials CREATE TABLE public.serials (
a INT2 NOT NULL DEFAULT nextval('test.public.serials_a_seq1':::STRING::REGCLASS),
b INT4 NOT NULL DEFAULT nextval('test.public.serials_b_seq1':::STRING::REGCLASS),
c INT8 NOT NULL DEFAULT nextval('test.public.serials_c_seq1':::STRING::REGCLASS),
a INT2 NOT NULL DEFAULT nextval('public.serials_a_seq1':::STRING::REGCLASS),
b INT4 NOT NULL DEFAULT nextval('public.serials_b_seq1':::STRING::REGCLASS),
c INT8 NOT NULL DEFAULT nextval('public.serials_c_seq1':::STRING::REGCLASS),
d INT8 NULL,
rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
CONSTRAINT "primary" PRIMARY KEY (rowid ASC),
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/show_create_all_tables
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ CREATE TABLE public.c (
);
CREATE SEQUENCE public.s MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1;
CREATE TABLE public.s_tbl (
id INT8 NOT NULL DEFAULT nextval('test_fk_order.public.s':::STRING::REGCLASS),
id INT8 NOT NULL DEFAULT nextval('public.s':::STRING::REGCLASS),
v INT8 NULL,
CONSTRAINT "primary" PRIMARY KEY (id ASC),
FAMILY f1 (id, v)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/show_create_all_tables_builtin
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ CREATE TABLE public.c (
);
CREATE SEQUENCE public.s MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1;
CREATE TABLE public.s_tbl (
id INT8 NOT NULL DEFAULT nextval('test_fk_order.public.s':::STRING::REGCLASS),
id INT8 NOT NULL DEFAULT nextval('public.s':::STRING::REGCLASS),
v INT8 NULL,
CONSTRAINT "primary" PRIMARY KEY (id ASC),
FAMILY f1 (id, v)
Expand Down
5 changes: 4 additions & 1 deletion pkg/sql/sem/tree/name_resolution.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,12 @@ type ObjectNameExistingResolver interface {
}

// QualifiedNameResolver is the helper interface to resolve qualified
// table names given an ID and the required table kind.
// table names given an ID and the required table kind, as well as the
// current database to determine whether or not to include the
// database in the qualification.
type QualifiedNameResolver interface {
GetQualifiedTableNameByID(ctx context.Context, id int64, requiredType RequiredTableKind) (*TableName, error)
CurrentDatabase() string
}

// NameResolutionResult is an opaque reference returned by LookupObject().
Expand Down

0 comments on commit 2f9f77b

Please sign in to comment.