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.

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

sql/randgen: avoid invalid inverted indexes in CREATE TABLE #85742

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

fqazi
Copy link
Collaborator

@fqazi fqazi commented Aug 8, 2022

Previously, the create table made via the randgen component
could create invalid CREATE TABLE statements where the last
columns of an inverted index were descending. This could
lead to unexpected failures in certain workloads, which
expected valid statements from this components. To address
this, this patch stops randgen from generating CREATE table
statements with invalid inverted indexes.

Release note: None

@fqazi fqazi requested a review from a team August 8, 2022 14:22
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@fqazi
Copy link
Collaborator Author

fqazi commented Aug 8, 2022

This eliminates one workload-related failure at least in the randomized schema changer workload, and probably helps out others too.

Copy link
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.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @fqazi)


pkg/sql/randgen/schema.go line 175 at r1 (raw file):

	nIdxs := rng.Intn(10)
	for i := 0; i < nIdxs; i++ {
		indexDef, ok := randIndexTableDefFromCols(rng, columnDefs, tableName, false /* isPrimaryIndex */, isMultiRegion)

Should we instead change this function to not set the direction illegally if it chooses to create an inverted index?

Previously, the create table made via the randgen component
could create invalid CREATE TABLE statements where the last
columns of an inverted index were descending. This could
lead to unexpected failures in certain workloads, which
expected valid statements from this components. To address
this, this patch stops randgen from generating CREATE table
statements with invalid inverted indexes.

Release note: None
@fqazi fqazi force-pushed the invertedIdxWrkloadFix branch from 5eb35aa to 16e5d55 Compare August 8, 2022 14:55
Copy link
Collaborator Author

@fqazi fqazi left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)


pkg/sql/randgen/schema.go line 175 at r1 (raw file):

Previously, ajwerner wrote…

Should we instead change this function to not set the direction illegally if it chooses to create an inverted index?

Good point, changed this to fix the generator instead of skipping over them.

@fqazi fqazi requested a review from ajwerner August 8, 2022 14:56
Copy link
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:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @fqazi)

@fqazi
Copy link
Collaborator Author

fqazi commented Aug 8, 2022

@ajwerner TFTR!

@fqazi
Copy link
Collaborator Author

fqazi commented Aug 8, 2022

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 8, 2022

Build succeeded:

@craig craig bot merged commit d58473e into cockroachdb:master Aug 8, 2022
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.

3 participants