Skip to content

Commit d7dab59

Browse files
adutraadutra
authored
JAVA-2555: Generate append/prepend constructs compatible with legacy C* versions (apache#1379)
1 parent 31809f4 commit d7dab59

12 files changed

Lines changed: 255 additions & 97 deletions

File tree

‎changelog/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
### 4.4.0 (in progress)
66

7+
- [improvement] JAVA-2596: Consider collection removals as idempotent in query builder
8+
- [bug] JAVA-2555: Generate append/prepend constructs compatible with legacy C* versions
79
- [bug] JAVA-2584: Ensure codec registry is able to create codecs for collections of UDTs and tuples
810
- [bug] JAVA-2583: IS NOT NULL clause should be idempotent
911
- [improvement] JAVA-2442: Don't check for schema agreement twice when completing a DDL query

‎manual/query_builder/idempotence/README.md‎

Lines changed: 63 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,15 @@ If you use the result of a user-defined function in an INSERT or UPDATE statemen
3939
of knowing if that function is idempotent:
4040

4141
```java
42-
Statement statement = insertInto("foo").value("k", function("generate_id")).build();
42+
SimpleStatement statement = insertInto("foo").value("k", function("generate_id")).build();
4343
// INSERT INTO foo (k) VALUES (generate_id())
4444
assert !statement.isIdempotent();
4545
```
4646

4747
This extends to arithmetic operations using such terms:
4848

4949
```java
50-
Statement statement =
50+
SimpleStatement statement =
5151
insertInto("foo").value("k", add(function("generate_id"), literal(1))).build();
5252
// INSERT INTO foo (k) VALUES (generate_id()+1)
5353
assert !statement.isIdempotent();
@@ -56,7 +56,7 @@ assert !statement.isIdempotent();
5656
Raw terms could be anything, so they are also considered unsafe by default:
5757

5858
```java
59-
Statement statement =
59+
SimpleStatement statement =
6060
insertInto("foo").value("k", raw("generate_id()+1")).build();
6161
// INSERT INTO foo (k) VALUES (generate_id()+1)
6262
assert !statement.isIdempotent();
@@ -68,7 +68,7 @@ If a WHERE clause in an UPDATE or DELETE statement uses a comparison with an uns
6868
potentially apply to different rows for each execution:
6969

7070
```java
71-
Statement statement =
71+
SimpleStatement statement =
7272
update("foo")
7373
.setColumn("v", bindMarker())
7474
.whereColumn("k").isEqualTo(function("non_idempotent_func"))
@@ -82,7 +82,7 @@ assert !statement.isIdempotent();
8282
Counter updates are never idempotent:
8383

8484
```java
85-
Statement statement =
85+
SimpleStatement statement =
8686
update("foo")
8787
.increment("c")
8888
.whereColumn("k").isEqualTo(bindMarker())
@@ -94,20 +94,20 @@ assert !statement.isIdempotent();
9494
Nor is appending or prepending an element to a list:
9595

9696
```java
97-
Statement statement =
97+
SimpleStatement statement =
9898
update("foo")
9999
.appendListElement("l", literal(1))
100100
.whereColumn("k").isEqualTo(bindMarker())
101101
.build();
102-
// UPDATE foo SET l+=[1] WHERE k=?
102+
// UPDATE foo SET l=l+[1] WHERE k=?
103103
assert !statement.isIdempotent();
104104
```
105105

106106
The generic `append` and `prepend` methods apply to any kind of collection, so we have to consider
107107
them unsafe by default too:
108108

109109
```java
110-
Statement statement =
110+
SimpleStatement statement =
111111
update("foo")
112112
.prepend("l", literal(Arrays.asList(1, 2, 3)))
113113
.whereColumn("k").isEqualTo(bindMarker())
@@ -116,12 +116,66 @@ Statement statement =
116116
assert !statement.isIdempotent();
117117
```
118118

119+
The generic `remove` method is however safe since collection removals are idempotent:
120+
121+
```java
122+
SimpleStatement statement =
123+
update("foo")
124+
.remove("l", literal(Arrays.asList(1, 2, 3)))
125+
.whereColumn("k").isEqualTo(bindMarker())
126+
.build();
127+
// UPDATE foo SET l=l-[1,2,3] WHERE k=?
128+
assert statement.isIdempotent();
129+
```
130+
131+
When appending, prepending or removing a single element to/from a collection, it is possible to use
132+
the dedicated methods listed below; their idempotence depends on the collection type (list, set or
133+
map), the operation (append, prepend or removal) and the idempotence of the element being
134+
added/removed:
135+
136+
1. `appendListElement` : not idempotent
137+
2. `prependListElement` : not idempotent
138+
3. `removeListElement` : idempotent if element is idempotent
139+
4. `appendSetElement` : idempotent if element is idempotent
140+
5. `prependSetElement` : idempotent if element is idempotent
141+
6. `removeSetElement` : idempotent if element is idempotent
142+
7. `appendMapElement` : idempotent if both key and value are idempotent
143+
8. `prependMapElement` : idempotent if both key and value are idempotent
144+
9. `removeMapElement` : idempotent if both key and value are idempotent
145+
146+
In practice, most invocations of the above methods will be idempotent because most collection
147+
elements are. For example, the following statement is idempotent since `literal(1)` is also
148+
idempotent:
149+
150+
```java
151+
SimpleStatement statement =
152+
update("foo")
153+
.removeListElement("l", literal(1))
154+
.whereColumn("k").isEqualTo(bindMarker())
155+
.build();
156+
// UPDATE foo SET l=l-[1] WHERE k=?
157+
assert statement.isIdempotent();
158+
```
159+
160+
However, in rare cases the resulting statement won't be marked idempotent, e.g. if you use a
161+
function to select a collection element:
162+
163+
```java
164+
SimpleStatement statement =
165+
update("foo")
166+
.removeListElement("l", function("myfunc"))
167+
.whereColumn("k").isEqualTo(bindMarker())
168+
.build();
169+
// UPDATE foo SET l=l-[myfunc()] WHERE k=?
170+
assert !statement.isIdempotent();
171+
```
172+
119173
### Unsafe deletions
120174

121175
Deleting from a list is not idempotent:
122176

123177
```java
124-
Statement statement =
178+
SimpleStatement statement =
125179
deleteFrom("foo")
126180
.element("l", literal(0))
127181
.whereColumn("k").isEqualTo(bindMarker())

‎query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/update/Assignment.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.datastax.oss.driver.internal.querybuilder.update.PrependListElementAssignment;
3535
import com.datastax.oss.driver.internal.querybuilder.update.PrependMapEntryAssignment;
3636
import com.datastax.oss.driver.internal.querybuilder.update.PrependSetElementAssignment;
37+
import com.datastax.oss.driver.internal.querybuilder.update.RemoveAssignment;
3738
import com.datastax.oss.driver.internal.querybuilder.update.RemoveListElementAssignment;
3839
import com.datastax.oss.driver.internal.querybuilder.update.RemoveMapEntryAssignment;
3940
import com.datastax.oss.driver.internal.querybuilder.update.RemoveSetElementAssignment;
@@ -146,13 +147,13 @@ static Assignment decrement(@NonNull String columnName) {
146147
}
147148

148149
/**
149-
* Appends to a collection column, as in {@code SET l+=?}.
150+
* Appends to a collection column, as in {@code SET l=l+?}.
150151
*
151152
* <p>The term must be a collection of the same type as the column.
152153
*/
153154
@NonNull
154155
static Assignment append(@NonNull CqlIdentifier columnId, @NonNull Term suffix) {
155-
return new AppendAssignment(new ColumnLeftOperand(columnId), suffix);
156+
return new AppendAssignment(columnId, suffix);
156157
}
157158

158159
/**
@@ -165,7 +166,7 @@ static Assignment append(@NonNull String columnName, @NonNull Term suffix) {
165166
}
166167

167168
/**
168-
* Appends a single element to a list column, as in {@code SET l+=[?]}.
169+
* Appends a single element to a list column, as in {@code SET l=l+[?]}.
169170
*
170171
* <p>The term must be of the same type as the column's elements.
171172
*/
@@ -184,7 +185,7 @@ static Assignment appendListElement(@NonNull String columnName, @NonNull Term su
184185
}
185186

186187
/**
187-
* Appends a single element to a set column, as in {@code SET s+={?}}.
188+
* Appends a single element to a set column, as in {@code SET s=s+{?}}.
188189
*
189190
* <p>The term must be of the same type as the column's elements.
190191
*/
@@ -203,7 +204,7 @@ static Assignment appendSetElement(@NonNull String columnName, @NonNull Term suf
203204
}
204205

205206
/**
206-
* Appends a single entry to a map column, as in {@code SET m+={?:?}}.
207+
* Appends a single entry to a map column, as in {@code SET m=m+{?:?}}.
207208
*
208209
* <p>The terms must be of the same type as the column's keys and values respectively.
209210
*/
@@ -302,7 +303,7 @@ static Assignment prependMapEntry(
302303
}
303304

304305
/**
305-
* Removes elements from a collection, as in {@code SET l-=[1,2,3]}.
306+
* Removes elements from a collection, as in {@code SET l=l-[1,2,3]}.
306307
*
307308
* <p>The term must be a collection of the same type as the column.
308309
*
@@ -313,7 +314,7 @@ static Assignment prependMapEntry(
313314
*/
314315
@NonNull
315316
static Assignment remove(@NonNull CqlIdentifier columnId, @NonNull Term collectionToRemove) {
316-
return new DefaultAssignment(new ColumnLeftOperand(columnId), "-=", collectionToRemove);
317+
return new RemoveAssignment(columnId, collectionToRemove);
317318
}
318319

319320
/**
@@ -326,7 +327,7 @@ static Assignment remove(@NonNull String columnName, @NonNull Term collectionToR
326327
}
327328

328329
/**
329-
* Removes a single element to a list column, as in {@code SET l-=[?]}.
330+
* Removes a single element from a list column, as in {@code SET l=l-[?]}.
330331
*
331332
* <p>The term must be of the same type as the column's elements.
332333
*/
@@ -345,7 +346,7 @@ static Assignment removeListElement(@NonNull String columnName, @NonNull Term su
345346
}
346347

347348
/**
348-
* Removes a single element to a set column, as in {@code SET s-={?}}.
349+
* Removes a single element from a set column, as in {@code SET s=s-{?}}.
349350
*
350351
* <p>The term must be of the same type as the column's elements.
351352
*/
@@ -364,7 +365,7 @@ static Assignment removeSetElement(@NonNull String columnName, @NonNull Term suf
364365
}
365366

366367
/**
367-
* Removes a single entry to a map column, as in {@code SET m-={?:?}}.
368+
* Removes a single entry from a map column, as in {@code SET m=m-{?:?}}.
368369
*
369370
* <p>The terms must be of the same type as the column's keys and values respectively.
370371
*/

‎query-builder/src/main/java/com/datastax/oss/driver/api/querybuilder/update/OngoingAssignment.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ default UpdateWithAssignments decrement(@NonNull String columnName) {
221221
}
222222

223223
/**
224-
* Appends to a collection column, as in {@code SET l+=?}.
224+
* Appends to a collection column, as in {@code SET l=l+?}.
225225
*
226226
* <p>The term must be a collection of the same type as the column.
227227
*
@@ -246,7 +246,7 @@ default UpdateWithAssignments append(@NonNull String columnName, @NonNull Term s
246246
}
247247

248248
/**
249-
* Appends a single element to a list column, as in {@code SET l+=[?]}.
249+
* Appends a single element to a list column, as in {@code SET l=l+[?]}.
250250
*
251251
* <p>The term must be of the same type as the column's elements.
252252
*
@@ -274,7 +274,7 @@ default UpdateWithAssignments appendListElement(
274274
}
275275

276276
/**
277-
* Appends a single element to a set column, as in {@code SET s+={?}}.
277+
* Appends a single element to a set column, as in {@code SET s=s+{?}}.
278278
*
279279
* <p>The term must be of the same type as the column's elements.
280280
*
@@ -299,7 +299,7 @@ default UpdateWithAssignments appendSetElement(@NonNull String columnName, @NonN
299299
}
300300

301301
/**
302-
* Appends a single entry to a map column, as in {@code SET m+={?:?}}.
302+
* Appends a single entry to a map column, as in {@code SET m=m+{?:?}}.
303303
*
304304
* <p>The terms must be of the same type as the column's keys and values respectively.
305305
*
@@ -436,7 +436,7 @@ default UpdateWithAssignments prependMapEntry(
436436
}
437437

438438
/**
439-
* Removes elements from a collection, as in {@code SET l-=[1,2,3]}.
439+
* Removes elements from a collection, as in {@code SET l=l-[1,2,3]}.
440440
*
441441
* <p>The term must be a collection of the same type as the column.
442442
*
@@ -469,7 +469,7 @@ default UpdateWithAssignments remove(
469469
}
470470

471471
/**
472-
* Removes a single element to a list column, as in {@code SET l-=[?]}.
472+
* Removes a single element to a list column, as in {@code SET l=l-[?]}.
473473
*
474474
* <p>The term must be of the same type as the column's elements.
475475
*
@@ -497,7 +497,7 @@ default UpdateWithAssignments removeListElement(
497497
}
498498

499499
/**
500-
* Removes a single element to a set column, as in {@code SET s-={?}}.
500+
* Removes a single element to a set column, as in {@code SET s=s-{?}}.
501501
*
502502
* <p>The term must be of the same type as the column's elements.
503503
*
@@ -522,7 +522,7 @@ default UpdateWithAssignments removeSetElement(@NonNull String columnName, @NonN
522522
}
523523

524524
/**
525-
* Removes a single entry to a map column, as in {@code SET m-={?:?}}.
525+
* Removes a single entry to a map column, as in {@code SET m=m-{?:?}}.
526526
*
527527
* <p>The terms must be of the same type as the column's keys and values respectively.
528528
*

‎query-builder/src/main/java/com/datastax/oss/driver/internal/querybuilder/update/AppendAssignment.java‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,15 @@
1515
*/
1616
package com.datastax.oss.driver.internal.querybuilder.update;
1717

18+
import com.datastax.oss.driver.api.core.CqlIdentifier;
1819
import com.datastax.oss.driver.api.querybuilder.term.Term;
19-
import com.datastax.oss.driver.internal.querybuilder.lhs.LeftOperand;
2020
import edu.umd.cs.findbugs.annotations.NonNull;
2121
import net.jcip.annotations.Immutable;
2222

2323
@Immutable
24-
public class AppendAssignment extends DefaultAssignment {
24+
public class AppendAssignment extends CollectionAssignment {
2525

26-
public AppendAssignment(@NonNull LeftOperand leftOperand, @NonNull Term rightOperand) {
27-
super(leftOperand, "+=", rightOperand);
28-
}
29-
30-
@Override
31-
public boolean isIdempotent() {
32-
// Not idempotent for lists, be pessimistic
33-
return false;
26+
public AppendAssignment(@NonNull CqlIdentifier columnId, @NonNull Term value) {
27+
super(columnId, Operator.APPEND, value);
3428
}
3529
}

0 commit comments

Comments
 (0)