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: fix a non-deterministic DELETE ... USING logic test #90251

Merged

Conversation

faizaanmadhani
Copy link
Contributor

@faizaanmadhani faizaanmadhani commented Oct 19, 2022

Previously, a logic test testing ORDER BY and LIMIT in DELETE ... USING statements would sometimes return a different row from the USING table when run through stress tests, due to the ORDER BY ... LIMIT clause allowing the USING to join on either of two rows. This fix changes the output columns of the RETURNING to ensure it only returns columns from the target table and ensures deterministic output.

Release note: None

Epic: CRDB-5498

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@faizaanmadhani faizaanmadhani requested review from mgartner and a team October 19, 2022 15:47
Copy link
Collaborator

@rytaft rytaft 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 r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @faizaanmadhani and @mgartner)


-- commits line 14 at r1:
If the linter complains, you might need to write this as "Epic: CRDB-5498"

@faizaanmadhani faizaanmadhani force-pushed the faizaan/delete-from-using-hotfix branch from c63273e to 4bc3617 Compare October 19, 2022 15:51
@rytaft
Copy link
Collaborator

rytaft commented Oct 19, 2022

-- commits line 14 at r1:

Previously, rytaft (Rebecca Taft) wrote…

If the linter complains, you might need to write this as "Epic: CRDB-5498"

Oh I see you already changed it in the PR message -- feel free to ignore!

Previously, a logic test testing ORDER BY and LIMIT in
DELETE ... USING statements would sometimes return a different
row from the USING table when run through stress tests,
due to the ORDER BY ... LIMIT clause allowing the USING to join
on either of two rows. This fix changes the output columns of
the RETURNING to ensure it only returns columns from the target
table and ensures deterministic output.

Release note: None

Epic: CRDB-5498
@faizaanmadhani faizaanmadhani force-pushed the faizaan/delete-from-using-hotfix branch from 4bc3617 to 1404540 Compare October 19, 2022 15:56
@faizaanmadhani
Copy link
Contributor Author

TFTRs!

bors r+

@craig
Copy link
Contributor

craig bot commented Oct 19, 2022

Build succeeded:

@craig craig bot merged commit 0dbe60e into cockroachdb:master Oct 19, 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.

4 participants