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

cql3/prepare_expr: force token() receiver name to be partition key token #13815

Merged
merged 2 commits into from
May 10, 2023

Conversation

cvybhu
Copy link
Contributor

@cvybhu cvybhu commented May 9, 2023

Let's say that we have a prepared statement with a token restriction:

SELECT * FROM some_table WHERE token(p1, p2) = ?

After calling prepare the drivers receives some information about the prepared statment, including names of values bound to each bind marker.

In case of a partition token restriction (token(p1, p2) = ?) there's an expectation that the name assigned to this bind marker will be "partition key token".

In a recent change the code handling token() expressions has been unified with the code that handles generic function calls, and as a result the name has changed to token(p1, p2).

It turns out that the Java driver relies on the name being "partition key token", so a change to token(p1, p2) broke some things.

This patch sets the name back to "partition key token". To achieve this we detect any restrictions that match the pattern token(p1, p2, p3) = X and set the receiver name for X to "partition key token".

Fixes: #13769

@cvybhu cvybhu requested a review from nyh as a code owner May 9, 2023 10:32
cvybhu added 2 commits May 9, 2023 12:32
Let's say that we have a prepared statement with a token restriction:
```cql
SELECT * FROM some_table WHERE token(p1, p2) = ?
```

After calling `prepare` the drivers receives some information
about the prepared statment, including names of values bound
to each bind marker.

In case of a partition token restriction (`token(p1, p2) = ?`)
there's an expectation that the name assigned to this bind marker
will be `"partition key token"`.

In a recent change the code handling `token()` expressions has been
unified with the code that handles generic function calls,
and as a result the name has changed to `token(p1, p2)`.

It turns out that the Java driver relies on the name being
`"partition key token"`, so a change to `token(p1, p2)`
broke some things.

This patch sets the name back to `"partition key token"`.
To achieve this we detect any restrictions that match
the pattern `token(p1, p2, p3) = X` and set the receiver
name for X to `"partition key token"`.

Fixes: scylladb#13769

Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
When preparing a query each bind marker gets a name.
For a query like:
```cql
SELECT * FROM some_table WHERE token(p1, p2) = ?
```
The bind marker's name should be `"partition key token"`.
Java driver relies on this name, having something else,
like `"token(p1, p2)"` be the name breaks the Java driver.

Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Copy link
Contributor

@fruch fruch left a comment

Choose a reason for hiding this comment

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

LGTM

@scylladb-promoter
Copy link
Contributor

@mykaul mykaul added this to the 5.3 milestone May 9, 2023
@scylladb-promoter scylladb-promoter merged commit 996f717 into scylladb:master May 10, 2023
3 checks passed
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.

"partition key token" name changed, break java-driver test
4 participants