Add guardrail for GROUP BY queries

Patch by Josh McKenzie; reviewed by David Capwell and Andres de la Pena for CASSANDRA-17509

CHANGES.txt

@@ -1,4 +1,5 @@
 4.1
+ * Add guardrail for GROUP BY queries (CASSANDRA-17509)
  * make pylib PEP and pylint compliant (CASSANDRA-17546)
  * Add support for vnodes in jvm-dtest (CASSANDRA-17332)
  * Remove guardrails global enable flag (CASSANDRA-17499)

conf/cassandra.yaml

@@ -1629,6 +1629,8 @@ drop_compact_storage_enabled: false
 # table_properties_disallowed: []
 # Guardrail to allow/disallow user-provided timestamps. Defaults to true.
 # user_timestamps_enabled: true
+# Guardrail to allow/disallow GROUP BY functionality.
+# group_by_enabled: true
 # Guardrail to warn or fail when using a page size greater than threshold.
 # The two thresholds default to -1 to disable.
 # page_size_warn_threshold: -1

src/java/org/apache/cassandra/config/Config.java

@@ -799,6 +799,7 @@ public class Config
     public volatile Set<ConsistencyLevel> write_consistency_levels_warned = Collections.emptySet();
     public volatile Set<ConsistencyLevel> write_consistency_levels_disallowed = Collections.emptySet();
     public volatile boolean user_timestamps_enabled = true;
+    public volatile boolean group_by_enabled = true;
     public volatile boolean secondary_indexes_enabled = true;
     public volatile boolean uncompressed_tables_enabled = true;
     public volatile boolean compact_tables_enabled = true;

src/java/org/apache/cassandra/config/GuardrailsOptions.java

@@ -310,6 +310,20 @@ public class GuardrailsOptions implements GuardrailsConfig
                                   x -> config.user_timestamps_enabled = x);
     }
 
+    @Override
+    public boolean getGroupByEnabled()
+    {
+        return config.group_by_enabled;
+    }
+
+    public void setGroupByEnabled(boolean enabled)
+    {
+        updatePropertyWithLogging("group_by_enabled",
+                                  enabled,
+                                  () -> config.group_by_enabled,
+                                  x -> config.group_by_enabled = x);
+    }
+
     @Override
     public boolean getSecondaryIndexesEnabled()
     {

src/java/org/apache/cassandra/cql3/statements/SelectStatement.java

@@ -1038,6 +1038,7 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement
         public final WhereClause whereClause;
         public final Term.Raw limit;
         public final Term.Raw perPartitionLimit;
+        private ClientState state;
 
         public RawStatement(QualifiedName cfName,
                             Parameters parameters,
@@ -1056,6 +1057,8 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement
 
         public SelectStatement prepare(ClientState state)
         {
+            // Cache locally for use by Guardrails
+            this.state = state;
             return prepare(false);
         }
 
@@ -1129,6 +1132,9 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement
         {
             boolean hasGroupBy = !parameters.groups.isEmpty();
 
+            if (hasGroupBy)
+                Guardrails.groupByEnabled.ensureEnabled(state);
+
             if (selectables.isEmpty()) // wildcard query
             {
                 return hasGroupBy ? Selection.wildcardWithGroupBy(table, boundNames, parameters.isJson, restrictions.returnStaticContentOnPartitionWithNoRows())

src/java/org/apache/cassandra/db/guardrails/Guardrails.java

@@ -136,6 +136,11 @@ public final class Guardrails implements GuardrailsMBean
                     state -> !CONFIG_PROVIDER.getOrCreate(state).getUserTimestampsEnabled(),
                     "User provided timestamps (USING TIMESTAMP)");
 
+    public static final DisableFlag groupByEnabled =
+    new DisableFlag("group_by",
+                    state -> !CONFIG_PROVIDER.getOrCreate(state).getGroupByEnabled(),
+                    "GROUP BY functionality");
+
     /**
      * Guardrail disabling user's ability to turn off compression
      */
@@ -492,6 +497,18 @@ public final class Guardrails implements GuardrailsMBean
         DEFAULT_CONFIG.setCompactTablesEnabled(enabled);
     }
 
+    @Override
+    public boolean getGroupByEnabled()
+    {
+        return DEFAULT_CONFIG.getGroupByEnabled();
+    }
+
+    @Override
+    public void setGroupByEnabled(boolean enabled)
+    {
+        DEFAULT_CONFIG.setGroupByEnabled(enabled);
+    }
+
     @Override
     public int getPageSizeWarnThreshold()
     {

src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java

@@ -144,6 +144,13 @@ public interface GuardrailsConfig
      */
     boolean getCompactTablesEnabled();
 
+    /**
+     * Returns whether GROUP BY functionality is allowed
+     *
+     * @return {@code true} if allowed, {@code false} otherwise.
+     */
+    boolean getGroupByEnabled();
+
     /**
      * @return The threshold to warn when page size exceeds given size.
      */

src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java

@@ -234,6 +234,20 @@ public interface GuardrailsMBean
      */
     void setCompactTablesEnabled(boolean enabled);
 
+    /**
+     * Returns whether GROUP BY queries are allowed.
+     *
+     * @return {@code true} if allowed, {@code false} otherwise.
+     */
+    boolean getGroupByEnabled();
+
+    /**
+     * Sets whether GROUP BY queries are allowed.
+     *
+     * @param enabled {@code true} if allowed, {@code false} otherwise.
+     */
+    void setGroupByEnabled(boolean enabled);
+
     /**
      * @return The threshold to warn when requested page size greater than threshold.
      * -1 means disabled.

test/unit/org/apache/cassandra/db/guardrails/GuardrailGroupByTest.java

@@ -0,0 +1,71 @@
+/*
+ * 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.db.guardrails;
+
+import org.junit.Test;
+
+import org.apache.cassandra.schema.SchemaConstants;
+import org.apache.cassandra.schema.SchemaKeyspaceTables;
+
+public class GuardrailGroupByTest extends GuardrailTester
+{
+    private static final String query = String.format("SELECT * FROM %s.%s WHERE keyspace_name='%s' GROUP BY table_name",
+                                                      SchemaConstants.SCHEMA_KEYSPACE_NAME,
+                                                      SchemaKeyspaceTables.TABLES,
+                                                      KEYSPACE);
+
+    private void setGuardrail(boolean enabled)
+    {
+        Guardrails.instance.setGroupByEnabled(enabled);
+    }
+
+    @Test
+    public void checkExplicitlyDisabled() throws Throwable
+    {
+        setGuardrail(false);
+        assertFails(query, "GROUP BY functionality is not allowed");
+    }
+
+    @Test
+    public void testExcludedUsers() throws Throwable
+    {
+        setGuardrail(false);
+        testExcludedUsers(() -> query);
+    }
+
+    @Test
+    public void checkEnabled() throws Throwable
+    {
+        setGuardrail(true);
+        assertValid(query);
+    }
+
+    @Test
+    public void checkView() throws Throwable
+    {
+        setGuardrail(false);
+        createTable( "CREATE TABLE %s(pk int, ck int, v int, PRIMARY KEY(pk, ck))");
+        String viewName = createView("CREATE MATERIALIZED VIEW %s AS " +
+                                     "SELECT * FROM %s WHERE pk IS NOT null and ck IS NOT null " +
+                                     "PRIMARY KEY(ck, pk)");
+        String viewQuery = "SELECT * FROM " + viewName + " WHERE ck=0 GROUP BY pk";
+        assertFails(viewQuery, "GROUP BY functionality is not allowed");
+        testExcludedUsers(() -> viewQuery);
+    }
+}