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

Apply configured CodecRegistry to StatementBuilder #1114

Closed
aakashacharya177 opened this issue Apr 8, 2021 · 6 comments
Closed

Apply configured CodecRegistry to StatementBuilder #1114

aakashacharya177 opened this issue Apr 8, 2021 · 6 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@aakashacharya177
Copy link

aakashacharya177 commented Apr 8, 2021

I am using spring data cassandra 3.1.6 artifact and trying to use custom codec.

The insert method in StatementFactory class is not passing converter/codec registry to the statement builder class and hence insert query is not being formed properly.

StatementBuilder builder = StatementBuilder.of(QueryBuilder.insertInto(tableName).valuesByIds(Collections.emptyMap())).bind((statement, factory) -> {
Map<CqlIdentifier, Term> values = createTerms(insertNulls, object, factory);
return statement.valuesByIds(values);
}).apply((statement) -> {
return (RegularInsert)addWriteOptions((Insert)statement, options);
});

I am using spring data stax oss driver version 4.9.0

Can you please look into this and let us know if this has been fixed or we have a workaround for the same?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 8, 2021
@mp911de
Copy link
Member

mp911de commented Apr 8, 2021

Right now, we only support CodecRegistry.DEFAULT. We have around 53 occurrences that call StatementBuilder.build(…).

A majority of the work could be done by introducing a build(StatementBuilder) method on the template API level where you can plugin in your own CodecRegistry.

An alternative approach could be enhancing StatementFactory.of(…) to make it accept CodecRegistry. All calls to StatementFactory.of(…) originate from StatementFactory and CassandraConverter is already associated with a CodecRegistry so that seems the better approach.

@mp911de mp911de added status: ideal-for-contribution An issue that a contributor can help us with type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 8, 2021
@aakashacharya177
Copy link
Author

Thanks for the response Mark.
Approach 2 seems to be a better approach where we can use the same codec registry of Cassandra converter.

We would be waiting for this enhancement. Can we get any tentative release plan?

@mp911de
Copy link
Member

mp911de commented Apr 12, 2021

Submitting a pull request can speed up things. That is a change that we could include also in a service release. Check out https://calendar.spring.io/ for the release schedule.

@mipo256
Copy link
Contributor

mipo256 commented Jan 1, 2024

@mp911de We already can supply the CodecRegistry to StatementBuilder. So is this issue still relevant?
Although there are still places where we do not allow CodecRegistry customization, such is CassandraSimpleTypeHolder. Do not know if it is necessary or not though.

@mp911de
Copy link
Member

mp911de commented Jan 2, 2024

All invocations to StatementBuilder use build() instead of build(ParameterHandling, CodecRegistry).

This ticket doesn't discuss the ability to specify CodecRegistry at StatementBuilder but the actual propagation of CodecRegistry. StatementBuilder should be extended also with a build(CodecRegistry) method to retain ParameterHandling defaulting.

Another thing that needs to be clarified is where CodecRegistry comes from. It could stem from CqlOperations, from SessionFactory or be configured on the CassandraTemplate level.

None is ideal, really. The most ideal approach would be to run StatementBuilder.build(CodecRegistry) within CqlOperations.execute(SessionCallback). While that would work well in the imperative style, any async or reactive code paths would be required to accept overhead of creating reactive/async wrapper types.

Happy to hear more opinions so that we can find the best approach.

@mp911de mp911de changed the title Pass CodecRegistry to StatementBuilder in CassandraTemplate Pass CodecRegistry to StatementBuilder in CassandraTemplate Jan 2, 2024
@mp911de mp911de changed the title Pass CodecRegistry to StatementBuilder in CassandraTemplate Use configured CodecRegistry in StatementBuilder Sep 9, 2024
@mp911de
Copy link
Member

mp911de commented Sep 9, 2024

After revisiting this request, I found that using the actual CodecRegistry from the session is rather difficult. It is much easier (and makes more sense) to initialize StatementBuilder with the CodecRegistry configured at CassandraConverter as StatementFactory (the place that creates all StatementBuilder instances) has access to the converter already.

@mp911de mp911de removed the status: ideal-for-contribution An issue that a contributor can help us with label Sep 9, 2024
@mp911de mp911de added this to the 4.4 M1 (2024.1.0) milestone Sep 9, 2024
@mp911de mp911de self-assigned this Sep 9, 2024
@mp911de mp911de changed the title Use configured CodecRegistry in StatementBuilder Apply configured CodecRegistry to StatementBuilder Sep 9, 2024
@mp911de mp911de closed this as completed in 66a0bf9 Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants