You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by br...@apache.org on 2021/04/12 13:39:53 UTC

[cassandra] branch trunk updated: Fix mixed cluster GROUP BY queries

This is an automated email from the ASF dual-hosted git repository.

brandonwilliams pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/trunk by this push:
     new d9e5dd2  Fix mixed cluster GROUP BY queries
d9e5dd2 is described below

commit d9e5dd2ff9be17b1ac32aa53a612117e63c40876
Author: Brandon Williams <br...@apache.org>
AuthorDate: Mon Apr 12 08:25:56 2021 -0500

    Fix mixed cluster GROUP BY queries
    
    Patch by Benjamin Lerer and Adam Holmberg, reviewed by brandonwilliams
    for CASSANDRA-16582
    
    Co-authored-by: Benjamin Lerer <bl...@apache.org>
    Co-authored-by: Adam Holmberg <ad...@datastax.com>
---
 CHANGES.txt                                        |  1 +
 .../org/apache/cassandra/db/filter/DataLimits.java | 10 +++-
 .../cassandra/distributed/upgrade/GroupByTest.java | 63 ++++++++++++++++++++++
 .../distributed/upgrade/UpgradeTestBase.java       |  6 +--
 4 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 1ebd470..9662d90 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 4.0-rc1
+ * Fix mixed cluster GROUP BY queries (CASSANDRA-16582)
  * Upgrade jflex to 1.8.2 (CASSANDRA-16576)
  * Binary releases no longer bundle the apidocs (javadoc) (CASSANDRA-15561)
  * Fix Streaming Repair metrics (CASSANDRA-16190)
diff --git a/src/java/org/apache/cassandra/db/filter/DataLimits.java b/src/java/org/apache/cassandra/db/filter/DataLimits.java
index bd97ea9..845cffd 100644
--- a/src/java/org/apache/cassandra/db/filter/DataLimits.java
+++ b/src/java/org/apache/cassandra/db/filter/DataLimits.java
@@ -81,7 +81,15 @@ public abstract class DataLimits
     // partition (see SelectStatement.makeFilter). So an "unbounded" distinct is still actually doing some filtering.
     public static final DataLimits DISTINCT_NONE = new CQLLimits(NO_LIMIT, 1, true);
 
