Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
100246: sql: deflake distsql_automatic_stats logic test r=rytaft,yuzefovich a=michae2

**sql: fix formatting of CREATE TABLE AS with storage parameters**

We were placing the storage parameters in the wrong place when formatting `CREATE TABLE AS` statements.

Fixes: #100243

Epic: None

Release note: None

**sql: deflake distsql_automatic_stats logic test**

Now that automatic stats are disabled for system tables in logic tests, `distsql_automatic_stats` was opening the floodgates with its first `SET CLUSTER SETTING sql.stats.automatic_collection.enabled = true`. With this enabled, the automatic stats refresher was executing `CREATE STATISTICS` for every system table, one at a time, often taking longer than the 45s logic test retry limit to get to the user-defined table we really care about.

Instead of using the cluster setting, change `distsql_automatic_stats` to use the per-table setting.

Fixes: #99751

Epic: None

Release note: None

100311: opt: prohibit hash-sharded index syntactic sugar in test catalog r=mgartner a=mgartner

The test catalog now panics when trying to build a hash-sharded index
instead of silently ignoring the `USING HASH` clause. This prevents
writing tests that incorrectly assume that `USING HASH` works as
expected in the test catalog. One test with `USING HASH` has been
updated.

Fixes #99129

Release note: None


Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
  • Loading branch information
3 people committed Mar 31, 2023
3 parents 08db74f + 5bfb0d9 + 6760c29 commit 3dc28b1
Show file tree
Hide file tree
Showing 13 changed files with 161 additions and 75 deletions.
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 9 additions & 23 deletions pkg/sql/logictest/testdata/logic_test/distsql_automatic_stats
Original file line number Diff line number Diff line change
@@ -1,19 +1,8 @@
# LogicTest: local !local !metamorphic-batch-sizes
# LogicTest: !metamorphic-batch-sizes

# TODO(yuzefovich): at the moment the tests below that assert a particular set
# of statistics are flaky, so we disable this file by using a contradictory
# config set. Once #99751 is addressed, "local !local" part should be removed.

# Disable automatic stats
# Enable automatic stats for this table.
statement ok
SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false

statement ok
CREATE TABLE data (a INT, b INT, c FLOAT, d DECIMAL, PRIMARY KEY (a, b, c), INDEX d_idx (d))

# Enable automatic stats
statement ok
SET CLUSTER SETTING sql.stats.automatic_collection.enabled = true
CREATE TABLE data (a INT, b INT, c FLOAT, d DECIMAL, PRIMARY KEY (a, b, c), INDEX d_idx (d)) WITH (sql_stats_automatic_collection_enabled = true)

# Generate all combinations of values 1 to 10.
statement ok
Expand All @@ -40,7 +29,7 @@ __auto__ {d} 1000 1 1000

# Disable automatic stats
statement ok
SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false
ALTER TABLE data SET (sql_stats_automatic_collection_enabled = false)

# Update more than 20% of the table.
statement ok
Expand All @@ -61,7 +50,7 @@ __auto__ {d} 1000 1 1000

# Enable automatic stats
statement ok
SET CLUSTER SETTING sql.stats.automatic_collection.enabled = true
ALTER TABLE data SET (sql_stats_automatic_collection_enabled = true)

# Update more than 20% of the table.
statement ok
Expand Down Expand Up @@ -129,7 +118,7 @@ __auto__ {d} 550 1 0

# Test CREATE TABLE ... AS
statement ok
CREATE TABLE copy AS SELECT * FROM data
CREATE TABLE copy WITH (sql_stats_automatic_collection_enabled = true) AS SELECT * FROM data

# Distinct count for rowid can be flaky, so don't show it. The estimate is
# almost always 550, but occasionally it is 549...
Expand All @@ -145,7 +134,7 @@ __auto__ {d} 550 0
__auto__ {rowid} 550 0

statement ok
CREATE TABLE test_create (x INT PRIMARY KEY, y CHAR)
CREATE TABLE test_create (x INT PRIMARY KEY, y CHAR) WITH (sql_stats_automatic_collection_enabled = true)

query TTIII colnames,retry
SELECT DISTINCT ON (column_names) statistics_name, column_names, row_count, distinct_count, null_count
Expand Down Expand Up @@ -174,16 +163,13 @@ __auto__ {rowid} 0 0
# Test user-defined schemas.

