Skip to content

Commit 47f7c9e

Browse files
mp911demp911de
committed
Polishing.
Introduce TermFactory.canBindCollection() to avoid unecessary parameter encoding. See #1172 Original pull request: #1178.
1 parent 81daf61 commit 47f7c9e

File tree

5 files changed

+113
-43
lines changed

5 files changed

+113
-43
lines changed

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/StatementFactory.java

+41-33
Original file line numberDiff line numberDiff line change
@@ -944,63 +944,65 @@ private static Relation toClause(CriteriaDefinition criteriaDefinition, TermFact
944944
() -> new IllegalArgumentException(String.format("Unknown operator [%s]", predicate.getOperator())));
945945

946946
ColumnRelationBuilder<Relation> column = Relation.column(columnName);
947+
Object value = predicate.getValue();
948+
947949
switch (predicateOperator) {
948950

949951
case EQ:
950-
return column.isEqualTo(factory.create(predicate.getValue()));
952+
return column.isEqualTo(factory.create(value));
951953

952954
case NE:
953-
return column.isNotEqualTo(factory.create(predicate.getValue()));
955+
return column.isNotEqualTo(factory.create(value));
954956

955957
case GT:
956-
return column.isGreaterThan(factory.create(predicate.getValue()));
958+
return column.isGreaterThan(factory.create(value));
957959

958960
case GTE:
959-
return column.isGreaterThanOrEqualTo(factory.create(predicate.getValue()));
961+
return column.isGreaterThanOrEqualTo(factory.create(value));
960962

961963
case LT:
962-
return column.isLessThan(factory.create(predicate.getValue()));
964+
return column.isLessThan(factory.create(value));
963965

964966
case LTE:
965-
return column.isLessThanOrEqualTo(factory.create(predicate.getValue()));
967+
return column.isLessThanOrEqualTo(factory.create(value));
966968

967969
case IN:
968970

969-
if (predicate.getValue() instanceof List
970-
|| (predicate.getValue() != null && predicate.getValue().getClass().isArray())) {
971-
Term term = factory.create(predicate.getValue());
972-
if (term instanceof BindMarker) {
973-
return column.in((BindMarker) term);
971+
if (isCollectionLike(value)) {
972+
973+
if (factory.canBindCollection()) {
974+
Term term = factory.create(value);
975+
return term instanceof BindMarker ? column.in((BindMarker) term) : column.in(term);
974976
}
975977

976-
return column.in(toLiterals(predicate.getValue()));
978+
return column.in(toLiterals(value));
977979
}
978980

979-
return column.in(factory.create(predicate.getValue()));
981+
return column.in(factory.create(value));
980982

981983
case LIKE:
982-
return column.like(factory.create(predicate.getValue()));
984+
return column.like(factory.create(value));
983985

984986
case IS_NOT_NULL:
985987
return column.isNotNull();
986988

987989
case CONTAINS:
988990

989-
Assert.state(predicate.getValue() != null,
991+
Assert.state(value != null,
990992
() -> String.format("CONTAINS value for column %s is null", columnName));
991993

992-
return column.contains(factory.create(predicate.getValue()));
994+
return column.contains(factory.create(value));
993995

994996
case CONTAINS_KEY:
995997

996-
Assert.state(predicate.getValue() != null,
998+
Assert.state(value != null,
997999
() -> String.format("CONTAINS KEY value for column %s is null", columnName));
9981000

999-
return column.containsKey(factory.create(predicate.getValue()));
1001+
return column.containsKey(factory.create(value));
10001002
}
10011003

10021004
throw new IllegalArgumentException(
1003-
String.format("Criteria %s %s %s not supported", columnName, predicate.getOperator(), predicate.getValue()));
1005+
String.format("Criteria %s %s %s not supported", columnName, predicate.getOperator(), value));
10041006
}
10051007

10061008
private static Condition toCondition(CriteriaDefinition criteriaDefinition, TermFactory factory) {
@@ -1014,43 +1016,45 @@ private static Condition toCondition(CriteriaDefinition criteriaDefinition, Term
10141016
() -> new IllegalArgumentException(String.format("Unknown operator [%s]", predicate.getOperator())));
10151017

10161018
ConditionBuilder<Condition> column = Condition.column(columnName);
1019+
Object value = predicate.getValue();
1020+
10171021
switch (predicateOperator) {
10181022

10191023
case EQ:
1020-
return column.isEqualTo(factory.create(predicate.getValue()));
1024+
return column.isEqualTo(factory.create(value));
10211025

10221026
case NE:
1023-
return column.isNotEqualTo(factory.create(predicate.getValue()));
1027+
return column.isNotEqualTo(factory.create(value));
10241028

10251029
case GT:
1026-
return column.isGreaterThan(factory.create(predicate.getValue()));
1030+
return column.isGreaterThan(factory.create(value));
10271031

10281032
case GTE:
1029-
return column.isGreaterThanOrEqualTo(factory.create(predicate.getValue()));
1033+
return column.isGreaterThanOrEqualTo(factory.create(value));
10301034

10311035
case LT:
1032-
return column.isLessThan(factory.create(predicate.getValue()));
1036+
return column.isLessThan(factory.create(value));
10331037

10341038
case LTE:
1035-
return column.isLessThanOrEqualTo(factory.create(predicate.getValue()));
1039+
return column.isLessThanOrEqualTo(factory.create(value));
10361040

10371041
case IN:
10381042

1039-
if (predicate.getValue() instanceof List
1040-
|| (predicate.getValue() != null && predicate.getValue().getClass().isArray())) {
1041-
Term term = factory.create(predicate.getValue());
1042-
if (term instanceof BindMarker) {
1043-
return column.in((BindMarker) term);
1043+
if (isCollectionLike(value)) {
1044+
1045+
if (factory.canBindCollection()) {
1046+
Term term = factory.create(value);
1047+
return term instanceof BindMarker ? column.in((BindMarker) term) : column.in(term);
10441048
}
10451049

1046-
return column.in(toLiterals(predicate.getValue()));
1050+
return column.in(toLiterals(value));
10471051
}
10481052

1049-
return column.in(factory.create(predicate.getValue()));
1053+
return column.in(factory.create(value));
10501054
}
10511055

10521056
throw new IllegalArgumentException(String.format("Criteria %s %s %s not supported for IF Conditions", columnName,
1053-
predicate.getOperator(), predicate.getValue()));
1057+
predicate.getOperator(), value));
10541058
}
10551059

10561060
static List<Term> toLiterals(@Nullable Object arrayOrList) {
@@ -1084,6 +1088,10 @@ static List<Term> toLiterals(@Nullable Object arrayOrList, Function<Object, Term
10841088
return Collections.emptyList();
10851089
}
10861090

1091+
private static boolean isCollectionLike(@Nullable Object value) {
1092+
return value instanceof List || (value != null && value.getClass().isArray());
1093+
}
1094+
10871095
static class SimpleSelector implements com.datastax.oss.driver.api.querybuilder.select.Selector {
10881096

10891097
private final String selector;

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/StatementBuilder.java

+11-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,17 @@ public SimpleStatement build(ParameterHandling parameterHandling, CodecRegistry
206206

207207
if (parameterHandling == ParameterHandling.INLINE) {
208208

209-
TermFactory termFactory = value -> toLiteralTerms(value, codecRegistry);
209+
TermFactory termFactory = new TermFactory() {
210+
@Override
211+
public Term create(@Nullable Object value) {
212+
return toLiteralTerms(value, codecRegistry);
213+
}
214+
215+
@Override
216+
public boolean canBindCollection() {
217+
return false;
218+
}
219+
};
210220

211221
for (BuilderRunnable<S> runnable : queryActions) {
212222
statement = runnable.run(statement, termFactory);

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/TermFactory.java

+12-2
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@
1515
*/
1616
package org.springframework.data.cassandra.core.cql.util;
1717

18-
import com.datastax.oss.driver.api.querybuilder.term.Term;
19-
2018
import org.springframework.lang.Nullable;
2119

20+
import com.datastax.oss.driver.api.querybuilder.term.Term;
21+
2222
/**
2323
* Factory for {@link Term} objects encapsulating a binding {@code value}. Classes implementing this factory interface
2424
* may return inline terms to render values as part of the query string, or bind markers to supply parameters on
@@ -39,4 +39,14 @@ public interface TermFactory {
3939
* @return the {@link Term} for the given {@code value}.
4040
*/
4141
Term create(@Nullable Object value);
42+
43+
/**
44+
* Check whether the term factory accepts {@link java.util.Collection} values to be created as {@link Term}.
45+
*
46+
* @return {@code true} whether the term factory can {@link java.util.Collection} values.
47+
* @since 3.1.14
48+
*/
49+
default boolean canBindCollection() {
50+
return true;
51+
}
4252
}

‎spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/StatementFactoryUnitTests.java

+47-5
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.springframework.data.domain.Sort.Direction.*;
2020

2121
import java.time.Duration;
22+
import java.util.Arrays;
2223
import java.util.Collections;
2324
import java.util.List;
2425
import java.util.Map;
@@ -43,6 +44,7 @@
4344
import org.springframework.data.cassandra.domain.Group;
4445
import org.springframework.data.domain.Sort;
4546

47+
import com.datastax.oss.driver.api.core.CqlIdentifier;
4648
import com.datastax.oss.driver.api.core.DefaultConsistencyLevel;
4749
import com.datastax.oss.driver.api.core.cql.SimpleStatement;
4850
import com.datastax.oss.driver.api.querybuilder.delete.Delete;
@@ -171,14 +173,54 @@ void shouldMapSelectQueryWithLimitAndAllowFiltering() {
171173
.isEqualTo("SELECT * FROM group LIMIT 10 ALLOW FILTERING");
172174
}
173175

174-
@Test
175-
void shouldMapSelectInQuery() {
176+
@Test // GH-1172
177+
void shouldMapSelectInQueryAsInlineValue() {
176178

177-
Query query = Query.query(Criteria.where("foo").in("bar"));
178-
179-
StatementBuilder<Select> select = statementFactory.select(query, groupEntity);
179+
StatementBuilder<Select> select = statementFactory.select(Query.query(Criteria.where("foo").in("bar")),
180+
groupEntity);
180181

181182
assertThat(select.build(ParameterHandling.INLINE).getQuery()).isEqualTo("SELECT * FROM group WHERE foo IN ('bar')");
183+
184+
select = statementFactory.select(Query.query(Criteria.where("foo").in("bar", "baz")), groupEntity);
185+
186+
assertThat(select.build(ParameterHandling.INLINE).getQuery())
187+
.isEqualTo("SELECT * FROM group WHERE foo IN ('bar','baz')");
188+
}
189+
190+
@Test // GH-1172
191+
void shouldMapSelectInQueryAsByIndexValue() {
192+
193+
StatementBuilder<Select> select = statementFactory.select(Query.query(Criteria.where("foo").in("bar")),
194+
groupEntity);
195+
SimpleStatement statement = select.build(ParameterHandling.BY_INDEX);
196+
197+
assertThat(statement.getQuery()).isEqualTo("SELECT * FROM group WHERE foo IN ?");
198+
assertThat(statement.getPositionalValues()).containsOnly(Collections.singletonList("bar"));
199+
200+
select = statementFactory.select(Query.query(Criteria.where("foo").in("bar", "baz")), groupEntity);
201+
statement = select.build(ParameterHandling.BY_INDEX);
202+
203+
assertThat(statement.getQuery()).isEqualTo("SELECT * FROM group WHERE foo IN ?");
204+
assertThat(statement.getPositionalValues()).containsOnly(Arrays.asList("bar", "baz"));
205+
}
206+
207+
@Test // GH-1172
208+
void shouldMapSelectInQueryAsByNamedValue() {
209+
210+
StatementBuilder<Select> select = statementFactory.select(Query.query(Criteria.where("foo").in("bar")),
211+
groupEntity);
212+
SimpleStatement statement = select.build(ParameterHandling.BY_NAME);
213+
214+
assertThat(statement.getQuery()).isEqualTo("SELECT * FROM group WHERE foo IN :p0");
215+
assertThat(statement.getNamedValues()).hasSize(1).containsEntry(CqlIdentifier.fromCql("p0"),
216+
Collections.singletonList("bar"));
217+
218+
select = statementFactory.select(Query.query(Criteria.where("foo").in("bar", "baz")), groupEntity);
219+
statement = select.build(ParameterHandling.BY_NAME);
220+
221+
assertThat(statement.getQuery()).isEqualTo("SELECT * FROM group WHERE foo IN :p0");
222+
assertThat(statement.getNamedValues()).hasSize(1).containsEntry(CqlIdentifier.fromCql("p0"),
223+
(Arrays.asList("bar", "baz")));
182224
}
183225

184226
@Test // DATACASS-343

‎spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/query/PartTreeCassandraQueryUnitTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ void shouldDeriveFieldInCollectionQuery() {
158158
String query = deriveQueryFromMethod(Repo.class, "findByFirstnameIn", new Class[] { Collection.class },
159159
Arrays.asList("Hank", "Walter")).getQuery();
160160

161-
assertThat(query).isEqualTo("SELECT * FROM person WHERE firstname IN ?");
161+
assertThat(query).isEqualTo("SELECT * FROM person WHERE firstname IN ('Hank','Walter')");
162162
}
163163

164164
@Test // DATACASS-172
@@ -184,7 +184,7 @@ void shouldDeriveUdtInCollectionQuery() {
184184
String query = deriveQueryFromMethod(Repo.class, "findByMainAddressIn", new Class[] { Collection.class },
185185
Collections.singleton(udtValue)).getQuery();
186186

187-
assertThat(query).isEqualTo("SELECT * FROM person WHERE mainaddress IN ?");
187+
assertThat(query).isEqualTo("SELECT * FROM person WHERE mainaddress IN ({city:NULL,country:NULL})");
188188
}
189189

190190
@Test // DATACASS-343

0 commit comments

Comments
 (0)