diff --git a/changelog/README.md b/changelog/README.md index 1b5e2c66257..008d8f245d2 100644 --- a/changelog/README.md +++ b/changelog/README.md @@ -4,6 +4,8 @@ ### 4.4.0 (in progress) +- [improvement] JAVA-2596: Consider collection removals as idempotent in query builder +- [bug] JAVA-2555: Generate append/prepend constructs compatible with legacy C* versions - [bug] JAVA-2584: Ensure codec registry is able to create codecs for collections of UDTs and tuples - [bug] JAVA-2583: IS NOT NULL clause should be idempotent - [improvement] JAVA-2442: Don't check for schema agreement twice when completing a DDL query diff --git a/manual/query_builder/idempotence/README.md b/manual/query_builder/idempotence/README.md index b064c81cdd6..9fd6d39114d 100644 --- a/manual/query_builder/idempotence/README.md +++ b/manual/query_builder/idempotence/README.md @@ -39,7 +39,7 @@ If you use the result of a user-defined function in an INSERT or UPDATE statemen of knowing if that function is idempotent: ```java -Statement statement = insertInto("foo").value("k", function("generate_id")).build(); +SimpleStatement statement = insertInto("foo").value("k", function("generate_id")).build(); // INSERT INTO foo (k) VALUES (generate_id()) assert !statement.isIdempotent(); ``` @@ -47,7 +47,7 @@ assert !statement.isIdempotent(); This extends to arithmetic operations using such terms: ```java -Statement statement = +SimpleStatement statement = insertInto("foo").value("k", add(function("generate_id"), literal(1))).build(); // INSERT INTO foo (k) VALUES (generate_id()+1) assert !statement.isIdempotent(); @@ -56,7 +56,7 @@ assert !statement.isIdempotent(); Raw terms could be anything, so they are also considered unsafe by default: ```java -Statement statement = +SimpleStatement statement = insertInto("foo").value("k", raw("generate_id()+1")).build(); // INSERT INTO foo (k) VALUES (generate_id()+1) assert !statement.isIdempotent(); @@ -68,7 +68,7 @@ If a WHERE clause in an UPDATE or DELETE statement uses a comparison with an uns potentially apply to different rows for each execution: ```java -Statement statement = +SimpleStatement statement = update("foo") .setColumn("v", bindMarker()) .whereColumn("k").isEqualTo(function("non_idempotent_func")) @@ -82,7 +82,7 @@ assert !statement.isIdempotent(); Counter updates are never idempotent: ```java -Statement statement = +SimpleStatement statement = update("foo") .increment("c") .whereColumn("k").isEqualTo(bindMarker()) @@ -94,12 +94,12 @@ assert !statement.isIdempotent(); Nor is appending or prepending an element to a list: ```java -Statement statement = +SimpleStatement statement = update("foo") .appendListElement("l", literal(1)) .whereColumn("k").isEqualTo(bindMarker()) .build(); -// UPDATE foo SET l+=[1] WHERE k=? +// UPDATE foo SET l=l+[1] WHERE k=? assert !statement.isIdempotent(); ``` @@ -107,7 +107,7 @@ The generic `append` and `prepend` methods apply to any kind of collection, so w them unsafe by default too: ```java -Statement statement = +SimpleStatement statement = update("foo") .prepend("l", literal(Arrays.asList(1, 2, 3))) .whereColumn("k").isEqualTo(bindMarker()) @@ -116,12 +116,66 @@ Statement statement = assert !statement.isIdempotent(); ``` +The generic `remove` method is however safe since collection removals are idempotent: + +```java +SimpleStatement statement = + update("foo") + .remove("l", literal(Arrays.asList(1, 2, 3))) + .whereColumn("k").isEqualTo(bindMarker()) + .build(); +// UPDATE foo SET l=l-[1,2,3] WHERE k=? +assert statement.isIdempotent(); +``` + +When appending, prepending or removing a single element to/from a collection, it is possible to use +the dedicated methods listed below; their idempotence depends on the collection type (list, set or +map), the operation (append, prepend or removal) and the idempotence of the element being +added/removed: + +1. `appendListElement` : not idempotent +2. `prependListElement` : not idempotent +3. `removeListElement` : idempotent if element is idempotent +4. `appendSetElement` : idempotent if element is idempotent +5. `prependSetElement` : idempotent if element is idempotent +6. `removeSetElement` : idempotent if element is idempotent +7. `appendMapElement` : idempotent if both key and value are idempotent +8. `prependMapElement` : idempotent if both key and value are idempotent +9. `removeMapElement` : idempotent if both key and value are idempotent + +In practice, most invocations of the above methods will be idempotent because most collection +elements are. For example, the following statement is idempotent since `literal(1)` is also +idempotent: + +```java +SimpleStatement statement = + update("foo") + .removeListElement("l", literal(1)) + .whereColumn("k").isEqualTo(bindMarker()) + .build(); +// UPDATE foo SET l=l-[1] WHERE k=? +assert statement.isIdempotent(); +``` + +However, in rare cases the resulting statement won't be marked idempotent, e.g. if you use a +function to select a collection element: + +```java +SimpleStatement statement = + update("foo") + .removeListElement("l", function("myfunc")) + .whereColumn("k").isEqualTo(bindMarker()) + .build(); +// UPDATE foo SET l=l-[myfunc()] WHERE k=? +assert !statement.isIdempotent(); +``` + ### Unsafe deletions Deleting from a list is not idempotent: ```java -Statement statement = +SimpleStatement statement = deleteFrom("foo") .element("l", literal(0)) .whereColumn("k").isEqualTo(bindMarker()) diff --git a/query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/update/Assignment.java b/query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/update/Assignment.java index d918c8aba42..e1e81e74c18 100644 --- a/query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/update/Assignment.java +++ b/query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/update/Assignment.java @@ -34,6 +34,7 @@ import com.datastax.oss.driver.internal.querybuilder.update.PrependListElementAssignment; import com.datastax.oss.driver.internal.querybuilder.update.PrependMapEntryAssignment; import com.datastax.oss.driver.internal.querybuilder.update.PrependSetElementAssignment; +import com.datastax.oss.driver.internal.querybuilder.update.RemoveAssignment; import com.datastax.oss.driver.internal.querybuilder.update.RemoveListElementAssignment; import com.datastax.oss.driver.internal.querybuilder.update.RemoveMapEntryAssignment; import com.datastax.oss.driver.internal.querybuilder.update.RemoveSetElementAssignment; @@ -146,13 +147,13 @@ static Assignment decrement(@NonNull String columnName) { } /** - * Appends to a collection column, as in {@code SET l+=?}. + * Appends to a collection column, as in {@code SET l=l+?}. * *

