sql: attempt to reduce duplication in legacy schema changer txn management #91567
Conversation
|
This change is |
There was a problem hiding this comment.
This stuff is crufty. This seems fine, though sort of shocking how repetitive the code is around these parts. If somebody didn't know better, they might assume it was for a reason, but I don't think it is.
What if we re-arranged things like in this commit?
Reviewed 1 of 1 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @ZhouXing19)
There was a problem hiding this comment.
What if we re-arranged things like in this commit?
Wow thanks for writing this up. Yeah it makes sense to have things all tracing down to txnWithExecutor. Do you want me to close this PR or cherry-pick your commit?
Reviewable status:
|
No strong preferences, cherry-pick works for me. |
…ement These functions can all be defined in terms of `txnWithExecutor`. It was confusing that they were not. Release note: None
ZhouXing19
force-pushed
the
fixed-timestamp-ie-runner
branch
from
0b468c0
to
e03be7b
Compare
ZhouXing19
changed the title
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status:
|
Thanks! |
|
Build succeeded: |
These functions can all be defined in terms of
txnWithExecutor. It wasconfusing that they were not.
Epic: none
Release note: None