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

sqlsmith: add DELETE FROM ... USING and UPDATE ... FROM support #98914

Merged
merged 1 commit into from
Mar 22, 2023

Conversation

yuzefovich
Copy link
Member

@yuzefovich yuzefovich commented Mar 18, 2023

This commit makes it so that the sqlsmith can now generate statements of the form DELETE FROM ... USING and UPDATE ... FROM. We toss a coin every time before deciding to add another table (meaning in 50% cases these forms are not used, in 25% we have 1 extra table, etc). It also adjusts the generation of the RETURNING clause for DELETE and UPDATE to be able to pick from any of the table sources.

Fixes: #98910.

Release note: None

@yuzefovich yuzefovich requested review from mgartner, michae2 and a team March 18, 2023 02:03
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@yuzefovich yuzefovich force-pushed the sqlsmith-delete-update branch from bf1aa42 to 584202d Compare March 18, 2023 02:05
Copy link
Member Author

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

I used cmd/smith to see how the new statements look (thanks Michael for that addition!), and they looked reasonable. Should I also actually run something (a command or a unit test) so that generated queries are executed against some DB, or is it ok to just let the nightly tests do that for us?

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

Copy link
Collaborator

@michae2 michae2 left a comment

Choose a reason for hiding this comment

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

:lgtm:

If you really want to run against a DB you could do something like:

# first shell
cockroach demo movr

# second shell
smith -url 'postgresql://demo:[email protected]:26257/movr?options=...'

But it's fine with me if we want to let the nightly tests try this out.

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

Copy link
Collaborator

@mgartner mgartner left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! :lgtm:

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

This commit makes it so that the sqlsmith can now generate statements
of the form `DELETE FROM ... USING` and `UPDATE ... FROM`. We toss
a coin every time before deciding to add another table (meaning in 50%
cases these forms are not used, in 25% we have 1 extra table, etc). It
also adjusts the generation of the RETURNING clause for DELETE and
UPDATE to be able to pick from any of the table sources.

Release note: None
@yuzefovich yuzefovich force-pushed the sqlsmith-delete-update branch from 584202d to c3304de Compare March 21, 2023 21:48
@yuzefovich
Copy link
Member Author

TFTRs!

I'll let nightly run tell us if anything is wrong (and will also backport to 23.1 but after a few days of baking on master).

bors r+

@craig
Copy link
Contributor

craig bot commented Mar 22, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 22, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 22, 2023

Build succeeded:

@craig craig bot merged commit 704f5ad into cockroachdb:master Mar 22, 2023
@yuzefovich yuzefovich deleted the sqlsmith-delete-update branch March 22, 2023 16:25
@yuzefovich
Copy link
Member Author

This seems to have found one bug on master, but otherwise didn't destabilize the tests, so I think it's worth backporting to 23.1.

@yuzefovich
Copy link
Member Author

blathers backport 23.1

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.

sqlsmith: generate statements of the form DELETE FROM ... USING and UPDATE ... FROM
4 participants