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

Add method to Query to be able to retrieve the Values #1700

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

tehsphinx
Copy link
Contributor

@tehsphinx tehsphinx commented May 27, 2023

In the context of gocql itself there is little point for this method. But when building wrappers for Query the values cannot be accessed by the wrapper. This becomes interesting for wrappers that implement functionality like BindStruct. A wrapper would need to keep track of the values itself. While this is possible, a wrapper of a wrapper cannot access the values any more if neither qocql nor the wrapper expose them.

This little addition to the library would help significantly in the creation of custom build wrappers that want to rely on the stability of this package as a foundation.

@martin-sucha
Copy link
Contributor

Hi!

Thank you for the pull request!

It is not clear to me why a wrapper needs to read the bound values, could you please elaborate?

For example gocqlx does not need to retrieve the values in BindStruct, it only sets the values:

https://github.com/scylladb/gocqlx/blob/dec046bd85e6a33abcc66eeb5dc61ae71c9bb132/queryx.go#L120-L132

Could you please post example code how the Values method would be used and what kind of wrapper it enables?

@tehsphinx
Copy link
Contributor Author

tehsphinx commented May 30, 2023

Hey @martin-sucha 👋

I'm writing a wrapper on gocqlx.Queryx and I can't access the Values to serialize them. Current use case is to send data changes (INSERT/UPDATE/DELETE) that fail (even with a retry policy) to a pulsar queue to be rerun later or looked at in the dead letter queue if they continue to fail.

I'm using an observer for this now (as I can't access the values in a wrapper). Exposing the Values would enable wrappers to be build more easily and more powerful.

Note: I'd have the same problem trying to centrally add this logic by wrapping gocql.Query. The only way one seems to have access to the values is with the String() method. Parsing that wouldn't be a very good idea.

@martin-sucha martin-sucha merged commit e943e60 into apache:master Jun 12, 2023
@martin-sucha
Copy link
Contributor

Seems that a similar thing can't easily be added outside of the package and it does not impact the API in a significant way. Merged. Thanks!

@tehsphinx tehsphinx deleted the values_access branch July 20, 2023 14:51
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.

2 participants