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 11:06:00 UTC
[1/2] git commit: Static columns with IF NOT EXISTS don't always work
as expected
Repository: cassandra
Updated Branches:
refs/heads/cassandra-2.1 a7a3282da -> e23e57fff
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.1
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)
{
[2/2] git commit: Merge branch 'cassandra-2.0' into cassandra-2.1
Posted by sl...@apache.org.
Merge branch 'cassandra-2.0' into cassandra-2.1
Conflicts:
CHANGES.txt
src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/e23e57ff
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/e23e57ff
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/e23e57ff
Branch: refs/heads/cassandra-2.1
Commit: e23e57fffe3573eae45f6ee54e400b837d6c6a14
Parents: a7a3282 f1f8384
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Mar 19 11:05:51 2014 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Mar 19 11:05:51 2014 +0100
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../org/apache/cassandra/config/Schema.java | 4 +-
.../cql3/statements/ModificationStatement.java | 39 ++++++++++++--------
3 files changed, 27 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/e23e57ff/CHANGES.txt
----------------------------------------------------------------------
diff --cc CHANGES.txt
index 33111ea,2bb3605..159d242
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@@ -48,8 -22,10 +48,9 @@@ Merged from 2.0
* 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)
++ * Static columns with IF NOT EXISTS don't always work as expected (CASSANDRA-6873)
2.0.6
http://git-wip-us.apache.org/repos/asf/cassandra/blob/e23e57ff/src/java/org/apache/cassandra/config/Schema.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/config/Schema.java
index c5c080f,f0a49dc..5c38a78
--- a/src/java/org/apache/cassandra/config/Schema.java
+++ b/src/java/org/apache/cassandra/config/Schema.java
@@@ -30,6 -31,7 +30,7 @@@ import org.slf4j.LoggerFactory
import org.apache.cassandra.db.*;
import org.apache.cassandra.db.Keyspace;
-import org.apache.cassandra.db.marshal.AbstractType;
++import org.apache.cassandra.db.index.SecondaryIndexManager;
import org.apache.cassandra.io.sstable.Descriptor;
import org.apache.cassandra.service.MigrationManager;
import org.apache.cassandra.utils.ConcurrentBiMap;
@@@ -356,9 -388,9 +357,8 @@@ public class Schem
continue;
// we want to digest only live columns
-- ColumnFamilyStore.removeDeletedColumnsOnly(row.cf, Integer.MAX_VALUE);
++ ColumnFamilyStore.removeDeletedColumnsOnly(row.cf, Integer.MAX_VALUE, SecondaryIndexManager.nullUpdater);
row.cf.purgeTombstones(Integer.MAX_VALUE);
--
row.cf.updateDigest(versionDigest);
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/e23e57ff/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
index eafdba5,526a26c..acc4802
--- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
@@@ -69,13 -76,15 +69,15 @@@ public abstract class ModificationState
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>()
+ private final Function<ColumnCondition, ColumnDefinition> getColumnForCondition = new Function<ColumnCondition, ColumnDefinition>()
{
- public ColumnIdentifier apply(ColumnCondition cond)
+ public ColumnDefinition apply(ColumnCondition cond)
{
- return cond.column.name;
+ return cond.column;
}
};
@@@ -158,15 -179,10 +160,10 @@@
public void addOperation(Operation op)
{
- if (op.isStatic(cfm))
+ if (op.column.isStatic())
- {
- if (columnOperations.isEmpty())
- setsOnlyStaticColumns = true;
- }
+ setsStaticColumns = true;
else
- {
- setsOnlyStaticColumns = false;
- }
+ setsRegularColumns = true;
columnOperations.add(op);
}
@@@ -187,8 -203,9 +184,9 @@@
public void addCondition(ColumnCondition cond) throws InvalidRequestException
{
List<ColumnCondition> conds = null;
- if (cond.column.kind == CFDefinition.Name.Kind.STATIC)
+ if (cond.column.isStatic())
{
+ setsStaticColumns = true;
if (staticConditions == null)
staticConditions = new ArrayList<ColumnCondition>();
conds = staticConditions;
@@@ -338,13 -360,23 +337,23 @@@
// 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 (ColumnDefinition def : cfm.clusteringColumns())
- if (processedKeys.get(def.name) != null)
- throw new InvalidRequestException(String.format("Invalid restriction on clustering column %s since the %s statement modifies only static columns", def.name, type));
- return cfm.comparator.staticPrefix();
+ // If we set no non-static columns, then it's fine not to have clustering columns
+ if (hasNoClusteringColumns)
- return cfm.getStaticColumnNameBuilder();
++ return cfm.comparator.staticPrefix();
+
+ // 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));
++ for (ColumnDefinition def : cfm.clusteringColumns())
++ if (processedKeys.get(def.name) != null)
++ throw new InvalidRequestException(String.format("Invalid restriction on clustering column %s since the %s statement modifies only static columns", def.name, type));
+ // we should get there as it contradicts hasNoClusteringColumns == false
+ throw new AssertionError();
+ }
}
return createClusteringPrefixBuilderInternal(variables);
@@@ -524,9 -579,9 +533,9 @@@
if (ifNotExists)
{
// 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
+ // columns and the prefix should be the clusteringPrefix. 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.comparator.staticPrefix() : clusteringPrefix);
+ conditions.addNotExist(clusteringPrefix);
}
else if (ifExists)
{