Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

opt: forbid USING HASH in optimizer tests #99129

Closed
mgartner opened this issue Mar 21, 2023 · 0 comments · Fixed by #100311
Closed

opt: forbid USING HASH in optimizer tests #99129

mgartner opened this issue Mar 21, 2023 · 0 comments · Fixed by #100311
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. T-sql-queries SQL Queries Team

Comments

@mgartner
Copy link
Collaborator

mgartner commented Mar 21, 2023 •
edited by cockroach-jira-scripts
Loading

Tests added in #86622 query a table with indexes that are supposed to be hash-sharded (they use the USING HASH syntactic sugar):

cockroach/pkg/sql/opt/xform/testdata/rules/join

Line 12879 in 554b1dd

CREATE TABLE u85353 (a INT, b INT, INDEX (a,b) USING HASH, INDEX (b) USING HASH)

The test catalog ignores this syntax, so the tests are not testing what they are supposed to be testing. We should:

  1. Fix these tests so that the indexes are manually hash-sharded, rather than using the USING HASH syntactic sugar.
  2. Make the test catalog panic if it encounters the USING HASH syntactic sugar so that we don't make this same mistake in the future.

Jira issue: CRDB-25714
@mgartner mgartner added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. labels Mar 21, 2023
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Mar 21, 2023
@mgartner mgartner mentioned this issue Mar 31, 2023
Merged
craig bot pushed a commit that referenced this issue Mar 31, 2023
@michae2 @mgartner
Merge #100246 #100311
3dc28b1 
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]>
@craig craig bot closed this as completed in 6760c29 Mar 31, 2023
@mgartner mgartner moved this to Done in SQL Queries Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-quick-win Likely to be a quick win for someone experienced. T-sql-queries SQL Queries Team
Projects
SQL Queries
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

opt: prohibit hash-sharded index syntactic sugar in test catalog mgartner/cockroach
1 participant
@mgartner