Skip to content

Conversation

@maswin
Copy link
Member

@maswin maswin commented Oct 7, 2025

Description

Partial cancel url path is of form /v1/statement/executing/partialCancel/<query_id>/...
Ref - https://github.com/trinodb/trino/blob/master/core/trino-main/src/main/java/io/trino/server/protocol/Query.java#L726

But in the code we are considering it to be of form - /v1/statement/partialCancel/<query_id>/... without the executing prefix which causes failures while extracting query id. Fixed it with some minor refactoring.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
(X) Release notes are required, with the following suggested text:

* bug fix: Fix partialCancel statement path redirection

@cla-bot cla-bot bot added the cla-signed label Oct 7, 2025
|| tokens[1].equals("scheduled")
|| tokens[1].equals("executing")
|| tokens[1].equals("partialCancel")) {
if (tokens.length >= 3 && QUERY_STATE_PATH.contains(tokens[1])) {
Copy link
Member

@vishalya vishalya Oct 21, 2025

Choose a reason for hiding this comment

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

I am fine with the current fix to go in as is, but we should definitely refactor this block of the code as some point.

@maswin
Copy link
Member Author

maswin commented Oct 23, 2025

@ebyhr - Is this good to be merged?

@ebyhr ebyhr merged commit 30c57ab into trinodb:main Oct 25, 2025
3 checks passed
@github-actions github-actions bot added this to the 17 milestone Oct 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants