Repository: cassandra Updated Branches: refs/heads/cassandra-2.0 4ce44df4d -> f1f8384a0
Static columns with IF NOT EXISTS don't always work as expected patch by slebresne; reviewed by iamaleksey for CASSANDRA-6783 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/f1f8384a Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/f1f8384a Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/f1f8384a Branch: refs/heads/cassandra-2.0 Commit: f1f8384a0c449e600b62280bbd601d6da08c3e74 Parents: 4ce44df Author: Sylvain Lebresne <[email protected]> Authored: Wed Mar 19 10:55:19 2014 +0100 Committer: Sylvain Lebresne <[email protected]> Committed: Wed Mar 19 10:55:19 2014 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../cql3/statements/ModificationStatement.java | 39 ++++++++++++-------- 2 files changed, 25 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/f1f8384a/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 650f12c..2bb3605 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -22,6 +22,7 @@ * Add paranoid disk failure option (CASSANDRA-6646) * Improve PerRowSecondaryIndex performance (CASSANDRA-6876) * Extend triggers to support CAS updates (CASSANDRA-6882) + * Static columns with IF NOT EXISTS don't always work as expected (CASSANDRA-6873) Merged from 1.2: * add extra SSL cipher suites (CASSANDRA-6613) * fix nodetool getsstables for blob PK (CASSANDRA-6803) http://git-wip-us.apache.org/repos/asf/cassandra/blob/f1f8384a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java index d96ea9c..526a26c 100644 --- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java @@ -76,7 +76,9 @@ public abstract class ModificationStatement implements CQLStatement, MeasurableF private boolean ifExists; private boolean hasNoClusteringColumns = true; - private boolean setsOnlyStaticColumns; + + private boolean setsStaticColumns; + private boolean setsRegularColumns; private final Function<ColumnCondition, ColumnIdentifier> getColumnForCondition = new Function<ColumnCondition, ColumnIdentifier>() { @@ -178,14 +180,9 @@ public abstract class ModificationStatement implements CQLStatement, MeasurableF public void addOperation(Operation op) { if (op.isStatic(cfm)) - { - if (columnOperations.isEmpty()) - setsOnlyStaticColumns = true; - } + setsStaticColumns = true; else - { - setsOnlyStaticColumns = false; - } + setsRegularColumns = true; columnOperations.add(op); } @@ -208,12 +205,14 @@ public abstract class ModificationStatement implements CQLStatement, MeasurableF List<ColumnCondition> conds = null; if (cond.column.kind == CFDefinition.Name.Kind.STATIC) { + setsStaticColumns = true; if (staticConditions == null) staticConditions = new ArrayList<ColumnCondition>(); conds = staticConditions; } else { + setsRegularColumns = true; if (columnConditions == null) columnConditions = new ArrayList<ColumnCondition>(); conds = columnConditions; @@ -361,13 +360,23 @@ public abstract class ModificationStatement implements CQLStatement, MeasurableF // UPDATE t SET s = 3 WHERE k = 0 AND v = 1 // DELETE v FROM t WHERE k = 0 AND v = 1 // sounds like you don't really understand what your are doing. - if (setsOnlyStaticColumns && columnConditions == null && (type != StatementType.INSERT || hasNoClusteringColumns)) + if (setsStaticColumns && !setsRegularColumns) { - // Reject if any clustering columns is set - for (CFDefinition.Name name : cfm.getCfDef().clusteringColumns()) - if (processedKeys.get(name.name) != null) - throw new InvalidRequestException(String.format("Invalid restriction on clustering column %s since the %s statement modifies only static columns", name.name, type)); - return cfm.getStaticColumnNameBuilder(); + // If we set no non-static columns, then it's fine not to have clustering columns + if (hasNoClusteringColumns) + return cfm.getStaticColumnNameBuilder(); + + // If we do have clustering columns however, then either it's an INSERT and the query is valid + // but we still need to build a proper prefix, or it's not an INSERT, and then we want to reject + // (see above) + if (type != StatementType.INSERT) + { + for (CFDefinition.Name name : cfm.getCfDef().clusteringColumns()) + if (processedKeys.get(name.name) != null) + throw new InvalidRequestException(String.format("Invalid restriction on clustering column %s since the %s statement modifies only static columns", name.name, type)); + // we should get there as it contradicts hasNoClusteringColumns == false + throw new AssertionError(); + } } return createClusteringPrefixBuilderInternal(variables); @@ -572,7 +581,7 @@ public abstract class ModificationStatement implements CQLStatement, MeasurableF // If we use ifNotExists, if the statement applies to any non static columns, then the condition is on the row of the non-static // columns and the prefix should be the rowPrefix. But if only static columns are set, then the ifNotExists apply to the existence // of any static columns and we should use the prefix for the "static part" of the partition. - conditions.addNotExist(setsOnlyStaticColumns ? cfm.getStaticColumnNameBuilder() : clusteringPrefix); + conditions.addNotExist(clusteringPrefix); } else if (ifExists) {
