Open Lorak-mmk opened 8 months ago
I'm convinced that with the new strongly typed serialization framework we really need BoundStatement
for both compatibility and performance purposes.
I believe we should work on it together with exploring possible implementation of Executable
trait (#713), so that both those new things are compatible.
After releasing serialization refactor several users reported use cases that are not well-supported by our new APIs. The problems come from the need to have specific (non-generic) owned type for query values. Previously
SerializedValues
was such a type. You could use it to serialize values ahead of time and then pass it around as you wish. After refactor you would need to use types likeVec<Box<dyn SerializeCql>>
orBox<dyn SerializeRow>
, depending on your use case. There are some disadvantagesVec<Box<dyn SerializeCql>>
and put that in a box - that's a lot more allocations and indirections.So in short: migration to new API is possible in every case (I think), but sometimes the performance cost is high.
I propose to introduce new query type (like
Query
andPreparedStatement
):BoundStatement
. This is a prepared statement that also stores serialized values. This would allow users to serialize parameters ahead of time while remaining type-safe and avoiding unnecessary allocations.My first idea for an API is a a
bind(&self, values: impl SerializeRow) -> BoundStatement
method onPreparedStatement
. We can also think about allowing to bind values one by one. This would complicate the API and implementation and introduce some runtime errors (e.g. if user first binds value by order and then by name), but would be more flexible.Main problem I see is how to adapt our Session API to this - creating another set of methods, like for
Query
andPreparedStatement
seems bloaty. There are more reasons to work on our Session API (potentially removingQuery
with values, bad naming etc), so I think we should discuss Session API changes in https://github.com/scylladb/scylla-rust-driver/issues/713 Right now probably the most reasonable solution that would allow us to supportBoundStatement
isExecutable
trait suggested in this issue.Relevant issues / discussions: https://github.com/scylladb/scylla-rust-driver/issues/890 https://github.com/scylladb/scylla-rust-driver/issues/713 https://scylladb-users.slack.com/archives/C01D7LCL44D/p1708948081839569