-    public enum Kind { CQL_LIMIT, CQL_PAGING_LIMIT, CQL_GROUP_BY_LIMIT, CQL_GROUP_BY_PAGING_LIMIT }
+    public enum Kind
+    {
+        CQL_LIMIT,
+        CQL_PAGING_LIMIT,
+        @Deprecated THRIFT_LIMIT, //Deprecated and unused in 4.0, stop publishing in 5.0, reclaim in 6.0
+        @Deprecated SUPER_COLUMN_COUNTING_LIMIT, //Deprecated and unused in 4.0, stop publishing in 5.0, reclaim in 6.0
+        CQL_GROUP_BY_LIMIT,
+        CQL_GROUP_BY_PAGING_LIMIT,
+    }
 
     public static DataLimits cqlLimits(int cqlRowLimit)
     {
diff --git a/test/distributed/org/apache/cassandra/distributed/upgrade/GroupByTest.java b/test/distributed/org/apache/cassandra/distributed/upgrade/GroupByTest.java
new file mode 100644
index 0000000..520838d
--- /dev/null
+++ b/test/distributed/org/apache/cassandra/distributed/upgrade/GroupByTest.java
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.upgrade;
+
+import org.junit.Test;
+
+import org.apache.cassandra.distributed.api.ConsistencyLevel;
+import org.apache.cassandra.distributed.shared.Versions;
+
+import static org.apache.cassandra.distributed.api.Feature.GOSSIP;
+import static org.apache.cassandra.distributed.api.Feature.NATIVE_PROTOCOL;
+import static org.apache.cassandra.distributed.api.Feature.NETWORK;
+import static org.apache.cassandra.distributed.shared.AssertUtils.assertRows;
+import static org.apache.cassandra.distributed.shared.AssertUtils.row;
+
+public class GroupByTest extends UpgradeTestBase
+{
+    @Test
+    public void testReads() throws Throwable
+    {
+        // CASSANDRA-16582: group-by across mixed version cluster would fail with ArrayIndexOutOfBoundException
+        new UpgradeTestBase.TestCase()
+        .nodes(2)
+        .upgrade(Versions.Major.v3X, Versions.Major.v4)
+        .nodesToUpgrade(1)
+        .withConfig(config -> config.with(GOSSIP, NETWORK))
+        .setup(cluster -> {
+            cluster.schemaChange(withKeyspace("CREATE TABLE %s.t (a int, b int, c int, v int, primary key (a, b, c))"));
+            String insert = withKeyspace("INSERT INTO %s.t (a, b, c, v) VALUES (?, ?, ?, ?)");
+            cluster.coordinator(1).execute(insert, ConsistencyLevel.ALL, 1, 1, 1, 3);
+            cluster.coordinator(1).execute(insert, ConsistencyLevel.ALL, 1, 2, 1, 6);
+            cluster.coordinator(1).execute(insert, ConsistencyLevel.ALL, 1, 2, 2, 12);
+            cluster.coordinator(1).execute(insert, ConsistencyLevel.ALL, 1, 3, 2, 12);
+        })
+        .runAfterNodeUpgrade((cluster, node) -> {
+            String query = withKeyspace("SELECT a, b, count(c) FROM %s.t GROUP BY a,b");
+            Object[][] expectedResult = {
+            row(1, 1, 1L),
+            row(1, 2, 2L),
+            row(1, 3, 1L)
+            };
+            assertRows(cluster.coordinator(1).execute(query, ConsistencyLevel.ALL), expectedResult);
+            assertRows(cluster.coordinator(2).execute(query, ConsistencyLevel.ALL), expectedResult);
+        })
+        .run();
+    }
+}
diff --git a/test/distributed/org/apache/cassandra/distributed/upgrade/UpgradeTestBase.java b/test/distributed/org/apache/cassandra/distributed/upgrade/UpgradeTestBase.java
index 46c8d24..aeac1dc 100644
--- a/test/distributed/org/apache/cassandra/distributed/upgrade/UpgradeTestBase.java
+++ b/test/distributed/org/apache/cassandra/distributed/upgrade/UpgradeTestBase.java
@@ -26,6 +26,7 @@ import java.util.List;
 import java.util.Set;
 import java.util.function.Consumer;
 
+import com.google.common.base.Preconditions;
 import org.junit.After;
 import org.junit.BeforeClass;
 
@@ -33,9 +34,7 @@ import org.apache.cassandra.db.DecoratedKey;
 import org.apache.cassandra.dht.Murmur3Partitioner;
 import org.apache.cassandra.distributed.UpgradeableCluster;
 import org.apache.cassandra.distributed.api.ICluster;
-import org.apache.cassandra.distributed.api.IInstance;
 import org.apache.cassandra.distributed.api.IInstanceConfig;
-import org.apache.cassandra.distributed.impl.AbstractCluster.AbstractBuilder;
 import org.apache.cassandra.distributed.impl.Instance;
 import org.apache.cassandra.distributed.shared.DistributedTestBase;
 import org.apache.cassandra.distributed.shared.Versions;
@@ -44,6 +43,7 @@ import org.apache.cassandra.utils.ByteBufferUtil;
 import static org.apache.cassandra.distributed.shared.Versions.Major;
 import static org.apache.cassandra.distributed.shared.Versions.Version;
 import static org.apache.cassandra.distributed.shared.Versions.find;
+import static org.apache.commons.lang3.ArrayUtils.isNotEmpty;
 
 public class UpgradeTestBase extends DistributedTestBase
 {
@@ -83,6 +83,7 @@ public class UpgradeTestBase extends DistributedTestBase
 
         public TestVersions(Version initial, Version ... upgrade)
         {
+            Preconditions.checkArgument(isNotEmpty(upgrade), "TestVersions must be constructed with one or more target upgrade versions.");
             this.initial = initial;
             this.upgrade = upgrade;
         }
@@ -189,7 +190,6 @@ public class UpgradeTestBase extends DistributedTestBase
                         runAfterClusterUpgrade.run(cluster);
                     }
                 }
-
             }
         }
         public TestCase nodesToUpgrade(int ... nodes)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org