cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject git commit: Static columns with IF NOT EXISTS don't always work as expected
Date Wed, 19 Mar 2014 09:56:15 GMT
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 <sylvain@datastax.com>
Authored: Wed Mar 19 10:55:19 2014 +0100
Committer: Sylvain Lebresne <sylvain@datastax.com>
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)
         {


Mime
View raw message