# Disable automatic stats
statement ok
SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false

statement ok
CREATE SCHEMA my_schema;
CREATE TABLE my_schema.my_table (k INT PRIMARY KEY, v STRING);
CREATE TABLE my_schema.my_table (k INT PRIMARY KEY, v STRING) WITH (sql_stats_automatic_collection_enabled = false)

# Enable automatic stats
statement ok
SET CLUSTER SETTING sql.stats.automatic_collection.enabled = true
ALTER TABLE my_schema.my_table SET (sql_stats_automatic_collection_enabled = true)

# Insert 10 rows.
statement ok
Expand Down
7 changes: 7 additions & 0 deletions pkg/sql/logictest/tests/fakedist-disk/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/sql/logictest/tests/fakedist/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/sql/logictest/tests/local-vec-off/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/sql/logictest/tests/local/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/sql/opt/testutils/testcat/create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,11 @@ func (tt *Table) addIndexWithVersion(
tt.addUniqueConstraint(def.Name, def.Columns, def.Predicate, false /* withoutIndex */)
}

// The test catalog does not support the hash-sharded index syntactic sugar.
if def.Sharded != nil {
panic("hash-sharded indexes are not supported by the test catalog")
}

idx := &Index{
IdxName: tt.makeIndexName(def.Name, def.Columns, typ),
Unique: typ != nonUniqueIndex,
Expand Down
131 changes: 80 additions & 51 deletions pkg/sql/opt/xform/testdata/rules/join
Original file line number Diff line number Diff line change
Expand Up @@ -12876,7 +12876,16 @@ CREATE TABLE t85353 (a INT, b INT)
----

exec-ddl
CREATE TABLE u85353 (a INT, b INT, INDEX (a,b) USING HASH, INDEX (b) USING HASH)
CREATE TABLE u85353 (
a INT,
b INT,
a_b_hash INT NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(a, b)), 8)) VIRTUAL,
b_hash INT NOT VISIBLE NOT NULL AS (mod(fnv32(crdb_internal.datums_to_bytes(b)), 8)) VIRTUAL,
INDEX a_b_idx (a_b_hash, a, b),
INDEX b_idx (b_hash, b),
CHECK (a_b_hash IN (0, 1, 2, 3, 4, 5, 6, 7)),
CHECK (b_hash IN (0, 1, 2, 3, 4, 5, 6, 7))
)
----

exec-ddl
Expand Down Expand Up @@ -12946,71 +12955,91 @@ ALTER TABLE u85353 INJECT STATISTICS
# should not reduce join selectivity and cause the following to
# choose lookup join.
opt
EXPLAIN (OPT) SELECT * FROM t85353 INNER JOIN u85353@u85353_b_idx USING (b) WHERE u85353.a < 10;
EXPLAIN (OPT) SELECT * FROM t85353 INNER JOIN u85353@b_idx USING (b) WHERE u85353.a < 10;
----
explain
├── columns: info:11
├── columns: info:13
├── mode: opt
├── immutable
└── project
├── columns: b:2!null a:1 a:6!null
└── inner-join (merge)
├── columns: t85353.a:1 t85353.b:2!null u85353.a:6!null u85353.b:7!null
├── left ordering: +7
├── right ordering: +2
├── fd: (2)==(7), (7)==(2)
├── select
│ ├── columns: u85353.a:6!null u85353.b:7
│ ├── ordering: +7
│ ├── index-join u85353
│ │ ├── columns: u85353.a:6 u85353.b:7
│ │ ├── ordering: +7
│ │ └── scan u85353@u85353_b_idx
│ │ ├── columns: u85353.b:7 u85353.rowid:8!null
│ │ ├── flags: force-index=u85353_b_idx
│ │ ├── key: (8)
│ │ ├── fd: (8)-->(7)
│ │ └── ordering: +7
│ └── filters
│ └── u85353.a:6 < 10 [outer=(6), constraints=(/6: (/NULL - /9]; tight)]
├── sort
│ ├── columns: t85353.a:1 t85353.b:2
│ ├── ordering: +2
│ └── scan t85353
│ └── columns: t85353.a:1 t85353.b:2
└── filters (true)
├── columns: b:2!null a:1 a:6!null a_b_hash:8 b_hash:9
├── immutable
├── fd: (2,6)-->(8), (2)-->(9)
└── inner-join (hash)
├── columns: t85353.a:1 t85353.b:2!null u85353.a:6!null u85353.b:7!null a_b_hash:8 b_hash:9
├── immutable
├── fd: (6,7)-->(8), (7)-->(9), (2)==(7), (7)==(2)
├── project
│ ├── columns: a_b_hash:8 b_hash:9 u85353.a:6!null u85353.b:7
│ ├── immutable
│ ├── fd: (6,7)-->(8), (7)-->(9)
│ ├── select
│ │ ├── columns: u85353.a:6!null u85353.b:7
│ │ ├── index-join u85353
│ │ │ ├── columns: u85353.a:6 u85353.b:7
│ │ │ └── scan u85353@b_idx
│ │ │ ├── columns: u85353.b:7 u85353.rowid:10!null
│ │ │ ├── constraint: /9/7/10: [/0 - /7]
│ │ │ ├── flags: force-index=b_idx
│ │ │ ├── key: (10)
│ │ │ └── fd: (10)-->(7)
│ │ └── filters
│ │ └── u85353.a:6 < 10 [outer=(6), constraints=(/6: (/NULL - /9]; tight)]
│ └── projections
│ ├── mod(fnv32(crdb_internal.datums_to_bytes(u85353.a:6, u85353.b:7)), 8) [as=a_b_hash:8, outer=(6,7), immutable]
│ └── mod(fnv32(crdb_internal.datums_to_bytes(u85353.b:7)), 8) [as=b_hash:9, outer=(7), immutable]
├── scan t85353
│ └── columns: t85353.a:1 t85353.b:2
└── filters
└── t85353.b:2 = u85353.b:7 [outer=(2,7), constraints=(/2: (/NULL - ]; /7: (/NULL - ]), fd=(2)==(7), (7)==(2)]

