CQL3 Predicate logic bug when using composite columns patch by slebresne; reviewed by tjake for CASSANDRA-4759
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/eff4b686 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/eff4b686 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/eff4b686 Branch: refs/heads/trunk Commit: eff4b686653fb15593c7fecd2033f60b65c9ee8f Parents: 2ce7b9b Author: Sylvain Lebresne <[email protected]> Authored: Thu Oct 4 17:00:38 2012 +0200 Committer: Sylvain Lebresne <[email protected]> Committed: Thu Oct 4 17:00:38 2012 +0200 ---------------------------------------------------------------------- CHANGES.txt | 2 +- .../cassandra/cql3/statements/SelectStatement.java | 21 +++++++++----- 2 files changed, 14 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/eff4b686/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index cb725cc..c0a252e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -13,7 +13,7 @@ * fix re-created keyspace disappering after 1.1.5 upgrade (CASSANDRA-4698) * (CLI) display elapsed time in 2 fraction digits (CASSANDRA-3460) * add authentication support to sstableloader (CASSANDRA-4712) - * Fix CQL3 'is reversed' logic (CASSANDRA-4716) + * Fix CQL3 'is reversed' logic (CASSANDRA-4716, 4759) * (CQL3) Don't return ReversedType in result set metadata (CASSANDRA-4717) * Pluggable Thrift transport factories for CLI (CASSANDRA-4609) * Backport adding AlterKeyspace statement (CASSANDRA-4611) http://git-wip-us.apache.org/repos/asf/cassandra/blob/eff4b686/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index d288294..92a8d67 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -334,8 +334,8 @@ public class SelectStatement implements CQLStatement if (isColumnRange()) { SliceRange sliceRange = new SliceRange(); - sliceRange.start = getRequestedBound(isReversed ? Bound.END : Bound.START, variables); - sliceRange.finish = getRequestedBound(isReversed ? Bound.START : Bound.END, variables); + sliceRange.start = getRequestedBound(Bound.START, variables); + sliceRange.finish = getRequestedBound(Bound.END, variables); sliceRange.reversed = isReversed; sliceRange.count = -1; // We use this for range slices, where the count is ignored in favor of the global column count thriftSlicePredicate.slice_range = sliceRange; @@ -495,6 +495,11 @@ public class SelectStatement implements CQLStatement { assert isColumnRange(); + // The end-of-component of composite doesn't depend on whether the + // component type is reversed or not (i.e. the ReversedType is applied + // to the component comparator but not to the end-of-component itself), + // it only depends on whether the slice is reversed + Bound eocBound = isReversed ? Bound.reverse(bound) : bound; ColumnNameBuilder builder = cfDef.getColumnNameBuilder(); for (CFDefinition.Name name : cfDef.columns.values()) { @@ -508,7 +513,7 @@ public class SelectStatement implements CQLStatement // There wasn't any non EQ relation on that key, we select all records having the preceding component as prefix. // For composites, if there was preceding component and we're computing the end, we must change the last component // End-Of-Component, otherwise we would be selecting only one record. - if (builder.componentCount() > 0 && b == Bound.END) + if (builder.componentCount() > 0 && eocBound == Bound.END) return builder.buildAsEndOfRange(); else return builder.build(); @@ -523,7 +528,7 @@ public class SelectStatement implements CQLStatement { Term t = r.bound(b); assert t != null; - return builder.add(t, r.getRelation(b), variables).build(); + return builder.add(t, r.getRelation(eocBound, b), variables).build(); } } // Means no relation at all or everything was an equal @@ -1283,14 +1288,14 @@ public class SelectStatement implements CQLStatement return bounds[b.idx] == null || boundInclusive[b.idx]; } - public Relation.Type getRelation(Bound b) + public Relation.Type getRelation(Bound eocBound, Bound inclusiveBound) { - switch (b) + switch (eocBound) { case START: - return boundInclusive[b.idx] ? Relation.Type.GTE : Relation.Type.GT; + return boundInclusive[inclusiveBound.idx] ? Relation.Type.GTE : Relation.Type.GT; case END: - return boundInclusive[b.idx] ? Relation.Type.LTE : Relation.Type.LT; + return boundInclusive[inclusiveBound.idx] ? Relation.Type.LTE : Relation.Type.LT; } throw new AssertionError(); }
