Closed spring-projects-issues closed 4 years ago
Mark Paluch commented
Thanks for your ticket. CachedPreparedStatementCreator
is String-query based and we currently don't plan to ship a feature release on the 1.x line. The caching is broken although it works in most cases. It uses an intrinsic cache that is held static final
. Please rather use an own caching implementation for now.
It would make sense for the upcoming 2.0 version, to extract a common ground for cache support so you can build an own implementation
James Howe commented
That's fine, I can probably just do it with a subclass for now
Dmitro Lisnichenko commented
It would be great if there was an option for a @Query
annotation or a separate annotation for query method in Repository for marking this method as idempotent.
Mark Paluch commented
I pushed a newly designed CachedPreparedStatementCreator
to https://github.com/spring-projects/spring-data-cassandra/tree/issue/DATACASS-403.
Care to give it a try? I'd love to get some feedback and what you think about it.
<dependency>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-cassandra</artifactId>
<version>2.0.0.DATACASS-403-SNAPSHOT</version>
</dependency>
<repository>
<id>spring-libs-snapshot</id>
<name>Spring Snapshot Repository</name>
<url>http://repo.spring.io/libs-snapshot</url>
</repository>
James Howe commented
The idea is you inject a PreparedStatementCache
bean and use it with CachedPreparedStatementCreator.of
?
Compared to the current version, subclasses cannot read the fields.
I have added a PreparedStatementCreator withConsistency(ConsistencyLevel cl)
, which looks harder to do with this new one.
Can you add protected getters maybe?
Mark Paluch commented
Thanks a lot for your feedback. It's up to you, whether you inject PreparedStatementCache
or manage the state yourself. Dependency injection would make sense if Cluster
/Session
are managed within the same scope to align lifetime duration.
Regarding query options, I'd propose to use QueryOptions
/WriteOptions
that apply the options to the created statement.
Regarding getters, I'm ok with adding a getter for the cache, as the cache might be of interest for outside users. As a subclass, you can intercept the values you're interested in within the constructor.
Does this make sense?
James Howe commented
Regarding query options My error handling reads a lot better with the fluent interface I added to change the consistency, especially as I never override any of the other options.
you can intercept the values you're interested in within the constructor Indeed, but that's more work for me, and I'd just be duplicating all the fields it already has
Mark Paluch commented
How about decorating CachedPreparedStatementCreator
with an own PreparedStatementCreator
? You can alter the input/output to your needs without interfering with the state of CachedPreparedStatementCreator
itself?
We will cache prepared statements using the CQL-text only without regard to statement flags. The first prepared statements flags will apply to all prepared statements retrieved from CachedPreparedStatementCreator
. You will be able to pass in QueryOptions
when creating CachedPreparedStatementCreator
from plain CQL. Decorations lets you post-process the result
James Howe commented
I don't want to change what a single creator returns, I want an entirely new creator derived from the first one. The first design was fine for that, it's just protected field access would have made it a bit simpler
Mark Paluch commented
Exposing the internal state violates encapsulation. We don't want to expose these elements any further. Otherwise, we're constrained to additional API we can't change any more and the API isn't related to the exposed contract of a caching PreparedStatementCreator
.
What's the reason for sticking to a subclass of CachedPreparedStatementCreator
instead of having a class that implements PreparedStatementCreator
and delegates calls to CachedPreparedStatementCreator
after pre-processing the input parameters?
John Blum commented
Reviewed, polished and merged! Thank you!
James Howe opened DATACASS-403 and commented
The driver version 3.1 changed how query idempotence is determined.
Any
BuiltStatements
from aQueryBuilder
will automatically determine their idempotence, but because they must be converted toString
in order to use the cache, this information is lost.If there were an alternate constructor that took a
Statement
instead of aString
, then the idempotence of the statement could be carried over to the prepared and bound statementsReference URL: http://docs.datastax.com/en/drivers/java/3.1/com/datastax/driver/core/Statement.html#isIdempotent--
Issue Links:
Referenced from: pull request https://github.com/spring-projects/spring-data-cassandra/pull/105