Skip to content

sqlsmith: fix flaky TestGenerateParse#66743

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
mgartner:66723-fix-sqlsmith-flake
Jun 24, 2021
Merged

sqlsmith: fix flaky TestGenerateParse#66743
craig[bot] merged 1 commit intocockroachdb:masterfrom
mgartner:66723-fix-sqlsmith-flake

Conversation

@mgartner
Copy link
Copy Markdown
Contributor

This commit fixes the sqlsmith.TestGenerateParse test that has been
flaking since #66599 when primary index stored columns were added to the
output of SHOW INDEXES.

sqlsmith uses SHOW INDEXES to build an in-memory representation of
indexes to help generate interesting randomized queries. It is not
equipped to handle rowid primary indexes, so rows output from
SHOW INDEXES with a column_name of rowid are filtered out. Stored
columns of primary indexes, now included in SHOW INDEXES, are not
filtered out, causing sqlsmith to build an in-memory representation of
rowid indexes that have only stored columns and no indexed columns.
This breaks the assumption within sqlsmith that each index should have
at least one indexed column, causing panics.

This commit fixes the issue by removing indexes with no indexed columns
from sqlsmith's in-memory index map.

Fixes #66723

Release note: None

@mgartner mgartner requested review from a team and postamar June 22, 2021 22:39
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@mgartner
Copy link
Copy Markdown
Contributor Author

I'll update this PR to unskip TestGenerateParse once #66736 is merged.

This commit fixes the `sqlsmith.TestGenerateParse` test that has been
flaking since cockroachdb#66599 when primary index stored columns were added to the
output of `SHOW INDEXES`.

sqlsmith uses `SHOW INDEXES` to build an in-memory representation of
indexes to help generate interesting randomized queries. It is not
equipped to handle `rowid` primary indexes, so rows output from
`SHOW INDEXES` with a `column_name` of `rowid` are filtered out. Stored
columns of primary indexes, now included in `SHOW INDEXES`, are not
filtered out, causing sqlsmith to build an in-memory representation of
`rowid` indexes that have only stored columns and no indexed columns.
This breaks the assumption within sqlsmith that each index should have
at least one indexed column, causing panics.

This commit fixes the issue by removing indexes with no indexed columns
from sqlsmith's in-memory index map.

Fixes cockroachdb#66723

Release note: None
@mgartner mgartner force-pushed the 66723-fix-sqlsmith-flake branch from c455fba to 9a24944 Compare June 23, 2021 02:02
@mgartner
Copy link
Copy Markdown
Contributor Author

I'll update this PR to unskip TestGenerateParse once #66736 is merged.

Done.

Copy link
Copy Markdown
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

bors r=ajwerner

Reviewed 3 of 3 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @postamar)

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jun 24, 2021

Build succeeded:

@craig craig bot merged commit 663a44a into cockroachdb:master Jun 24, 2021
@mgartner mgartner deleted the 66723-fix-sqlsmith-flake branch June 29, 2021 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

internal/sqlsmith: "index out of range" panic in makeMergeJoinExprwith

3 participants