# The derived equijoin condition between the hash bucket column in
# u85353@u85353_a_b_idx and a similar hash bucket function expression on t85353.b
# should not reduce join selectivity and cause the following to
# choose lookup join.
opt
EXPLAIN (OPT) SELECT * FROM t85353 INNER JOIN u85353@u85353_a_b_idx USING (a,b) WHERE u85353.a < 10;
EXPLAIN (OPT) SELECT * FROM t85353 INNER JOIN u85353@a_b_idx USING (a,b) WHERE u85353.a < 10;
----
explain
├── columns: info:11
├── columns: info:13
├── mode: opt
├── immutable
└── project
├── columns: a:1!null b:2!null
└── inner-join (merge)
├── columns: t85353.a:1!null t85353.b:2!null u85353.a:6!null u85353.b:7!null
├── left ordering: +6,+7
├── right ordering: +1,+2
├── fd: (1)==(6), (6)==(1), (2)==(7), (7)==(2)
├── scan u85353@u85353_a_b_idx
│ ├── columns: u85353.a:6!null u85353.b:7
│ ├── constraint: /6/7/8: (/NULL - /9]
│ ├── flags: force-index=u85353_a_b_idx
│ └── ordering: +6,+7
├── sort
├── columns: a:1!null b:2!null a_b_hash:8 b_hash:9
├── immutable
├── fd: (1,2)-->(8), (2)-->(9)
└── inner-join (hash)
├── columns: t85353.a:1!null t85353.b:2!null u85353.a:6!null u85353.b:7!null a_b_hash:8 b_hash:9
├── immutable
├── fd: (6,7)-->(8), (7)-->(9), (1)==(6), (6)==(1), (2)==(7), (7)==(2)
├── project
│ ├── columns: a_b_hash:8 b_hash:9 u85353.a:6!null u85353.b:7
│ ├── immutable
│ ├── fd: (6,7)-->(8), (7)-->(9)
│ ├── scan u85353@a_b_idx
│ │ ├── columns: u85353.a:6!null u85353.b:7
│ │ ├── constraint: /8/6/7/10
│ │ │ ├── (/0/NULL - /0/9]
│ │ │ ├── (/1/NULL - /1/9]
│ │ │ ├── (/2/NULL - /2/9]
│ │ │ ├── (/3/NULL - /3/9]
│ │ │ ├── (/4/NULL - /4/9]
│ │ │ ├── (/5/NULL - /5/9]
│ │ │ ├── (/6/NULL - /6/9]
│ │ │ └── (/7/NULL - /7/9]
│ │ └── flags: force-index=a_b_idx
│ └── projections
│ ├── mod(fnv32(crdb_internal.datums_to_bytes(u85353.a:6, u85353.b:7)), 8) [as=a_b_hash:8, outer=(6,7), immutable]
│ └── mod(fnv32(crdb_internal.datums_to_bytes(u85353.b:7)), 8) [as=b_hash:9, outer=(7), immutable]
├── select
│ ├── columns: t85353.a:1!null t85353.b:2
│ ├── ordering: +1,+2
│ └── select
│ ├── columns: t85353.a:1!null t85353.b:2
│ ├── scan t85353
│ │ └── columns: t85353.a:1 t85353.b:2
│ └── filters
│ └── t85353.a:1 < 10 [outer=(1), constraints=(/1: (/NULL - /9]; tight)]
└── filters (true)
│ ├── scan t85353
│ │ └── columns: t85353.a:1 t85353.b:2
│ └── filters
│ └── t85353.a:1 < 10 [outer=(1), constraints=(/1: (/NULL - /9]; tight)]
└── filters
├── t85353.a:1 = u85353.a:6 [outer=(1,6), constraints=(/1: (/NULL - ]; /6: (/NULL - ]), fd=(1)==(6), (6)==(1)]
└── t85353.b:2 = u85353.b:7 [outer=(2,7), constraints=(/2: (/NULL - ]; /7: (/NULL - ]), fd=(2)==(7), (7)==(2)]

