Skip to content

Commit e0038da

Browse files
committed
JAVA-1089: Set LWT made from BuiltStatements to non-idempotent.
(cherry picked from 2.1)
1 parent 802a78e commit e0038da

10 files changed

Lines changed: 183 additions & 56 deletions

File tree

changelog/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Merged from 2.1 branch:
1313
- [improvement] JAVA-879: Mapper.map() accepts mapper-generated and user queries.
1414
- [bug] JAVA-1100: Exception when connecting with shaded java driver in OSGI
1515
- [bug] JAVA-1064: getTable create statement doesn't properly handle quotes in primary key.
16+
- [bug] JAVA-1089: Set LWT made from BuiltStatements to non-idempotent.
1617

1718

1819
### 3.0.0

driver-core/src/main/java/com/datastax/driver/core/querybuilder/Batch.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public class Batch extends BuiltStatement {
3737
private int nonBuiltStatementValues;
3838

3939
Batch(RegularStatement[] statements, boolean logged) {
40-
super((String) null);
40+
super(null, null, null);
4141
this.statements = statements.length == 0
4242
? new ArrayList<RegularStatement>()
4343
: new ArrayList<RegularStatement>(statements.length);

driver-core/src/main/java/com/datastax/driver/core/querybuilder/BuiltStatement.java

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@
7777
*/
7878
public abstract class BuiltStatement extends RegularStatement {
7979

80-
private static final Pattern lowercaseId = Pattern.compile("[a-z][a-z0-9_]*");
80+
private static final Pattern lowercaseAlphanumeric = Pattern.compile("[a-z][a-z0-9_]*");
8181

8282
private final List<ColumnMetadata> partitionKey;
8383
private final List<Object> routingKeyValues;
@@ -94,22 +94,16 @@ public abstract class BuiltStatement extends RegularStatement {
9494
boolean hasBindMarkers;
9595
private boolean forceNoValues;
9696

97-
BuiltStatement(String keyspace) {
98-
this.partitionKey = null;
99-
this.routingKeyValues = null;
97+
BuiltStatement(String keyspace, List<ColumnMetadata> partitionKey, List<Object> routingKeyValues) {
98+
this.partitionKey = partitionKey;
99+
this.routingKeyValues = routingKeyValues;
100100
this.keyspace = keyspace;
101101
}
102102

103-
BuiltStatement(TableMetadata tableMetadata) {
104-
this.partitionKey = tableMetadata.getPartitionKey();
105-
this.routingKeyValues = Arrays.asList(new Object[tableMetadata.getPartitionKey().size()]);
106-
this.keyspace = escapeId(tableMetadata.getKeyspace().getName());
107-
}
108-
109103
// Same as Metadata.escapeId, but we don't have access to it here.
110104
protected static String escapeId(String ident) {
111105
// we don't need to escape if it's lowercase and match non-quoted CQL3 ids.
112-
return lowercaseId.matcher(ident).matches() ? ident : Metadata.quote(ident);
106+
return lowercaseAlphanumeric.matcher(ident).matches() ? ident : Metadata.quote(ident);
113107
}
114108

115109
@Override
@@ -352,7 +346,7 @@ abstract static class ForwardingStatement<T extends BuiltStatement> extends Buil
352346
T statement;
353347

354348
ForwardingStatement(T statement) {
355-
super((String) null);
349+
super(null, null, null);
356350
this.statement = statement;
357351
}
358352

driver-core/src/main/java/com/datastax/driver/core/querybuilder/Delete.java

Lines changed: 73 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@
1616
package com.datastax.driver.core.querybuilder;
1717

1818
import com.datastax.driver.core.CodecRegistry;
19+
import com.datastax.driver.core.ColumnMetadata;
1920
import com.datastax.driver.core.TableMetadata;
2021

2122
import java.util.ArrayList;
23+
import java.util.Arrays;
2224
import java.util.List;
2325

2426
/**
@@ -34,21 +36,33 @@ public class Delete extends BuiltStatement {
3436
private boolean ifExists;
3537

3638
Delete(String keyspace, String table, List<Selector> columns) {
37-
super(keyspace);
38-
this.table = table;
39-
this.columns = columns;
40-
this.where = new Where(this);
41-
this.usings = new Options(this);
42-
this.conditions = new Conditions(this);
39+
this(keyspace, table, null, null, columns);
4340
}
4441

4542
Delete(TableMetadata table, List<Selector> columns) {
46-
super(table);
47-
this.table = escapeId(table.getName());
43+
this(escapeId(table.getKeyspace().getName()),
44+
escapeId(table.getName()),
45+
Arrays.asList(new Object[table.getPartitionKey().size()]),
46+
table.getPartitionKey(),
47+
columns);
48+
}
49+
50+
Delete(String keyspace,
51+
String table,
52+
List<Object> routingKeyValues,
53+
List<ColumnMetadata> partitionKey,
54+
List<Selector> columns) {
55+
super(keyspace, partitionKey, routingKeyValues);
56+
this.table = table;
4857
this.columns = columns;
4958
this.where = new Where(this);
5059
this.usings = new Options(this);
5160
this.conditions = new Conditions(this);
61+
62+
// This is for JAVA-1089, if the query deletes an element in a list, the statement should be non-idempotent.
63+
if (!areIdempotent(columns)) {
64+
setNonIdempotentOps();
65+
}
5266
}
5367

5468
@Override
@@ -110,6 +124,9 @@ public Where where() {
110124
* Adds a conditions clause (IF) to this statement.
111125
* <p/>
112126
* This is a shorter/more readable version for {@code onlyIf().and(condition)}.
127+
* <p/>
128+
* This will configure the statement as non-idempotent, see {@link com.datastax.driver.core.Statement#isIdempotent()}
129+
* for more information.
113130
*
114131
* @param condition the condition to add.
115132
* @return the conditions of this query to which more conditions can be added.
@@ -120,6 +137,9 @@ public Conditions onlyIf(Clause condition) {
120137

121138
/**
122139
* Adds a conditions clause (IF) to this statement.
140+
* <p/>
141+
* This will configure the statement as non-idempotent, see {@link com.datastax.driver.core.Statement#isIdempotent()}
142+
* for more information.
123143
*
124144
* @return the conditions of this query to which more conditions can be added.
125145
*/
@@ -161,11 +181,14 @@ public Options using() {
161181
* Please keep in mind that using this option has a non negligible
162182
* performance impact and should be avoided when possible.
163183
* </p>
184+
* This will configure the statement as non-idempotent, see {@link com.datastax.driver.core.Statement#isIdempotent()}
185+
* for more information.
164186
*
165187
* @return this DELETE statement.
166188
*/
167189
public Delete ifExists() {
168190
this.ifExists = true;
191+
setNonIdempotentOps();
169192
return this;
170193
}
171194

@@ -189,6 +212,9 @@ public static class Where extends BuiltStatement.ForwardingStatement<Delete> {
189212
public Where and(Clause clause) {
190213
clauses.add(clause);
191214
statement.maybeAddRoutingKey(clause.name(), clause.firstValue());
215+
if (!hasNonIdempotentOps() && !Utils.isIdempotent(clause)) {
216+
statement.setNonIdempotentOps();
217+
}
192218
checkForBindMarkers(clause);
193219
return this;
194220
}
@@ -354,7 +380,7 @@ public Selection column(String columnName) {
354380
* @return this in-build DELETE Selection
355381
*/
356382
public Selection listElt(String columnName, int idx) {
357-
columns.add(new CollectionElementSelector(columnName, idx));
383+
columns.add(new ListElementSelector(columnName, idx));
358384
return this;
359385
}
360386

@@ -367,7 +393,7 @@ public Selection listElt(String columnName, int idx) {
367393
* @return this in-build DELETE Selection
368394
*/
369395
public Selection listElt(String columnName, BindMarker idx) {
370-
columns.add(new CollectionElementSelector(columnName, idx));
396+
columns.add(new ListElementSelector(columnName, idx));
371397
return this;
372398
}
373399

@@ -379,7 +405,7 @@ public Selection listElt(String columnName, BindMarker idx) {
379405
* @return this in-build DELETE Selection
380406
*/
381407
public Selection setElt(String columnName, Object element) {
382-
columns.add(new CollectionElementSelector(columnName, element));
408+
columns.add(new SetElementSelector(columnName, element));
383409
return this;
384410
}
385411

@@ -392,7 +418,7 @@ public Selection setElt(String columnName, Object element) {
392418
* @return this in-build DELETE Selection
393419
*/
394420
public Selection setElt(String columnName, BindMarker element) {
395-
columns.add(new CollectionElementSelector(columnName, element));
421+
columns.add(new SetElementSelector(columnName, element));
396422
return this;
397423
}
398424

@@ -404,7 +430,7 @@ public Selection setElt(String columnName, BindMarker element) {
404430
* @return this in-build DELETE Selection
405431
*/
406432
public Selection mapElt(String columnName, Object key) {
407-
columns.add(new CollectionElementSelector(columnName, key));
433+
columns.add(new MapElementSelector(columnName, key));
408434
return this;
409435
}
410436
}
@@ -414,7 +440,7 @@ public Selection mapElt(String columnName, Object key) {
414440
* A selector can be either a column name,
415441
* a list element, a set element or a map entry.
416442
*/
417-
static class Selector extends Utils.Appendeable {
443+
private static class Selector extends Utils.Appendeable {
418444

419445
private final String columnName;
420446

@@ -441,9 +467,9 @@ public String toString() {
441467
/**
442468
* A selector representing a list index, a set element or a map key in a DELETE selection clause.
443469
*/
444-
static class CollectionElementSelector extends Selector {
470+
private static class CollectionElementSelector extends Selector {
445471

446-
private final Object key;
472+
protected final Object key;
447473

448474
CollectionElementSelector(String columnName, Object key) {
449475
super(columnName);
@@ -465,6 +491,36 @@ boolean containsBindMarker() {
465491

466492
}
467493

494+
private static class ListElementSelector extends CollectionElementSelector {
495+
496+
ListElementSelector(String columnName, Object key) {
497+
super(columnName, key);
498+
}
499+
}
500+
501+
private static class SetElementSelector extends CollectionElementSelector {
502+
503+
SetElementSelector(String columnName, Object key) {
504+
super(columnName, key);
505+
}
506+
}
507+
508+
private static class MapElementSelector extends CollectionElementSelector {
509+
510+
MapElementSelector(String columnName, Object key) {
511+
super(columnName, key);
512+
}
513+
}
514+
515+
private boolean areIdempotent(List<Selector> selectors) {
516+
for (Selector sel : selectors) {
517+
if (sel instanceof ListElementSelector) {
518+
return false;
519+
}
520+
}
521+
return true;
522+
}
523+
468524
/**
469525
* Conditions for a DELETE statement.
470526
* <p>
@@ -494,6 +550,7 @@ public static class Conditions extends BuiltStatement.ForwardingStatement<Delete
494550
* @return this {@code Conditions} clause.
495551
*/
496552
public Conditions and(Clause condition) {
553+
statement.setNonIdempotentOps();
497554
conditions.add(condition);
498555
checkForBindMarkers(condition);
499556
return this;

driver-core/src/main/java/com/datastax/driver/core/querybuilder/Insert.java

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.datastax.driver.core.querybuilder;
1717

1818
import com.datastax.driver.core.CodecRegistry;
19+
import com.datastax.driver.core.ColumnMetadata;
1920
import com.datastax.driver.core.TableMetadata;
2021

2122
import java.util.ArrayList;
@@ -34,14 +35,22 @@ public class Insert extends BuiltStatement {
3435
private boolean ifNotExists;
3536

3637
Insert(String keyspace, String table) {
37-
super(keyspace);
38-
this.table = table;
39-
this.usings = new Options(this);
38+
this(keyspace, table, null, null);
4039
}
4140

4241
Insert(TableMetadata table) {
43-
super(table);
44-
this.table = escapeId(table.getName());
42+
this(escapeId(table.getKeyspace().getName()),
43+
escapeId(table.getName()),
44+
Arrays.asList(new Object[table.getPartitionKey().size()]),
45+
table.getPartitionKey());
46+
}
47+
48+
Insert(String keyspace,
49+
String table,
50+
List<Object> routingKeyValues,
51+
List<ColumnMetadata> partitionKey) {
52+
super(keyspace, partitionKey, routingKeyValues);
53+
this.table = table;
4554
this.usings = new Options(this);
4655
}
4756

@@ -157,10 +166,14 @@ public Options using() {
157166
* <p/>
158167
* Please keep in mind that using this option has a non negligible
159168
* performance impact and should be avoided when possible.
169+
* <p/>
170+
* This will configure the statement as non-idempotent, see {@link com.datastax.driver.core.Statement#isIdempotent()}
171+
* for more information.
160172
*
161173
* @return this INSERT statement.
162174
*/
163175
public Insert ifNotExists() {
176+
this.setNonIdempotentOps();
164177
this.ifNotExists = true;
165178
return this;
166179
}

driver-core/src/main/java/com/datastax/driver/core/querybuilder/Select.java

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.datastax.driver.core.querybuilder;
1717

1818
import com.datastax.driver.core.CodecRegistry;
19+
import com.datastax.driver.core.ColumnMetadata;
1920
import com.datastax.driver.core.TableMetadata;
2021

2122
import java.util.ArrayList;
@@ -39,16 +40,26 @@ public class Select extends BuiltStatement {
3940
private boolean allowFiltering;
4041

4142
Select(String keyspace, String table, List<Object> columnNames, boolean isDistinct) {
42-
super(keyspace);
43-
this.table = table;
44-
this.isDistinct = isDistinct;
45-
this.columnNames = columnNames;
46-
this.where = new Where(this);
43+
this(keyspace, table, null, null, columnNames, isDistinct);
4744
}
4845

4946
Select(TableMetadata table, List<Object> columnNames, boolean isDistinct) {
50-
super(table);
51-
this.table = escapeId(table.getName());
47+
this(escapeId(table.getKeyspace().getName()),
48+
escapeId(table.getName()),
49+
Arrays.asList(new Object[table.getPartitionKey().size()]),
50+
table.getPartitionKey(),
51+
columnNames,
52+
isDistinct);
53+
}
54+
55+
Select(String keyspace,
56+
String table,
57+
List<Object> routingKeyValues,
58+
List<ColumnMetadata> partitionKey,
59+
List<Object> columnNames,
60+
boolean isDistinct) {
61+
super(keyspace, partitionKey, routingKeyValues);
62+
this.table = table;
5263
this.isDistinct = isDistinct;
5364
this.columnNames = columnNames;
5465
this.where = new Where(this);

0 commit comments

Comments
 (0)