The term must be a collection of the same type as the column. */ @NonNull static Assignment append(@NonNull CqlIdentifier columnId, @NonNull Term suffix) { - return new AppendAssignment(new ColumnLeftOperand(columnId), suffix); + return new AppendAssignment(columnId, suffix); } /** @@ -165,7 +166,7 @@ static Assignment append(@NonNull String columnName, @NonNull Term suffix) { } /** - * Appends a single element to a list column, as in {@code SET l+=[?]}. + * Appends a single element to a list column, as in {@code SET l=l+[?]}. * *

The term must be of the same type as the column's elements. */ @@ -184,7 +185,7 @@ static Assignment appendListElement(@NonNull String columnName, @NonNull Term su } /** - * Appends a single element to a set column, as in {@code SET s+={?}}. + * Appends a single element to a set column, as in {@code SET s=s+{?}}. * *

The term must be of the same type as the column's elements. */ @@ -203,7 +204,7 @@ static Assignment appendSetElement(@NonNull String columnName, @NonNull Term suf } /** - * Appends a single entry to a map column, as in {@code SET m+={?:?}}. + * Appends a single entry to a map column, as in {@code SET m=m+{?:?}}. * *

The terms must be of the same type as the column's keys and values respectively. */ @@ -302,7 +303,7 @@ static Assignment prependMapEntry( } /** - * Removes elements from a collection, as in {@code SET l-=[1,2,3]}. + * Removes elements from a collection, as in {@code SET l=l-[1,2,3]}. * *

The term must be a collection of the same type as the column. * @@ -313,7 +314,7 @@ static Assignment prependMapEntry( */ @NonNull static Assignment remove(@NonNull CqlIdentifier columnId, @NonNull Term collectionToRemove) { - return new DefaultAssignment(new ColumnLeftOperand(columnId), "-=", collectionToRemove); + return new RemoveAssignment(columnId, collectionToRemove); } /** @@ -326,7 +327,7 @@ static Assignment remove(@NonNull String columnName, @NonNull Term collectionToR } /** - * Removes a single element to a list column, as in {@code SET l-=[?]}. + * Removes a single element from a list column, as in {@code SET l=l-[?]}. * *

The term must be of the same type as the column's elements. */ @@ -345,7 +346,7 @@ static Assignment removeListElement(@NonNull String columnName, @NonNull Term su } /** - * Removes a single element to a set column, as in {@code SET s-={?}}. + * Removes a single element from a set column, as in {@code SET s=s-{?}}. * *

The term must be of the same type as the column's elements. */ @@ -364,7 +365,7 @@ static Assignment removeSetElement(@NonNull String columnName, @NonNull Term suf } /** - * Removes a single entry to a map column, as in {@code SET m-={?:?}}. + * Removes a single entry from a map column, as in {@code SET m=m-{?:?}}. * *

The terms must be of the same type as the column's keys and values respectively. */ diff --git a/query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/update/OngoingAssignment.java b/query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/update/OngoingAssignment.java index 67af1f09e34..093e2047274 100644 --- a/query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/update/OngoingAssignment.java +++ b/query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/update/OngoingAssignment.java @@ -221,7 +221,7 @@ default UpdateWithAssignments decrement(@NonNull String columnName) { } /** - * Appends to a collection column, as in {@code SET l+=?}. + * Appends to a collection column, as in {@code SET l=l+?}. * *

The term must be a collection of the same type as the column. * @@ -246,7 +246,7 @@ default UpdateWithAssignments append(@NonNull String columnName, @NonNull Term s } /** - * Appends a single element to a list column, as in {@code SET l+=[?]}. + * Appends a single element to a list column, as in {@code SET l=l+[?]}. * *

The term must be of the same type as the column's elements. * @@ -274,7 +274,7 @@ default UpdateWithAssignments appendListElement( } /** - * Appends a single element to a set column, as in {@code SET s+={?}}. + * Appends a single element to a set column, as in {@code SET s=s+{?}}. * *

The term must be of the same type as the column's elements. * @@ -299,7 +299,7 @@ default UpdateWithAssignments appendSetElement(@NonNull String columnName, @NonN } /** - * Appends a single entry to a map column, as in {@code SET m+={?:?}}. + * Appends a single entry to a map column, as in {@code SET m=m+{?:?}}. * *

The terms must be of the same type as the column's keys and values respectively. * @@ -436,7 +436,7 @@ default UpdateWithAssignments prependMapEntry( } /** - * Removes elements from a collection, as in {@code SET l-=[1,2,3]}. + * Removes elements from a collection, as in {@code SET l=l-[1,2,3]}. * *

The term must be a collection of the same type as the column. * @@ -469,7 +469,7 @@ default UpdateWithAssignments remove( } /** - * Removes a single element to a list column, as in {@code SET l-=[?]}. + * Removes a single element to a list column, as in {@code SET l=l-[?]}. * *

The term must be of the same type as the column's elements. * @@ -497,7 +497,7 @@ default UpdateWithAssignments removeListElement( } /** - * Removes a single element to a set column, as in {@code SET s-={?}}. + * Removes a single element to a set column, as in {@code SET s=s-{?}}. * *

The term must be of the same type as the column's elements. * @@ -522,7 +522,7 @@ default UpdateWithAssignments removeSetElement(@NonNull String columnName, @NonN } /** - * Removes a single entry to a map column, as in {@code SET m-={?:?}}. + * Removes a single entry to a map column, as in {@code SET m=m-{?:?}}. * *

The terms must be of the same type as the column's keys and values respectively. * diff --git a/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/AppendAssignment.java b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/AppendAssignment.java index 271c0bcca16..3213a097928 100644 --- a/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/AppendAssignment.java +++ b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/AppendAssignment.java @@ -15,21 +15,15 @@ */ package com.datastax.oss.driver.internal.querybuilder.update; +import com.datastax.oss.driver.api.core.CqlIdentifier; import com.datastax.oss.driver.api.querybuilder.term.Term; -import com.datastax.oss.driver.internal.querybuilder.lhs.LeftOperand; import edu.umd.cs.findbugs.annotations.NonNull; import net.jcip.annotations.Immutable; @Immutable -public class AppendAssignment extends DefaultAssignment { +public class AppendAssignment extends CollectionAssignment { - public AppendAssignment(@NonNull LeftOperand leftOperand, @NonNull Term rightOperand) { - super(leftOperand, "+=", rightOperand); - } - - @Override - public boolean isIdempotent() { - // Not idempotent for lists, be pessimistic - return false; + public AppendAssignment(@NonNull CqlIdentifier columnId, @NonNull Term value) { + super(columnId, Operator.APPEND, value); } } diff --git a/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/CollectionAssignment.java b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/CollectionAssignment.java new file mode 100644 index 00000000000..e89a5028055 --- /dev/null +++ b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/CollectionAssignment.java @@ -0,0 +1,81 @@ +/* + * Copyright DataStax, Inc. + * + * Licensed 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.update; + +import com.datastax.oss.driver.api.core.CqlIdentifier; +import com.datastax.oss.driver.api.querybuilder.term.Term; +import com.datastax.oss.driver.api.querybuilder.update.Assignment; +import com.datastax.oss.driver.shaded.guava.common.base.Preconditions; +import edu.umd.cs.findbugs.annotations.NonNull; +import net.jcip.annotations.Immutable; + +@Immutable +public abstract class CollectionAssignment implements Assignment { + + public enum Operator { + APPEND("%1$s=%1$s+%2$s"), + PREPEND("%1$s=%2$s+%1$s"), + REMOVE("%1$s=%1$s-%2$s"), + ; + + public final String pattern; + + Operator(String pattern) { + this.pattern = pattern; + } + } + + private final CqlIdentifier columnId; + private final Operator operator; + private final Term value; + + protected CollectionAssignment( + @NonNull CqlIdentifier columnId, @NonNull Operator operator, @NonNull Term value) { + Preconditions.checkNotNull(columnId); + Preconditions.checkNotNull(value); + this.columnId = columnId; + this.operator = operator; + this.value = value; + } + + @Override + public void appendTo(@NonNull StringBuilder builder) { + builder.append(String.format(operator.pattern, columnId.asCql(true), buildRightOperand())); + } + + private String buildRightOperand() { + StringBuilder builder = new StringBuilder(); + value.appendTo(builder); + return builder.toString(); + } + + @Override + public boolean isIdempotent() { + // REMOVE is idempotent if the collection being removed is idempotent; APPEND and PREPEND are + // not idempotent for lists, so be pessimistic + return operator == Operator.REMOVE && value.isIdempotent(); + } + + @NonNull + public CqlIdentifier getColumnId() { + return columnId; + } + + @NonNull + public Term getValue() { + return value; + } +} diff --git a/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/CollectionElementAssignment.java b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/CollectionElementAssignment.java index 35da1ca0f6e..4d63e49c0f0 100644 --- a/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/CollectionElementAssignment.java +++ b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/CollectionElementAssignment.java @@ -27,9 +27,9 @@ public abstract class CollectionElementAssignment implements Assignment { public enum Operator { - APPEND("%s+=%s"), + APPEND("%1$s=%1$s+%2$s"), PREPEND("%1$s=%2$s+%1$s"), - REMOVE("%s-=%s"), + REMOVE("%1$s=%1$s-%2$s"), ; public final String pattern; diff --git a/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/PrependAssignment.java b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/PrependAssignment.java index d58a6ffa18e..1b461bbb2c6 100644 --- a/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/PrependAssignment.java +++ b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/PrependAssignment.java @@ -17,42 +17,13 @@ import com.datastax.oss.driver.api.core.CqlIdentifier; import com.datastax.oss.driver.api.querybuilder.term.Term; -import com.datastax.oss.driver.api.querybuilder.update.Assignment; import edu.umd.cs.findbugs.annotations.NonNull; import net.jcip.annotations.Immutable; @Immutable -public class PrependAssignment implements Assignment { - - private final CqlIdentifier columnId; - private final Term prefix; +public class PrependAssignment extends CollectionAssignment { public PrependAssignment(@NonNull CqlIdentifier columnId, @NonNull Term prefix) { - this.columnId = columnId; - this.prefix = prefix; - } - - @Override - public void appendTo(@NonNull StringBuilder builder) { - String column = columnId.asCql(true); - builder.append(column).append('='); - prefix.appendTo(builder); - builder.append('+').append(column); - } - - @Override - public boolean isIdempotent() { - // Not idempotent for lists, be pessimistic - return false; - } - - @NonNull - public CqlIdentifier getColumnId() { - return columnId; - } - - @NonNull - public Term getPrefix() { - return prefix; + super(columnId, Operator.PREPEND, prefix); } } diff --git a/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/RemoveAssignment.java b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/RemoveAssignment.java new file mode 100644 index 00000000000..453b8a318f6 --- /dev/null +++ b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/RemoveAssignment.java @@ -0,0 +1,29 @@ +/* + * Copyright DataStax, Inc. + * + * Licensed 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.update; + +import com.datastax.oss.driver.api.core.CqlIdentifier; +import com.datastax.oss.driver.api.querybuilder.term.Term; +import edu.umd.cs.findbugs.annotations.NonNull; +import net.jcip.annotations.Immutable; + +@Immutable +public class RemoveAssignment extends CollectionAssignment { + + public RemoveAssignment(@NonNull CqlIdentifier columnId, @NonNull Term value) { + super(columnId, Operator.REMOVE, value); + } +} diff --git a/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/RemoveListElementAssignment.java b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/RemoveListElementAssignment.java index 985a871fe5e..080104965c6 100644 --- a/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/RemoveListElementAssignment.java +++ b/query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/RemoveListElementAssignment.java @@ -26,9 +26,4 @@ public class RemoveListElementAssignment extends CollectionElementAssignment { public RemoveListElementAssignment(@NonNull CqlIdentifier columnId, @NonNull Term element) { super(columnId, Operator.REMOVE, null, element, '[', ']'); } - - @Override - public boolean isIdempotent() { - return false; - } } diff --git a/query-builder/src/test/java/com/datastax/oss/driver/api/querybuilder/update/UpdateFluentAssignmentTest.java b/query-builder/src/test/java/com/datastax/oss/driver/api/querybuilder/update/UpdateFluentAssignmentTest.java index 184ad2e2dbf..bf9e13ef2d5 100644 --- a/query-builder/src/test/java/com/datastax/oss/driver/api/querybuilder/update/UpdateFluentAssignmentTest.java +++ b/query-builder/src/test/java/com/datastax/oss/driver/api/querybuilder/update/UpdateFluentAssignmentTest.java @@ -82,15 +82,15 @@ public void should_generate_list_operations() { Literal listLiteral = literal(ImmutableList.of(1, 2, 3)); assertThat(update("foo").append("l", bindMarker()).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET l+=? WHERE k=?"); + .hasCql("UPDATE foo SET l=l+? WHERE k=?"); assertThat(update("foo").append("l", listLiteral).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET l+=[1,2,3] WHERE k=?"); + .hasCql("UPDATE foo SET l=l+[1,2,3] WHERE k=?"); assertThat( update("foo") .appendListElement("l", bindMarker()) .whereColumn("k") .isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET l+=[?] WHERE k=?"); + .hasCql("UPDATE foo SET l=l+[?] WHERE k=?"); assertThat(update("foo").prepend("l", bindMarker()).whereColumn("k").isEqualTo(bindMarker())) .hasCql("UPDATE foo SET l=?+l WHERE k=?"); @@ -104,15 +104,15 @@ public void should_generate_list_operations() { .hasCql("UPDATE foo SET l=[?]+l WHERE k=?"); assertThat(update("foo").remove("l", bindMarker()).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET l-=? WHERE k=?"); + .hasCql("UPDATE foo SET l=l-? WHERE k=?"); assertThat(update("foo").remove("l", listLiteral).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET l-=[1,2,3] WHERE k=?"); + .hasCql("UPDATE foo SET l=l-[1,2,3] WHERE k=?"); assertThat( update("foo") .removeListElement("l", bindMarker()) .whereColumn("k") .isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET l-=[?] WHERE k=?"); + .hasCql("UPDATE foo SET l=l-[?] WHERE k=?"); } @Test @@ -120,15 +120,15 @@ public void should_generate_set_operations() { Literal setLiteral = literal(ImmutableSet.of(1, 2, 3)); assertThat(update("foo").append("s", bindMarker()).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET s+=? WHERE k=?"); + .hasCql("UPDATE foo SET s=s+? WHERE k=?"); assertThat(update("foo").append("s", setLiteral).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET s+={1,2,3} WHERE k=?"); + .hasCql("UPDATE foo SET s=s+{1,2,3} WHERE k=?"); assertThat( update("foo") .appendSetElement("s", bindMarker()) .whereColumn("k") .isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET s+={?} WHERE k=?"); + .hasCql("UPDATE foo SET s=s+{?} WHERE k=?"); assertThat(update("foo").prepend("s", bindMarker()).whereColumn("k").isEqualTo(bindMarker())) .hasCql("UPDATE foo SET s=?+s WHERE k=?"); @@ -142,15 +142,15 @@ public void should_generate_set_operations() { .hasCql("UPDATE foo SET s={?}+s WHERE k=?"); assertThat(update("foo").remove("s", bindMarker()).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET s-=? WHERE k=?"); + .hasCql("UPDATE foo SET s=s-? WHERE k=?"); assertThat(update("foo").remove("s", setLiteral).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET s-={1,2,3} WHERE k=?"); + .hasCql("UPDATE foo SET s=s-{1,2,3} WHERE k=?"); assertThat( update("foo") .removeSetElement("s", bindMarker()) .whereColumn("k") .isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET s-={?} WHERE k=?"); + .hasCql("UPDATE foo SET s=s-{?} WHERE k=?"); } @Test @@ -158,15 +158,15 @@ public void should_generate_map_operations() { Literal mapLiteral = literal(ImmutableMap.of(1, "foo", 2, "bar")); assertThat(update("foo").append("m", bindMarker()).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET m+=? WHERE k=?"); + .hasCql("UPDATE foo SET m=m+? WHERE k=?"); assertThat(update("foo").append("m", mapLiteral).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET m+={1:'foo',2:'bar'} WHERE k=?"); + .hasCql("UPDATE foo SET m=m+{1:'foo',2:'bar'} WHERE k=?"); assertThat( update("foo") .appendMapEntry("m", literal(1), literal("foo")) .whereColumn("k") .isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET m+={1:'foo'} WHERE k=?"); + .hasCql("UPDATE foo SET m=m+{1:'foo'} WHERE k=?"); assertThat(update("foo").prepend("m", bindMarker()).whereColumn("k").isEqualTo(bindMarker())) .hasCql("UPDATE foo SET m=?+m WHERE k=?"); @@ -180,14 +180,14 @@ public void should_generate_map_operations() { .hasCql("UPDATE foo SET m={1:'foo'}+m WHERE k=?"); assertThat(update("foo").remove("m", bindMarker()).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET m-=? WHERE k=?"); + .hasCql("UPDATE foo SET m=m-? WHERE k=?"); assertThat(update("foo").remove("m", mapLiteral).whereColumn("k").isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET m-={1:'foo',2:'bar'} WHERE k=?"); + .hasCql("UPDATE foo SET m=m-{1:'foo',2:'bar'} WHERE k=?"); assertThat( update("foo") .removeMapEntry("m", literal(1), literal("foo")) .whereColumn("k") .isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET m-={1:'foo'} WHERE k=?"); + .hasCql("UPDATE foo SET m=m-{1:'foo'} WHERE k=?"); } } diff --git a/query-builder/src/test/java/com/datastax/oss/driver/api/querybuilder/update/UpdateIdempotenceTest.java b/query-builder/src/test/java/com/datastax/oss/driver/api/querybuilder/update/UpdateIdempotenceTest.java index 09f778d041a..1a3b05614ea 100644 --- a/query-builder/src/test/java/com/datastax/oss/driver/api/querybuilder/update/UpdateIdempotenceTest.java +++ b/query-builder/src/test/java/com/datastax/oss/driver/api/querybuilder/update/UpdateIdempotenceTest.java @@ -104,7 +104,7 @@ public void should_not_be_idempotent_if_adding_element_to_list() { .appendListElement("l", literal(1)) .whereColumn("k") .isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET l+=[1] WHERE k=?") + .hasCql("UPDATE foo SET l=l+[1] WHERE k=?") .isNotIdempotent(); assertThat( update("foo") @@ -120,14 +120,37 @@ public void should_not_be_idempotent_if_adding_element_to_list() { .appendSetElement("s", literal(1)) .whereColumn("k") .isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET s+={1} WHERE k=?") + .hasCql("UPDATE foo SET s=s+{1} WHERE k=?") .isIdempotent(); assertThat( update("foo") .appendMapEntry("m", literal(1), literal("bar")) .whereColumn("k") .isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET m+={1:'bar'} WHERE k=?") + .hasCql("UPDATE foo SET m=m+{1:'bar'} WHERE k=?") + .isIdempotent(); + + // Also, removals are always safe: + assertThat( + update("foo") + .removeListElement("l", literal(1)) + .whereColumn("k") + .isEqualTo(bindMarker())) + .hasCql("UPDATE foo SET l=l-[1] WHERE k=?") + .isIdempotent(); + assertThat( + update("foo") + .removeSetElement("s", literal(1)) + .whereColumn("k") + .isEqualTo(bindMarker())) + .hasCql("UPDATE foo SET s=s-{1} WHERE k=?") + .isIdempotent(); + assertThat( + update("foo") + .removeMapEntry("m", literal(1), literal("bar")) + .whereColumn("k") + .isEqualTo(bindMarker())) + .hasCql("UPDATE foo SET m=m-{1:'bar'} WHERE k=?") .isIdempotent(); } @@ -138,7 +161,7 @@ public void should_not_be_idempotent_if_concatenating_to_collection() { .append("l", literal(Arrays.asList(1, 2, 3))) .whereColumn("k") .isEqualTo(bindMarker())) - .hasCql("UPDATE foo SET l+=[1,2,3] WHERE k=?") + .hasCql("UPDATE foo SET l=l+[1,2,3] WHERE k=?") .isNotIdempotent(); assertThat( update("foo") @@ -147,6 +170,14 @@ public void should_not_be_idempotent_if_concatenating_to_collection() { .isEqualTo(bindMarker())) .hasCql("UPDATE foo SET l=[1,2,3]+l WHERE k=?") .isNotIdempotent(); + // However, removals are always safe: + assertThat( + update("foo") + .remove("l", literal(Arrays.asList(1, 2, 3))) + .whereColumn("k") + .isEqualTo(bindMarker())) + .hasCql("UPDATE foo SET l=l-[1,2,3] WHERE k=?") + .isIdempotent(); } @Test