From 236c47e65ce95dcc7c8c75706715b5a2a88fd237 Mon Sep 17 00:00:00 2001 From: Aleksey Yeshchenko Date: Thu, 16 Aug 2018 12:45:55 +0100 Subject: [PATCH] Fix static column order for SELECT * wildcard queries patch by Aleksey Yeschenko; reviewed by Benedict Elliott Smith for CASSANDRA-14638 --- CHANGES.txt | 1 + NEWS.txt | 10 +++ doc/cql3/CQL.textile | 2 +- src/java/org/apache/cassandra/db/Columns.java | 22 ++++-- .../org/apache/cassandra/db/ColumnsTest.java | 69 ++++++++++++++++++- 5 files changed, 95 insertions(+), 9 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 7e4d88c4d1..eccece24b1 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.18 + * Fix static column order for SELECT * wildcard queries (CASSANDRA-14638) * sstableloader should use discovered broadcast address to connect intra-cluster (CASSANDRA-14522) * Fix reading columns with non-UTF names from schema (CASSANDRA-14468) Merged from 2.2: diff --git a/NEWS.txt b/NEWS.txt index 234154e582..29864bce6c 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -42,6 +42,16 @@ restore snapshots created with the previous major version using the 'sstableloader' tool. You can upgrade the file format of your snapshots using the provided 'sstableupgrade' tool. +3.0.18 +====== + +Upgrading +--------- + - The order of static columns in SELECT * has been fixed to match that of 2.0 and 2.1 - they are now sorted + alphabetically again, by their name, just like regular columns are. If you use prepared statements and + SELECT * queries, and have both simple and collection static columns in those tables, and are upgrading from an + earlier 3.0 version, then you might be affected by this change. Please see CASSANDRA-14638 for details. + 3.0.17 ===== diff --git a/doc/cql3/CQL.textile b/doc/cql3/CQL.textile index cc2b9aa327..5f35d57048 100644 --- a/doc/cql3/CQL.textile +++ b/doc/cql3/CQL.textile @@ -1079,7 +1079,7 @@ The @SELECT@ statements reads one or more columns for one or more rows in a tabl h4(#selectSelection). @@ -The @@ determines which columns needs to be queried and returned in the result-set. It consists of either the comma-separated list of or the wildcard character (@*@) to select all the columns defined for the table. +The @@ determines which columns needs to be queried and returned in the result-set. It consists of either the comma-separated list of or the wildcard character (@*@) to select all the columns defined for the table. Please note that for wildcard @SELECT@ queries the order of columns returned is not specified and is not guaranteed to be stable between Cassandra versions. A @@ is either a column name to retrieve or a @@ of one or more @@s. The function allowed are the same as for @@ and are described in the "function section":#functions. In addition to these generic functions, the @WRITETIME@ (resp. @TTL@) function allows to select the timestamp of when the column was inserted (resp. the time to live (in seconds) for the column (or null if the column has no expiration set)). diff --git a/src/java/org/apache/cassandra/db/Columns.java b/src/java/org/apache/cassandra/db/Columns.java index eb4f7611ad..45ce91ec0a 100644 --- a/src/java/org/apache/cassandra/db/Columns.java +++ b/src/java/org/apache/cassandra/db/Columns.java @@ -50,7 +50,16 @@ public class Columns extends AbstractCollection implements Col { public static final Serializer serializer = new Serializer(); public static final Columns NONE = new Columns(BTree.empty(), 0); - public static final ColumnDefinition FIRST_COMPLEX = + + private static final ColumnDefinition FIRST_COMPLEX_STATIC = + new ColumnDefinition("", + "", + ColumnIdentifier.getInterned(ByteBufferUtil.EMPTY_BYTE_BUFFER, UTF8Type.instance), + SetType.getInstance(UTF8Type.instance, true), + ColumnDefinition.NO_POSITION, + ColumnDefinition.Kind.STATIC); + + private static final ColumnDefinition FIRST_COMPLEX_REGULAR = new ColumnDefinition("", "", ColumnIdentifier.getInterned(ByteBufferUtil.EMPTY_BYTE_BUFFER, UTF8Type.instance), @@ -99,11 +108,14 @@ public class Columns extends AbstractCollection implements Col private static int findFirstComplexIdx(Object[] tree) { - // have fast path for common no-complex case + if (BTree.isEmpty(tree)) + return 0; + int size = BTree.size(tree); - if (!BTree.isEmpty(tree) && BTree.findByIndex(tree, size - 1).isSimple()) - return size; - return BTree.ceilIndex(tree, Comparator.naturalOrder(), FIRST_COMPLEX); + ColumnDefinition last = BTree.findByIndex(tree, size - 1); + return last.isSimple() + ? size + : BTree.ceilIndex(tree, Comparator.naturalOrder(), last.isStatic() ? FIRST_COMPLEX_STATIC : FIRST_COMPLEX_REGULAR); } /** diff --git a/test/unit/org/apache/cassandra/db/ColumnsTest.java b/test/unit/org/apache/cassandra/db/ColumnsTest.java index 4e3df80354..1a245a025d 100644 --- a/test/unit/org/apache/cassandra/db/ColumnsTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnsTest.java @@ -23,6 +23,7 @@ import java.util.*; import java.util.concurrent.ThreadLocalRandom; import java.util.function.Predicate; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterators; import com.google.common.collect.Lists; @@ -33,6 +34,7 @@ import junit.framework.Assert; import org.apache.cassandra.MockSchema; import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.config.ColumnDefinition; +import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.marshal.SetType; import org.apache.cassandra.db.marshal.UTF8Type; import org.apache.cassandra.io.util.DataInputBuffer; @@ -129,21 +131,77 @@ public class ColumnsTest { List names = new ArrayList<>(); for (int i = 0; i < 50; i++) - names.add("clustering_" + i); + names.add("regular_" + i); List defs = new ArrayList<>(); - addClustering(names, defs); + addRegular(names, defs); Columns columns = Columns.from(new HashSet<>(defs)); defs = new ArrayList<>(); - addClustering(names.subList(0, 8), defs); + addRegular(names.subList(0, 8), defs); Columns subset = Columns.from(new HashSet<>(defs)); Assert.assertTrue(columns.containsAll(subset)); } + @Test + public void testStaticColumns() + { + testColumns(ColumnDefinition.Kind.STATIC); + } + + @Test + public void testRegularColumns() + { + testColumns(ColumnDefinition.Kind.REGULAR); + } + + private void testColumns(ColumnDefinition.Kind kind) + { + List definitions = ImmutableList.of( + def("a", UTF8Type.instance, kind), + def("b", SetType.getInstance(UTF8Type.instance, true), kind), + def("c", UTF8Type.instance, kind), + def("d", SetType.getInstance(UTF8Type.instance, true), kind), + def("e", UTF8Type.instance, kind), + def("f", SetType.getInstance(UTF8Type.instance, true), kind), + def("g", UTF8Type.instance, kind), + def("h", SetType.getInstance(UTF8Type.instance, true), kind) + ); + Columns columns = Columns.from(definitions); + + // test simpleColumnCount() + Assert.assertEquals(4, columns.simpleColumnCount()); + + // test simpleColumns() + List simpleColumnsExpected = + ImmutableList.of(definitions.get(0), definitions.get(2), definitions.get(4), definitions.get(6)); + List simpleColumnsActual = new ArrayList<>(); + Iterators.addAll(simpleColumnsActual, columns.simpleColumns()); + Assert.assertEquals(simpleColumnsExpected, simpleColumnsActual); + + // test complexColumnCount() + Assert.assertEquals(4, columns.complexColumnCount()); + + // test complexColumns() + List complexColumnsExpected = + ImmutableList.of(definitions.get(1), definitions.get(3), definitions.get(5), definitions.get(7)); + List complexColumnsActual = new ArrayList<>(); + Iterators.addAll(complexColumnsActual, columns.complexColumns()); + Assert.assertEquals(complexColumnsExpected, complexColumnsActual); + + // test size() + Assert.assertEquals(8, columns.size()); + + // test selectOrderIterator() + List columnsExpected = definitions; + List columnsActual = new ArrayList<>(); + Iterators.addAll(columnsActual, columns.selectOrderIterator()); + Assert.assertEquals(columnsExpected, columnsActual); + } + private void testSerializeSubset(ColumnsCheck input) throws IOException { testSerializeSubset(input.columns, input.columns, input.definitions); @@ -403,6 +461,11 @@ public class ColumnsTest results.add(ColumnDefinition.regularDef(cfMetaData, bytes(name), SetType.getInstance(UTF8Type.instance, true))); } + private static ColumnDefinition def(String name, AbstractType type, ColumnDefinition.Kind kind) + { + return new ColumnDefinition(cfMetaData, bytes(name), type, ColumnDefinition.NO_POSITION, kind); + } + private static CFMetaData mock(Columns columns) { if (columns.isEmpty())