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

Prepared statement with IN(?) does not work #866

Closed
RoDmitry opened this issue Dec 3, 2023 · 9 comments
Closed

Prepared statement with IN(?) does not work #866

RoDmitry opened this issue Dec 3, 2023 · 9 comments

Comments

@RoDmitry
Copy link
Contributor

RoDmitry commented Dec 3, 2023

Code:

let data = vec![0_i16, 1, 2, 3, 4]; // changed to i32, i64
let ps = db
    .prepare("SELECT some FROM t WHERE pk = 1 AND ck IN(?)")
    .await?;
let res = db
    .execute(&ps, (data,))
    .await
    .unwrap();

Error if ck is smallint:

called `Result::unwrap()` on an `Err` value: DbError(Invalid, "Exception while binding column \"value(in(lang_id))\": marshaling error: Expected 2 bytes for a smallint (34)")

Error if ck is int:

called `Result::unwrap()` on an `Err` value: DbError(Invalid, "Exception while binding column \"value(in(lang_id))\": marshaling error: Validation failed for type org.apache.cassandra.db.marshal.Int32Type: got 44 bytes")

Error if ck is bigint:

called `Result::unwrap()` on an `Err` value: DbError(Invalid, "Exception while binding column \"value(in(lang_id))\": marshaling error: Validation failed for type org.apache.cassandra.db.marshal.LongType: got 64 bytes")

Table:

CREATE TABLE t (
      pk bigint,
      ck smallint,
      some text,
      PRIMARY KEY (pk, ck)
);

What am I doing wrong? Or have it been always broken?

@piodul
Copy link
Collaborator

piodul commented Dec 4, 2023

You just need to drop the parentheses around the question mark:

SELECT some FROM t WHERE pk = 1 AND ck IN ?

Otherwise, IIUC the (?) in IN (?) is interpreted as a single-element tuple. You are passing a list, but the DB expects a single element for the bind marker - hence the error.

The next release will do type checking before sending values to the DB and should have more helpful error messages.

@RoDmitry
Copy link
Contributor Author

RoDmitry commented Dec 4, 2023

Thank you a lot! It works now.

@RoDmitry RoDmitry closed this as completed Dec 4, 2023
@mykaul
Copy link
Contributor

mykaul commented Dec 4, 2023

Are we missing an example perhaps?

@nyh
Copy link

nyh commented Dec 4, 2023

I wonder if the Rust driver could have detected that he's passing the wrong type for a bound variable, and refused to send the request instead of sending it and the server complaining in that unfriendly way.

If I remember correctly, the Python driver has such checks. It often frustrates me when I write tests (because I can't send incorrect types to the server to check how the server handles them), but for real users - such client-side checks can be helpful as we see here.

@piodul
Copy link
Collaborator

piodul commented Dec 4, 2023

Are we missing an example perhaps?

@mykaul The main issue here is that the CQL syntax for one-element tuples is confusing. It's not specific to the driver. Perhaps the meaning of IN (?) vs. IN ? should be clarified somewhere in the OSS Scylla docs.

Another thing is that the driver does not type check data before sending and the errors returned from the DB are confusing. This is being worked on and will be fixed in the next release (#463). The driver will report a message like:

Expected a smallint, got list<smallint>

While this does not suggest a solution, it at least points users in the right direction.

@piodul
Copy link
Collaborator

piodul commented Dec 4, 2023

I wonder if the Rust driver could have detected that he's passing the wrong type for a bound variable, and refused to send the request instead of sending it and the server complaining in that unfriendly way.

@nyh Yes, it is possible. After preparing a statement, Scylla returns information about all of the bind markers, including their names and types. We intend to use it to implement type checking.

@RoDmitry
Copy link
Contributor Author

RoDmitry commented Dec 4, 2023

Are we missing an example perhaps?

@mykaul I think so, I have tried to find it, but couldn't.

@steptron
Copy link

Please add an example somewhere. I ran into the same issue. There is no example in the docs or in the examples folder. The only way I could fix it, is by coming across this issue, which took many search attempts to get here. Thanks

@Lorak-mmk
Copy link
Collaborator

Lorak-mmk commented Mar 26, 2024

Sure, we can add an example - but primarly I'd like the error messages to be clear enough that users won't need to search the internet for the solution.
I checked how the example from this issue looks now. The error I got is:

Error: Serializing values failed: SerializationError: Failed to serialize query arguments (alloc::vec::Vec<i16>,): failed to serialize column value(in(b)): SerializationError: Failed to type check Rust type alloc::vec::Vec<i16> against CQL type Int: the CQL type the tuple was attempted to was neither a set or a list

Apart from the obvious mistake in the last sentence (which we need to fix), it does say Failed to type check Rust type alloc::vec::Vec<i16> against CQL type Int - which I thought points quite well towards what's the problem, but I'm probably wrong.
How should a good error message look like in this case?

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

No branches or pull requests

6 participants