Skip to content

Commit

Permalink
sqlsmith: fix flaky TestGenerateParse
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mgartner committed Jun 23, 2021
1 parent 3b8b689 commit 9a24944
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 3 deletions.
1 change: 0 additions & 1 deletion pkg/internal/sqlsmith/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ go_test(
"//pkg/server",
"//pkg/sql/parser",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/util/leaktest",
Expand Down
8 changes: 8 additions & 0 deletions pkg/internal/sqlsmith/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,14 @@ func (s *Smither) extractIndexes(
if err := rows.Err(); err != nil {
return nil, err
}
// Remove indexes with empty Columns. This is the case for rowid indexes
// where the only index column, rowid, is ignored in the SQL statement
// above, but the stored columns are not.
for name, idx := range indexes {
if len(idx.Columns) == 0 {
delete(indexes, name)
}
}
ret[*t.TableName] = indexes
}
return ret, nil
Expand Down
2 changes: 0 additions & 2 deletions pkg/internal/sqlsmith/sqlsmith_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
"github.com/cockroachdb/cockroach/pkg/sql/parser"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/randutil"
Expand Down Expand Up @@ -140,7 +139,6 @@ func TestRandTableInserts(t *testing.T) {
// sometimes put them into bad states that the parser would never do.
func TestGenerateParse(t *testing.T) {
defer leaktest.AfterTest(t)()
skip.WithIssue(t, 66723, "flaky test")
defer utilccl.TestingEnableEnterprise()()

ctx := context.Background()
Expand Down

0 comments on commit 9a24944

Please sign in to comment.