From 04ef85cf9c9fd64c2a451a8ed1b8944ea12a27bf Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Tue, 26 Nov 2019 14:08:37 -0500 Subject: [PATCH 1/4] sql: fix ALTER INDEX IF EXISTS for non-existing index Previously, if ALTER INDEX IF EXISTS (AIIE) was used on an unqualified index name that did not match any index, the database would return an "index does not exist" error. Now, the statement succeeds, but does nothing. Release note (bug fix): ALTER INDEX IF EXISTS no longer fails when using an unqualified index name that does not match any existing index. Now it is a no-op. --- .../testdata/logic_test/rename_index | 40 ++++++++++++++++++- pkg/sql/parser/parse_test.go | 1 + pkg/sql/rename_index.go | 6 ++- 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/rename_index b/pkg/sql/logictest/testdata/logic_test/rename_index index ca0738a9a30d..e0f9d4e56cea 100644 --- a/pkg/sql/logictest/testdata/logic_test/rename_index +++ b/pkg/sql/logictest/testdata/logic_test/rename_index @@ -7,9 +7,21 @@ CREATE TABLE users ( UNIQUE INDEX bar (id, name) ) +statement ok +CREATE TABLE users_dupe ( + id INT PRIMARY KEY, + name VARCHAR NOT NULL, + title VARCHAR, + INDEX foo (name), + UNIQUE INDEX bar (id, name) +) + statement ok INSERT INTO users VALUES (1, 'tom', 'cat'),(2, 'jerry', 'rat') +statement ok +INSERT INTO users_dupe VALUES (1, 'tom', 'cat'),(2, 'jerry', 'rat') + query TTBITTBB colnames SHOW INDEXES FROM users ---- @@ -20,6 +32,16 @@ users foo true 2 id ASC false users bar false 1 id ASC false false users bar false 2 name ASC false false +query TTBITTBB colnames +SHOW INDEXES FROM users_dupe +---- +table_name index_name non_unique seq_in_index column_name direction storing implicit +users_dupe primary false 1 id ASC false false +users_dupe foo true 1 name ASC false false +users_dupe foo true 2 id ASC false true +users_dupe bar false 1 id ASC false false +users_dupe bar false 2 name ASC false false + statement error index name "bar" already exists ALTER INDEX users@foo RENAME TO bar @@ -29,11 +51,27 @@ ALTER INDEX users@foo RENAME TO "" statement error index "ffo" does not exist ALTER INDEX users@ffo RENAME TO ufo +statement error index "ffo" does not exist +ALTER INDEX ffo RENAME TO ufo + +statement error index name "foo" is ambiguous +ALTER INDEX foo RENAME TO ufo + +statement error index name "foo" is ambiguous +ALTER INDEX IF EXISTS foo RENAME TO ufo + statement ok ALTER INDEX IF EXISTS users@ffo RENAME TO ufo +# Regression test for #42399. +statement ok +ALTER INDEX IF EXISTS ffo RENAME TO ufo + +statement ok +ALTER INDEX users@foo RENAME TO ufooo + statement ok -ALTER INDEX users@foo RENAME TO ufoo +ALTER INDEX IF EXISTS ufooo RENAME TO ufoo statement ok ALTER INDEX ufoo RENAME TO ufo diff --git a/pkg/sql/parser/parse_test.go b/pkg/sql/parser/parse_test.go index 8b11d79b1787..80249d030c08 100644 --- a/pkg/sql/parser/parse_test.go +++ b/pkg/sql/parser/parse_test.go @@ -1100,6 +1100,7 @@ func TestParse(t *testing.T) { {`EXPLAIN ALTER INDEX b RENAME TO b`}, {`ALTER INDEX a@b RENAME TO b`}, {`ALTER INDEX a@primary RENAME TO like`}, + {`ALTER INDEX IF EXISTS b RENAME TO b`}, {`ALTER INDEX IF EXISTS a@b RENAME TO b`}, {`ALTER INDEX IF EXISTS a@primary RENAME TO like`}, diff --git a/pkg/sql/rename_index.go b/pkg/sql/rename_index.go index db7b6d871f85..ae6b1f389394 100644 --- a/pkg/sql/rename_index.go +++ b/pkg/sql/rename_index.go @@ -34,10 +34,14 @@ type renameIndexNode struct { // notes: postgres requires CREATE on the table. // mysql requires ALTER, CREATE, INSERT on the table. func (p *planner) RenameIndex(ctx context.Context, n *tree.RenameIndex) (planNode, error) { - _, tableDesc, err := expandMutableIndexName(ctx, p, n.Index, true /* requireTable */) + _, tableDesc, err := expandMutableIndexName(ctx, p, n.Index, !n.IfExists /* requireTable */) if err != nil { return nil, err } + if tableDesc == nil { + // IfExists specified and table did not exist -- noop. + return newZeroNode(nil /* columns */), nil + } idx, _, err := tableDesc.FindIndexByName(string(n.Index.Index)) if err != nil { From fa4cbe53fc637d07bf361fb359dd158d51d41ae4 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Tue, 26 Nov 2019 16:24:17 -0500 Subject: [PATCH 2/4] sql: add more tests to drop_index logic test Test edge cases around ambiguous, non-existing, and/or unqualified index names, to reach test parity with alter_index logic test. Release note: None --- .../logictest/testdata/logic_test/drop_index | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/pkg/sql/logictest/testdata/logic_test/drop_index b/pkg/sql/logictest/testdata/logic_test/drop_index index 745fb4cf63a7..bc42b4ceff35 100644 --- a/pkg/sql/logictest/testdata/logic_test/drop_index +++ b/pkg/sql/logictest/testdata/logic_test/drop_index @@ -13,7 +13,9 @@ CREATE TABLE users ( statement ok CREATE TABLE othertable ( x INT, - INDEX baw (x) + y INT, + INDEX baw (x), + INDEX yak (y, x) ) statement error index name "baw" is ambiguous @@ -22,6 +24,27 @@ DROP INDEX baw statement error index name "baw" is ambiguous DROP INDEX IF EXISTS baw +statement error index "ark" does not exist +DROP INDEX ark + +statement ok +DROP INDEX IF EXISTS ark + +statement error index "ark" does not exist +DROP INDEX users@ark + +statement ok +DROP INDEX IF EXISTS users@ark + +statement ok +DROP INDEX yak + +statement ok +CREATE INDEX yak ON othertable (y, x) + +statement ok +DROP INDEX IF EXISTS yak + statement ok DROP TABLE othertable From 4b4e7da8ed06bda68db0dba6bf62a29600baf7ec Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Wed, 27 Nov 2019 10:21:58 -0800 Subject: [PATCH 3/4] colexec: fix a bug with top K sort not handling K > 1024 Previously, top K sorter when emitting the data would copy always from the beginning of the stored vectors. This is incorrect behavior when K is greater than coldata.BatchSize(), and now this has been fixed. Release note (bug fix): A bug with incorrect handling of Top K sort by the vectorized engine when K is greater than 1024 has been fixed. --- pkg/sql/colexec/sort_test.go | 6 +++--- pkg/sql/colexec/sorttopk.go | 9 +++++---- pkg/sql/logictest/testdata/logic_test/vectorize | 15 +++++++++++++++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/pkg/sql/colexec/sort_test.go b/pkg/sql/colexec/sort_test.go index 68489d083f47..f668906ba3dd 100644 --- a/pkg/sql/colexec/sort_test.go +++ b/pkg/sql/colexec/sort_test.go @@ -147,9 +147,9 @@ func TestSort(t *testing.T) { func TestSortRandomized(t *testing.T) { defer leaktest.AfterTest(t)() rng, _ := randutil.NewPseudoRand() - nTups := 1025 - k := uint16(4) - maxCols := 5 + nTups := coldata.BatchSize()*2 + 1 + k := uint16(rng.Intn(int(nTups))) + 1 + maxCols := 3 // TODO(yuzefovich): randomize types as well. typs := make([]coltypes.T, maxCols) for i := range typs { diff --git a/pkg/sql/colexec/sorttopk.go b/pkg/sql/colexec/sorttopk.go index a67e51da0aac..648e6600f436 100644 --- a/pkg/sql/colexec/sorttopk.go +++ b/pkg/sql/colexec/sorttopk.go @@ -214,10 +214,11 @@ func (t *topKSorter) emit() coldata.Batch { vec, coldata.CopySliceArgs{ SliceArgs: coldata.SliceArgs{ - ColType: t.inputTypes[i], - Src: t.topK.ColVec(i), - Sel: t.sel, - SrcEndIdx: uint64(toEmit), + ColType: t.inputTypes[i], + Src: t.topK.ColVec(i), + Sel: t.sel, + SrcStartIdx: uint64(t.emitted), + SrcEndIdx: uint64(t.emitted + toEmit), }, }, ) diff --git a/pkg/sql/logictest/testdata/logic_test/vectorize b/pkg/sql/logictest/testdata/logic_test/vectorize index ee57373c26b5..ea3eaf05e991 100644 --- a/pkg/sql/logictest/testdata/logic_test/vectorize +++ b/pkg/sql/logictest/testdata/logic_test/vectorize @@ -991,3 +991,18 @@ NULL NULL 1 NULL 1 1 1 NULL 0 1 1 2 + +# Regression test for #42816 - top K sort when K is greated than +# coldata.BatchSize(). +statement ok +CREATE TABLE t_42816 (a int); INSERT INTO t_42816 SELECT * FROM generate_series(0, 1025) + +query I +SELECT * FROM t_42816 ORDER BY a OFFSET 1020 LIMIT 10 +---- +1020 +1021 +1022 +1023 +1024 +1025 From 4925add55f6f3b26a95962ec84d03875e16bd58d Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Wed, 27 Nov 2019 10:55:52 -0800 Subject: [PATCH 4/4] workload, roachtest: skip TPCH Q4 for vec on TPCH query 4 in some cases can hit a memory limit error, so we will skip it for now (until the disk spilling is put in place). Release note: None --- pkg/cmd/roachtest/tpchvec.go | 2 ++ pkg/workload/querybench/tpch-queries-vec | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/roachtest/tpchvec.go b/pkg/cmd/roachtest/tpchvec.go index e8bc9bc83893..e7ac7b4144ed 100644 --- a/pkg/cmd/roachtest/tpchvec.go +++ b/pkg/cmd/roachtest/tpchvec.go @@ -37,6 +37,7 @@ func registerTPCHVec(r *testRegistry) { // TODO(yuzefovich): remove this once we have disk spilling. 1: "needs disk spilling", 3: "needs disk spilling", + 4: "needs disk spilling", 8: "needs disk spilling", 9: "needs disk spilling", 19: "needs disk spilling", @@ -47,6 +48,7 @@ func registerTPCHVec(r *testRegistry) { // vec on. 1: "skipped with vec on", 3: "skipped with vec on", + 4: "skipped with vec on", 8: "skipped with vec on", 9: "skipped with vec on", 19: "skipped with vec on", diff --git a/pkg/workload/querybench/tpch-queries-vec b/pkg/workload/querybench/tpch-queries-vec index 520f4873ea26..9a4f835fa0ba 100644 --- a/pkg/workload/querybench/tpch-queries-vec +++ b/pkg/workload/querybench/tpch-queries-vec @@ -2,7 +2,7 @@ -- understands. It is a subset of TPCH queries that is currently supported by -- vectorized execution engine. --- TODO(yuzefovich): we skip queries 1, 3, 9 because they need disk spilling. +-- TODO(yuzefovich): we skip queries 1, 3, 4, 9 because they need disk spilling. -- query 1 -- SELECT l_returnflag, l_linestatus, sum(l_quantity) AS sum_qty, sum(l_extendedprice) AS sum_base_price, sum(l_extendedprice * (1 - l_discount)) AS sum_disc_price, sum(l_extendedprice * (1 - l_discount) * (1 + l_tax)) AS sum_charge, avg(l_quantity) AS avg_qty, avg(l_extendedprice) AS avg_price, avg(l_discount) AS avg_disc, count(*) AS count_order FROM lineitem WHERE l_shipdate <= DATE '1998-12-01' - INTERVAL '90' DAY GROUP BY l_returnflag, l_linestatus ORDER BY l_returnflag, l_linestatus @@ -11,7 +11,7 @@ -- SELECT l_orderkey, sum(l_extendedprice * (1 - l_discount)) AS revenue, o_orderdate, o_shippriority FROM customer, orders, lineitem WHERE c_mktsegment = 'BUILDING' AND c_custkey = o_custkey AND l_orderkey = o_orderkey AND o_orderDATE < DATE '1995-03-15' AND l_shipdate > DATE '1995-03-15' GROUP BY l_orderkey, o_orderdate, o_shippriority ORDER BY revenue DESC, o_orderdate LIMIT 10 -- query 4 -SELECT o_orderpriority, count(*) AS order_count FROM orders WHERE o_orderdate >= DATE '1993-07-01' AND o_orderdate < DATE '1993-07-01' + INTERVAL '3' MONTH AND EXISTS ( SELECT * FROM lineitem WHERE l_orderkey = o_orderkey AND l_commitDATE < l_receiptdate) GROUP BY o_orderpriority ORDER BY o_orderpriority +-- SELECT o_orderpriority, count(*) AS order_count FROM orders WHERE o_orderdate >= DATE '1993-07-01' AND o_orderdate < DATE '1993-07-01' + INTERVAL '3' MONTH AND EXISTS ( SELECT * FROM lineitem WHERE l_orderkey = o_orderkey AND l_commitDATE < l_receiptdate) GROUP BY o_orderpriority ORDER BY o_orderpriority -- query 5 SELECT n_name, sum(l_extendedprice * (1 - l_discount)) AS revenue FROM customer, orders, lineitem, supplier, nation, region WHERE c_custkey = o_custkey AND l_orderkey = o_orderkey AND l_suppkey = s_suppkey AND c_nationkey = s_nationkey AND s_nationkey = n_nationkey AND n_regionkey = r_regionkey AND r_name = 'ASIA' AND o_orderDATE >= DATE '1994-01-01' AND o_orderDATE < DATE '1994-01-01' + INTERVAL '1' YEAR GROUP BY n_name ORDER BY revenue DESC