diff --git a/CHANGES.txt b/CHANGES.txt index 6e3d95c704..58087fd554 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -8,6 +8,7 @@ Merged from 2.2: * Expose phi values from failure detector via JMX and tweak debug and trace logging (CASSANDRA-9526) Merged from 2.1: + * Fix conditions on static columns (CASSANDRA-10264) * AssertionError: attempted to delete non-existing file CommitLog (CASSANDRA-10377) diff --git a/pylib/cqlshlib/cql3handling.py b/pylib/cqlshlib/cql3handling.py index 1398e0def0..42e542ffa3 100644 --- a/pylib/cqlshlib/cql3handling.py +++ b/pylib/cqlshlib/cql3handling.py @@ -146,7 +146,7 @@ JUNK ::= /([ \t\r\f\v]+|(--|[/][/])[^\n\r]*([\n\r]|$)|[/][*].*?[*][/])/ ; ::= "*" ; ::= ";" ; ::= /[-+=,().]/ ; - ::= /[<>]=?/ ; + ::= /[<>!]=?/ ; ::= /[][{}]/ ; ::= "-"? ; @@ -894,7 +894,7 @@ syntax_rules += r''' ; ::= ( "AND" )* ; - ::= ( "[" "]" )? ( ( "=" | "<" | ">" | "<=" | ">=" | "!=" ) + ::= ( "[" "]" )? (("=" | "<" | ">" | "<=" | ">=" | "!=") | "IN" "(" ( "," )* ")") ; ''' diff --git a/src/java/org/apache/cassandra/cql3/Operations.java b/src/java/org/apache/cassandra/cql3/Operations.java index c4cade18e5..0ef85177bf 100644 --- a/src/java/org/apache/cassandra/cql3/Operations.java +++ b/src/java/org/apache/cassandra/cql3/Operations.java @@ -22,6 +22,7 @@ import java.util.Iterator; import java.util.List; import org.apache.cassandra.cql3.functions.Function; +import org.apache.cassandra.cql3.statements.StatementType; import com.google.common.collect.Iterables; import com.google.common.collect.Iterators; @@ -32,6 +33,11 @@ import com.google.common.collect.Iterators; */ public final class Operations implements Iterable { + /** + * The type of statement. + */ + private final StatementType type; + /** * The operations on regular columns. */ @@ -42,6 +48,11 @@ public final class Operations implements Iterable */ private final List staticOperations = new ArrayList<>(); + public Operations(StatementType type) + { + this.type = type; + } + /** * Checks if some of the operations apply to static columns. * @@ -59,7 +70,10 @@ public final class Operations implements Iterable */ public boolean appliesToRegularColumns() { - return !regularOperations.isEmpty(); + // If we have regular operations, this applies to regular columns. + // Otherwise, if the statement is a DELETE and staticOperations is also empty, this means we have no operations, + // which for a DELETE means a full row deletion. Which means the operation applies to all columns and regular ones in particular. + return !regularOperations.isEmpty() || (type.isDelete() && staticOperations.isEmpty()); } /** diff --git a/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java b/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java index d51f26119d..0efe35c47c 100644 --- a/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/DeleteStatement.java @@ -119,7 +119,7 @@ public class DeleteStatement extends ModificationStatement List> conditions, boolean ifExists) { - super(name, attrs, conditions, false, ifExists); + super(name, StatementType.DELETE, attrs, conditions, false, ifExists); this.deletions = deletions; this.whereClause = whereClause; } @@ -131,7 +131,7 @@ public class DeleteStatement extends ModificationStatement Conditions conditions, Attributes attrs) { - Operations operations = new Operations(); + Operations operations = new Operations(type); for (Operation.RawDeletion deletion : deletions) { @@ -146,8 +146,7 @@ public class DeleteStatement extends ModificationStatement operations.add(op); } - StatementRestrictions restrictions = newRestrictions(StatementType.DELETE, - cfm, + StatementRestrictions restrictions = newRestrictions(cfm, boundNames, operations, whereClause, diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java index a1290eeadb..b8814778fd 100644 --- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java @@ -736,14 +736,21 @@ public abstract class ModificationStatement implements CQLStatement public static abstract class Parsed extends CFStatement { + protected final StatementType type; private final Attributes.Raw attrs; private final List> conditions; private final boolean ifNotExists; private final boolean ifExists; - protected Parsed(CFName name, Attributes.Raw attrs, List> conditions, boolean ifNotExists, boolean ifExists) + protected Parsed(CFName name, + StatementType type, + Attributes.Raw attrs, + List> conditions, + boolean ifNotExists, + boolean ifExists) { super(name); + this.type = type; this.attrs = attrs; this.conditions = conditions == null ? Collections.>emptyList() : conditions; this.ifNotExists = ifNotExists; @@ -840,7 +847,6 @@ public abstract class ModificationStatement implements CQLStatement /** * Creates the restrictions. * - * @param type the statement type * @param cfm the column family meta data * @param boundNames the bound names * @param operations the column operations @@ -848,12 +854,11 @@ public abstract class ModificationStatement implements CQLStatement * @param conditions the conditions * @return the restrictions */ - protected static StatementRestrictions newRestrictions(StatementType type, - CFMetaData cfm, - VariableSpecifications boundNames, - Operations operations, - WhereClause where, - Conditions conditions) + protected StatementRestrictions newRestrictions(CFMetaData cfm, + VariableSpecifications boundNames, + Operations operations, + WhereClause where, + Conditions conditions) { if (where.containsCustomExpressions()) throw new InvalidRequestException(CUSTOM_EXPRESSIONS_NOT_ALLOWED); diff --git a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java index d6d0266190..6f872d4644 100644 --- a/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java @@ -131,7 +131,7 @@ public class UpdateStatement extends ModificationStatement List columnValues, boolean ifNotExists) { - super(name, attrs, null, ifNotExists, false); + super(name, StatementType.INSERT, attrs, null, ifNotExists, false); this.columnNames = columnNames; this.columnValues = columnValues; } @@ -152,7 +152,7 @@ public class UpdateStatement extends ModificationStatement checkContainsNoDuplicates(columnNames, "The column names contains duplicates"); WhereClause.Builder whereClause = new WhereClause.Builder(); - Operations operations = new Operations(); + Operations operations = new Operations(type); boolean hasClusteringColumnsSet = false; for (int i = 0; i < columnNames.size(); i++) @@ -178,7 +178,7 @@ public class UpdateStatement extends ModificationStatement boolean applyOnlyToStaticColumns = appliesOnlyToStaticColumns(operations, conditions) && !hasClusteringColumnsSet; - StatementRestrictions restrictions = new StatementRestrictions(StatementType.INSERT, + StatementRestrictions restrictions = new StatementRestrictions(type, cfm, whereClause.build(), boundNames, @@ -187,7 +187,7 @@ public class UpdateStatement extends ModificationStatement false, false); - return new UpdateStatement(StatementType.INSERT, + return new UpdateStatement(type, boundNames.size(), cfm, operations, @@ -206,7 +206,7 @@ public class UpdateStatement extends ModificationStatement public ParsedInsertJson(CFName name, Attributes.Raw attrs, Json.Raw jsonValue, boolean ifNotExists) { - super(name, attrs, null, ifNotExists, false); + super(name, StatementType.INSERT, attrs, null, ifNotExists, false); this.jsonValue = jsonValue; } @@ -222,7 +222,7 @@ public class UpdateStatement extends ModificationStatement Json.Prepared prepared = jsonValue.prepareAndCollectMarkers(cfm, defs, boundNames); WhereClause.Builder whereClause = new WhereClause.Builder(); - Operations operations = new Operations(); + Operations operations = new Operations(type); boolean hasClusteringColumnsSet = false; for (ColumnDefinition def : defs) @@ -247,7 +247,7 @@ public class UpdateStatement extends ModificationStatement boolean applyOnlyToStaticColumns = appliesOnlyToStaticColumns(operations, conditions) && !hasClusteringColumnsSet; - StatementRestrictions restrictions = new StatementRestrictions(StatementType.INSERT, + StatementRestrictions restrictions = new StatementRestrictions(type, cfm, whereClause.build(), boundNames, @@ -256,7 +256,7 @@ public class UpdateStatement extends ModificationStatement false, false); - return new UpdateStatement(StatementType.INSERT, + return new UpdateStatement(type, boundNames.size(), cfm, operations, @@ -289,7 +289,7 @@ public class UpdateStatement extends ModificationStatement List> conditions, boolean ifExists) { - super(name, attrs, conditions, false, ifExists); + super(name, StatementType.UPDATE, attrs, conditions, false, ifExists); this.updates = updates; this.whereClause = whereClause; } @@ -300,7 +300,7 @@ public class UpdateStatement extends ModificationStatement Conditions conditions, Attributes attrs) { - Operations operations = new Operations(); + Operations operations = new Operations(type); for (Pair entry : updates) { @@ -313,14 +313,13 @@ public class UpdateStatement extends ModificationStatement operations.add(operation); } - StatementRestrictions restrictions = newRestrictions(StatementType.UPDATE, - cfm, + StatementRestrictions restrictions = newRestrictions(cfm, boundNames, operations, whereClause, conditions); - return new UpdateStatement(StatementType.UPDATE, + return new UpdateStatement(type, boundNames.size(), cfm, operations, diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java index 9cde6d7ccd..ade80bb1d0 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/InsertUpdateIfConditionTest.java @@ -207,6 +207,17 @@ public class InsertUpdateIfConditionTest extends CQLTester "DELETE FROM %s WHERE k = 'k' AND i IN (0, 1) IF v = 'foo'"); assertInvalidMessage("IN on the clustering key columns is not supported with conditional deletions", "DELETE FROM %s WHERE k = 'k' AND i IN (0, 1) IF EXISTS"); + + createTable("CREATE TABLE %s(k int, s int static, i int, v text, PRIMARY KEY(k, i))"); + execute("INSERT INTO %s (k, s, i, v) VALUES ( 1, 1, 2, '1')"); + assertRows(execute("DELETE v FROM %s WHERE k = 1 AND i = 2 IF s != 1"), row(false, 1)); + assertRows(execute("DELETE v FROM %s WHERE k = 1 AND i = 2 IF s = 1"), row(true)); + assertRows(execute("SELECT * FROM %s WHERE k = 1 AND i = 2"), row(1, 2, 1, null)); + + assertRows(execute("DELETE FROM %s WHERE k = 1 AND i = 2 IF s != 1"), row(false, 1)); + assertRows(execute("DELETE FROM %s WHERE k = 1 AND i = 2 IF s = 1"), row(true)); + assertEmpty(execute("SELECT * FROM %s WHERE k = 1 AND i = 2")); + assertRows(execute("SELECT * FROM %s WHERE k = 1"), row(1, null, 1, null)); } /**