Skip to content

sql: add assertion for txn passed to internal executor#88295

Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom
ZhouXing19:add-assert-ie-0920
Sep 26, 2022
Merged

sql: add assertion for txn passed to internal executor#88295
craig[bot] merged 2 commits intocockroachdb:masterfrom
ZhouXing19:add-assert-ie-0920

Conversation

@ZhouXing19
Copy link
Copy Markdown
Collaborator

@ZhouXing19 ZhouXing19 commented Sep 20, 2022

If an internal executor is created with a txn binding to it, it should not be used to execute a statement with a nil txn.

Release note: None

@ZhouXing19 ZhouXing19 requested a review from ajwerner September 20, 2022 20:46
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@ZhouXing19 ZhouXing19 changed the title sql/internalExecutor: add assertion for txn sql: add assertion for txn passed to internal executor Sep 20, 2022
@ZhouXing19 ZhouXing19 requested a review from a team September 21, 2022 02:14
Previously, the internal executor always run queries with a nil txn, even though
it's constructed with a txn binding to it.

We now change it to pass the txn to the internal executor in
`findNextTableToUpgrade()`

Release note: None
If a internal executor is created with a txn binding to it, it should not be
used to execute statement with a nil txn.

Release note: None
@ZhouXing19
Copy link
Copy Markdown
Collaborator Author

RFAL, thanks!

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:

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

@ZhouXing19
Copy link
Copy Markdown
Collaborator Author

Thanks!
bors r=ajwerner

@craig
Copy link
Copy Markdown
Contributor

craig Bot commented Sep 26, 2022

Build succeeded:

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