From d9e5dd2ff9be17b1ac32aa53a612117e63c40876 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 12 Apr 2021 08:25:56 -0500 Subject: [PATCH] Fix mixed cluster GROUP BY queries Patch by Benjamin Lerer and Adam Holmberg, reviewed by brandonwilliams for CASSANDRA-16582 Co-authored-by: Benjamin Lerer Co-authored-by: Adam Holmberg --- CHANGES.txt | 1 + .../cassandra/db/filter/DataLimits.java | 10 ++- .../distributed/upgrade/GroupByTest.java | 63 +++++++++++++++++++ .../distributed/upgrade/UpgradeTestBase.java | 6 +- 4 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 test/distributed/org/apache/cassandra/distributed/upgrade/GroupByTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 1ebd470182..9662d9080a 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 bd97ea9845..845cffd445 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 0000000000..520838dd2a --- /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 46c8d24630..aeac1dc93a 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)