### BEGIN Regression tests for issue #69617

Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/parser/testdata/create_table
Original file line number Diff line number Diff line change
Expand Up @@ -2062,6 +2062,14 @@ CREATE TABLE IF NOT EXISTS a (x, y FAMILY f1) AS SELECT (*) FROM b -- fully pare
CREATE TABLE IF NOT EXISTS a (x, y FAMILY f1) AS SELECT * FROM b -- literals removed
CREATE TABLE IF NOT EXISTS _ (_, _ FAMILY _) AS SELECT * FROM _ -- identifiers removed

parse
CREATE TABLE a WITH (fillfactor=100) AS SELECT * FROM b
----
CREATE TABLE a WITH (fillfactor = 100) AS SELECT * FROM b -- normalized!
CREATE TABLE a WITH (fillfactor = (100)) AS SELECT (*) FROM b -- fully parenthesized
CREATE TABLE a WITH (fillfactor = _) AS SELECT * FROM b -- literals removed
CREATE TABLE _ WITH (_ = 100) AS SELECT * FROM _ -- identifiers removed

error
CREATE TABLE test (
foo INT8 FAMILY a FAMILY b
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/sem/tree/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -1524,6 +1524,11 @@ func (node *CreateTable) FormatBody(ctx *FmtCtx) {
ctx.FormatNode(&node.Defs)
ctx.WriteByte(')')
}
if node.StorageParams != nil {
ctx.WriteString(` WITH (`)
ctx.FormatNode(&node.StorageParams)
ctx.WriteByte(')')
}
ctx.WriteString(" AS ")
ctx.FormatNode(node.AsSource)
} else {
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/sem/tree/pretty.go
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,10 @@ func (node *CreateTable) doc(p *PrettyCfg) pretty.Doc {
title = pretty.ConcatSpace(title,
p.bracket("(", p.Doc(&node.Defs), ")"))
}
if node.StorageParams != nil {
title = pretty.ConcatSpace(title, pretty.Keyword("WITH"))
title = pretty.ConcatSpace(title, p.bracket(`(`, p.Doc(&node.StorageParams), `)`))
}
title = pretty.ConcatSpace(title, pretty.Keyword("AS"))
} else {
title = pretty.ConcatSpace(title,
Expand All @@ -1281,7 +1285,7 @@ func (node *CreateTable) doc(p *PrettyCfg) pretty.Doc {
if node.PartitionByTable != nil {
clauses = append(clauses, p.Doc(node.PartitionByTable))
}
if node.StorageParams != nil {
if node.StorageParams != nil && !node.As() {
clauses = append(
clauses,
pretty.ConcatSpace(
Expand Down

0 comments on commit 3dc28b1

Please sign in to comment.