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

JAVA-3109, JAVA-2980: Support binding collections of attributes in IN clause query builder #1935

Open
wants to merge 3 commits into
base: 4.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,17 @@ default boolean accepts(@NonNull DataType cqlType) {
@NonNull
String format(@Nullable JavaTypeT value);

/**
* Formats items from collection type as valid list of CQL literals.
*
* <p>Implementing this method is not strictly mandatory. Default implementation falls back to
* {@link #format(JavaTypeT)}. Method is used primarily for literal values in the query builder
* (see {@code DefaultLiteral#appendElementsTo(StringBuilder)}).
*/
default String formatElements(@Nullable JavaTypeT value) {
return format(value);
}

/**
* Parse the given CQL literal into an instance of the Java type handled by this codec.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZonedDateTime;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -252,6 +253,11 @@ public final boolean isArray() {
return token.isArray();
}

/** Returns true if this type is a Java collection (e.g. list or set). */
public boolean isCollection() {
return Collection.class.isAssignableFrom(token.getRawType());
}

/** Returns true if this type is one of the nine primitive types (including {@code void}). */
public final boolean isPrimitive() {
return token.isPrimitive();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,14 @@ public String format(@Nullable List<ElementT> value) {
return "NULL";
}
StringBuilder sb = new StringBuilder("[");
sb.append(formatElements(value));
sb.append("]");
return sb.toString();
}

@Override
public String formatElements(@Nullable List<ElementT> value) {
StringBuilder sb = new StringBuilder();
boolean first = true;
for (ElementT t : value) {
if (first) {
Expand All @@ -151,7 +159,6 @@ public String format(@Nullable List<ElementT> value) {
}
sb.append(elementCodec.format(t));
}
sb.append("]");
return sb.toString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ public String format(@Nullable Set<ElementT> value) {
return "NULL";
}
StringBuilder sb = new StringBuilder("{");
sb.append(formatElements(value));
sb.append("}");
return sb.toString();
}

@Override
public String formatElements(@Nullable Set<ElementT> value) {
StringBuilder sb = new StringBuilder();
boolean first = true;
for (ElementT t : value) {
if (first) {
Expand All @@ -152,7 +160,6 @@ public String format(@Nullable Set<ElementT> value) {
}
sb.append(elementCodec.format(t));
}
sb.append("}");
return sb.toString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,13 @@
/** An element in the query builder DSL, that will generate part of a CQL query. */
public interface CqlSnippet {
void appendTo(@NonNull StringBuilder builder);

/**
* Optional method used in collection types to append all elements to CQL query. List and set
* codecs typically enclose elements with '[]', '{}' characters. When we would like to append
* elements directly inside IN clause, mentioned behaviour generates incorrect CQL statement.
*/
default void appendElementsTo(@NonNull StringBuilder builder) {
appendTo(builder);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
package com.datastax.oss.driver.api.querybuilder.relation;

import com.datastax.oss.driver.api.querybuilder.BindMarker;
import com.datastax.oss.driver.api.querybuilder.QueryBuilder;
import com.datastax.oss.driver.api.querybuilder.term.Term;
import com.datastax.oss.driver.internal.querybuilder.term.UnwrappedCollectionTerm;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import java.util.Arrays;
Expand All @@ -41,7 +41,7 @@ default ResultT in(@NonNull BindMarker bindMarker) {
*/
@NonNull
default ResultT in(@NonNull Iterable<Term> alternatives) {
return build(" IN ", QueryBuilder.tuple(alternatives));
return build(" IN ", new UnwrappedCollectionTerm(alternatives));
}

/** Var-arg equivalent of {@link #in(Iterable)} . */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,29 @@ public static void append(
}
}

public static void appendElements(
@NonNull Iterable<? extends CqlSnippet> snippets,
@NonNull StringBuilder builder,
@Nullable String prefix,
@NonNull String separator,
@Nullable String suffix) {
boolean first = true;
for (CqlSnippet snippet : snippets) {
if (first) {
if (prefix != null) {
builder.append(prefix);
}
first = false;
} else {
builder.append(separator);
}
snippet.appendElementsTo(builder);
}
if (!first && suffix != null) {
builder.append(suffix);
}
}

public static void qualify(
@Nullable CqlIdentifier keyspace,
@NonNull CqlIdentifier element,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ public void appendTo(@NonNull StringBuilder builder) {
}
}

@Override
public void appendElementsTo(@NonNull StringBuilder builder) {
if (value != null) {
if (codec.getJavaType().isCollection()) {
builder.append(codec.formatElements(value));
} else {
builder.append(codec.format(value));
}
}
}

@Override
public boolean isIdempotent() {
return true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.datastax.oss.driver.internal.querybuilder.term;

import com.datastax.oss.driver.api.querybuilder.term.Term;
import com.datastax.oss.driver.internal.querybuilder.CqlHelper;
import edu.umd.cs.findbugs.annotations.NonNull;
import net.jcip.annotations.Immutable;

@Immutable
public class UnwrappedCollectionTerm implements Term {

private final Iterable<? extends Term> components;

public UnwrappedCollectionTerm(@NonNull Iterable<? extends Term> components) {
this.components = components;
}

@Override
public void appendTo(@NonNull StringBuilder builder) {
CqlHelper.appendElements(components, builder, "(", ",", ")");
}

@Override
public boolean isIdempotent() {
for (Term component : components) {
if (!component.isIdempotent()) {
return false;
}
}
return true;
}

@NonNull
public Iterable<? extends Term> getComponents() {
return components;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@

import static com.datastax.oss.driver.api.querybuilder.Assertions.assertThat;
import static com.datastax.oss.driver.api.querybuilder.QueryBuilder.bindMarker;
import static com.datastax.oss.driver.api.querybuilder.QueryBuilder.literal;
import static com.datastax.oss.driver.api.querybuilder.QueryBuilder.raw;
import static com.datastax.oss.driver.api.querybuilder.QueryBuilder.selectFrom;
import static com.datastax.oss.driver.api.querybuilder.QueryBuilder.tuple;

import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet;
import java.util.Arrays;
import java.util.List;
import java.util.UUID;
import org.junit.Test;

public class RelationTest {
Expand All @@ -47,6 +52,25 @@ public void should_generate_in_relation() {
.hasCql("SELECT * FROM foo WHERE k IN ?");
assertThat(selectFrom("foo").all().where(Relation.column("k").in(bindMarker(), bindMarker())))
.hasCql("SELECT * FROM foo WHERE k IN (?,?)");
assertThat(
selectFrom("foo")
.all()
.where(Relation.column("k").in(literal(Arrays.asList("vector", "data")))))
.hasCql("SELECT * FROM foo WHERE k IN ('vector','data')");
List<UUID> uuids =
Arrays.asList(
UUID.fromString("d3f5c945-74bd-4a99-b6d7-aa73e54a5b75"),
UUID.fromString("464a834d-8fd8-4842-9fba-47bc599b8083"));
assertThat(selectFrom("foo").all().where(Relation.column("k").in(literal(uuids))))
.hasCql(
"SELECT * FROM foo WHERE k IN (d3f5c945-74bd-4a99-b6d7-aa73e54a5b75,464a834d-8fd8-4842-9fba-47bc599b8083)");
assertThat(
selectFrom("foo")
.all()
.where(Relation.column("k").in(literal(ImmutableSet.of(1, 3, 5, 7)))))
.hasCql("SELECT * FROM foo WHERE k IN (1,3,5,7)");
assertThat(selectFrom("foo").all().where(Relation.column("k").in(literal("atomic value"))))
.hasCql("SELECT * FROM foo WHERE k IN ('atomic value')");
}

@Test
Expand Down