fix: query id for TERMINATE should be case insensitive #5005
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
fixes: #5003
The query id passed to
TERMINATE <queryId>
is currently case-sensitive. So, for example,May work, but:
Won't.
CSAS and CTAS create queries with upper case ids. However,
INSERT INTO
creates mixed case queries, e.g.InsertQuery_3
. Again, users must get the case right to terminate them.SQL is meant to be case-insensitive by default! Hence this change switches terminate to be case-insensitive.
Testing done
Because the command topics of users will contain existing queries with query ids in upper and mixed case, we need to be a little careful any fix doesn't stop users from terminating historic queries or meaning restores leave previously terminated queries running. There is also another open PR that we need to ensure works with any change made here: #5002
To this end I went through the following steps, and ensured each worked, (this is also roughly the commit list):
QueryId
internally case-insensitive, but externally case-preserving, i.e. a queryId ofID
andid
would generate the same hash code and return true fromequals
, but we returnID
andid
fromtoString
. This is the least invasive fix. I ran both the master branch and feat: speed up restarts by not building topologies for terminated queries #5002 to ensure this wouldn't cause any issues elsewhere in the code.RecoveryTest
to have some terminates with the wrong case.QueryId
to be always uppercase.It was not easily achievable to update
QueryId
to just uppercase its query id, as the text representation is used to, among other things, build the names for internal topics.Reviewer checklist