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

StatementFactory should accept keyspace for statement creation #1275

Closed
KasperDeng opened this issue Jun 5, 2022 · 6 comments
Closed

StatementFactory should accept keyspace for statement creation #1275

KasperDeng opened this issue Jun 5, 2022 · 6 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@KasperDeng
Copy link

KasperDeng commented Jun 5, 2022

Background

Cassanda introduced a new prepare statement behavior in February 2022. The behavior will makes some incompatible changes and would a prepared statement id mismatch for existing clients in most case of upgrade scenario with traffic requests.
The new prepare statement behavior required the cassandra client to have a fully qualified table name (format: keyspace.table) in CQL for statement preparation.

There is a configuration to force the new behavior of preparing statement, force_new_prepared_statement_behaviour. But from version 3.0.26, 3.11.12, 4.0.2, 4.1, the behavior is enforced forcefully.

If the client is not in such behavior, there are some warning logs in Cassandra service side.

ClientWarn.instance.warn(String.format("`USE <keyspace>` with prepared statements is considered to be an anti-pattern due to ambiguity in non-qualified table names. " + "Please consider removing instances of `Session#setKeyspace(<keyspace>)`, `Session#execute(\"USE <keyspace>\")` and `cluster.newSession(<keyspace>)` from your code, and " + "always use fully qualified table names (e.g. <keyspace>.<table>). " + "Keyspace used: %s, statement keyspace: %s, statement id: %s", getRawKeyspace(),preparedKeyspace, statementId));
state.getClientState().warnAboutUseWithPreparedStatements(statementId, prepared.keyspace);
String msg = String.format("Tried to execute a prepared unqalified statement on a keyspace it was not prepared on. " +
  " Executing the resulting prepared statement will return unexpected results: %s (on keyspace %s, previously prepared on %s)",
statementId, state.getClientState().getRawKeyspace(), prepared.keyspace);
nospam.error(msg);

Problems in spring-data-cassandra

Spring data cassandra provides a highly abstraction to developer to access the Cassandra database, especially the data repository makes the code clean and hide the details of the cassandra driver.
But for the case, it looks the underlay cql is based on the StatementFactory, which creates the StatementBuilder by Cassandra's driver's class QueryBuilder. But only uses its API (selectFrom, insert, insertInto, update, deleteFrom) with table name, not the API with keyspace.
Then the keyspace attribute is null in DefaultSelect, DefaultInsert, DefaultUpdate, DefaultDelete, then the CqlHelper.qualify cannot build the keyspace in the cql in asCql() of above classes.

Candidate Solutions

A possible solution in current our application is to clone such StatementFactory class and use those QueryBuilder APIs with keyspace parameter, and injects such StatementFactory instance to ReactiveCassandraTemplate based on Spring bean wire mechanism.

Can spring-data-cassandra have some mechanism to improve this part?
BTW, should the cassandra entity have @keyspace annotation which is also support the SpEL for runtime rendering mechanism, besides the @table annotation.
And the @keyspace can be integrated with the @query annotation or named queries as well?

Reference

Don't consider current keyspace in prepared statement id when the query is qualified
Fix Prepared Statements behaviours after 15252

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 5, 2022
@mp911de mp911de self-assigned this Jul 25, 2022
@mp911de
Copy link
Member

mp911de commented Jul 27, 2022

I'm not fully sure I understand what you're asking for. Do you look for a way to associate a keyspace when CassandraTemplate builds CQL statements using SimpleStatement.setKeyspace or a way to insert the keyspace when we build the actual CQL statement?

We have an open pull request to customize the keyspace for entities, see #179.

It turned out that adding keyspace details requires a bit more thought and we hadn't had the bandwidth to follow up. We do not want to introduce a @Keyspace annotation as the database name/keyspace name is a configuration concern that should not be defined within the data model but rather in a place where it can be changed for deployment customization.

@mp911de mp911de added the status: waiting-for-feedback We need additional information before we can continue label Jul 27, 2022
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Aug 3, 2022
@keyspace

This comment was marked as off-topic.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Aug 5, 2022
@KasperDeng
Copy link
Author

Hi, sorry that may be a bit complicate in above issue description. I try to make it simple here.

The idea here just want spring-data-cassandra to help make the native CQL with fully qualify name "." in the CQL instead of only "<tableName" in the CQL. For those methods in ReactiveCrudRepository, spring-data-cassandra will help generate the CQL based on the user defined entity. But the final CQL generated by StatementFactory only has table name there.

I see Apache Cassandra Server recommending client to have a fully qualified table name (with keyspace name) to do query operation. That's what I want.

Not sure the idea is simple to understand or not now. :)

@mp911de mp911de changed the title Cassandra Introduces New Behavior to Involve Fully Qualified Table Name (with Keyspace) in CQL in Prepare Statement StatementFactory should accept keyspace for statement creation Jun 8, 2023
@mp911de mp911de added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Jun 8, 2023
@mp911de
Copy link
Member

mp911de commented Jun 8, 2023

Related: #179

@mp911de mp911de added this to the 4.4 M1 (2024.1.0) milestone Jul 24, 2024
@mp911de
Copy link
Member

mp911de commented Jul 24, 2024

That is in place now. Feel free to give the snapshots a try.

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