Skip to content

Commit

Permalink
sql: use correct FuncExpr when encoding sequences
Browse files Browse the repository at this point in the history
Previously, when encoding sequences by swapping sequence
names for IDs, we were always wrapping the sequence in
a nextval func. This is incorrect, and instead
we should wrap the sequence in whatever function
it came in before this encoding. This patch makes
this change.

Release justification: bug fix for new functionality
Release note (bug fix): use correct FuncExpr when encoding sequences.
  • Loading branch information
the-ericwang35 committed Mar 3, 2021
1 parent 9bf97ac commit c5df5ec
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 15 deletions.
43 changes: 29 additions & 14 deletions pkg/sql/logictest/testdata/logic_test/sequences_regclass
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ statement ok
ALTER TABLE foo ADD COLUMN l INT NOT NULL

statement ok
ALTER TABLE FOO ALTER COLUMN l SET DEFAULT nextval('test_seq2')
ALTER TABLE FOO ALTER COLUMN l SET DEFAULT currval('test_seq2')

statement ok
SELECT nextval('test_seq2')

query TT
SHOW CREATE TABLE foo
Expand All @@ -38,7 +41,7 @@ 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 nextval('test.public.test_seq2':::STRING::REGCLASS),
l INT8 NOT NULL DEFAULT currval('test.public.test_seq2':::STRING::REGCLASS),
CONSTRAINT "primary" PRIMARY KEY (i ASC),
FAMILY "primary" (i, j, k, l)
)
Expand Down Expand Up @@ -103,10 +106,13 @@ CREATE SEQUENCE s2
statement ok
CREATE TABLE bar (
i SERIAL PRIMARY KEY,
j INT NOT NULL DEFAULT nextval($s1_id::regclass),
j INT NOT NULL DEFAULT currval($s1_id::regclass),
k INT NOT NULL DEFAULT nextval('s2'),
FAMILY (i, j, k))

statement ok
SELECT nextval($s1_id::regclass)

statement ok
INSERT INTO bar VALUES (default, default, default)

Expand All @@ -125,7 +131,7 @@ 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 nextval('test.public.new_s1':::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),
CONSTRAINT "primary" PRIMARY KEY (i ASC),
FAMILY fam_0_i_j_k (i, j, k)
Expand All @@ -139,7 +145,7 @@ query III
SELECT i, j, k FROM bar ORDER BY i, j, k
----
1 1 1
2 2 2
2 1 2


# Test that databases with sequences can be renamed, even if they are
Expand All @@ -164,9 +170,12 @@ CREATE SEQUENCE other_db.s2
statement ok
CREATE TABLE other_db.t (
i INT NOT NULL DEFAULT nextval($s_id::regclass),
j INT NOT NULL DEFAULT nextval('other_db.public.s2'),
j INT NOT NULL DEFAULT currval('other_db.public.s2'),
FAMILY (i, j))

statement ok
SELECT nextval('other_db.public.s2')

statement ok
INSERT INTO other_db.t VALUES (default, default)

Expand All @@ -179,7 +188,7 @@ SHOW CREATE TABLE new_other_db.t
----
new_other_db.public.t CREATE TABLE public.t (
i INT8 NOT NULL DEFAULT nextval('new_other_db.public.s':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT nextval('new_other_db.public.s2':::STRING::REGCLASS),
j INT8 NOT NULL DEFAULT currval('new_other_db.public.s2':::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 @@ -193,7 +202,7 @@ query II
SELECT i, j FROM new_other_db.t ORDER BY i, j
----
1 1
2 2
2 1


# Test that sequences can change schemas even if they're referenced
Expand All @@ -219,9 +228,12 @@ statement ok
CREATE TABLE tb (
i SERIAL PRIMARY KEY,
j INT NOT NULL DEFAULT nextval($sc_s1_id::regclass),
k INT NOT NULL DEFAULT nextval('test.public.sc_s2'),
k INT NOT NULL DEFAULT currval('test.public.sc_s2'),
FAMILY (i, j, k))

statement ok
SELECT nextval('test.public.sc_s2')

statement ok
INSERT INTO tb VALUES (default, default, default)

Expand All @@ -241,7 +253,7 @@ 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 nextval('test.test_schema.sc_s2':::STRING::REGCLASS),
k INT8 NOT NULL DEFAULT currval('test.test_schema.sc_s2':::STRING::REGCLASS),
CONSTRAINT "primary" PRIMARY KEY (i ASC),
FAMILY fam_0_i_j_k (i, j, k)
)
Expand All @@ -254,7 +266,7 @@ query III
SELECT i, j, k FROM tb ORDER BY i, j, k
----
1 1 1
2 2 2
2 2 1


# Test that sequences can have their schemas renamed even if
Expand All @@ -277,9 +289,12 @@ statement ok
CREATE TABLE foo (
i SERIAL PRIMARY KEY,
j INT NOT NULL DEFAULT nextval($s3_id::regclass),
k INT NOT NULL DEFAULT nextval('test.test_schema.s4'),
k INT NOT NULL DEFAULT currval('test.test_schema.s4'),
FAMILY (i, j, k))

statement ok
SELECT nextval('test.test_schema.s4')

statement ok
INSERT INTO foo VALUES (default, default, default)

Expand All @@ -293,7 +308,7 @@ 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 nextval('test.new_test_schema.s4':::STRING::REGCLASS),
k INT8 NOT NULL DEFAULT currval('test.new_test_schema.s4':::STRING::REGCLASS),
CONSTRAINT "primary" PRIMARY KEY (i ASC),
FAMILY fam_0_i_j_k (i, j, k)
)
Expand All @@ -306,7 +321,7 @@ query III
SELECT i, j, k FROM new_test_schema.foo ORDER BY i, j, k
----
1 1 1
2 2 2
2 2 1

statement ok
SET SCHEMA public
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/sequence/sequence.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func ReplaceSequenceNamesWithIDs(
return true, expr, nil
}
return false, &tree.FuncExpr{
Func: tree.WrapFunction("nextval"),
Func: t.Func,
Exprs: tree.Exprs{
&tree.AnnotateTypeExpr{
Type: types.RegClass,
Expand Down

0 comments on commit c5df5ec

Please sign in to comment.