You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2014/03/19 10:56:15 UTC
git commit: Static columns with IF NOT EXISTS don't always work as
expected
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 <sy...@datastax.com>
Authored: Wed Mar 19 10:55:19 2014 +0100
Committer: Sylvain Lebresne <sy...@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)
{