This is an automated email from the ASF dual-hosted git repository.
jmckenzie 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 72af1f75fc Add guardrail for GROUP BY queries
72af1f75fc is described below
commit 72af1f75fccf877f8996da0a0d8bc1a6adcd30e0
Author: Josh McKenzie <[email protected]>
AuthorDate: Thu Mar 31 11:09:07 2022 -0400
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 +
conf/cassandra.yaml | 2 +
src/java/org/apache/cassandra/config/Config.java | 1 +
.../apache/cassandra/config/GuardrailsOptions.java | 14 +++++
.../cassandra/cql3/statements/SelectStatement.java | 6 ++
.../apache/cassandra/db/guardrails/Guardrails.java | 17 ++++++
.../cassandra/db/guardrails/GuardrailsConfig.java | 7 +++
.../cassandra/db/guardrails/GuardrailsMBean.java | 14 +++++
.../db/guardrails/GuardrailGroupByTest.java | 71 ++++++++++++++++++++++
9 files changed, 133 insertions(+)
diff --git a/CHANGES.txt b/CHANGES.txt
index a0aa717be6..377e91b75c 100644
--- a/CHANGES.txt
+++ b/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)
diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml
index 668ab83db4..e8ebe029ef 100644
--- a/conf/cassandra.yaml
+++ b/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
diff --git a/src/java/org/apache/cassandra/config/Config.java
b/src/java/org/apache/cassandra/config/Config.java
index 9717dbc135..464c7992a0 100644
--- a/src/java/org/apache/cassandra/config/Config.java
+++ b/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;
diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java
b/src/java/org/apache/cassandra/config/GuardrailsOptions.java
index 5047cbaa2b..b00075ab21 100644
--- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java
+++ b/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()
{
diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
index 1637ba5488..d55a64b951 100644
--- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
+++ b/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())
diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
index baac800a0e..cc54b279a4 100644
--- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java
+++ b/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()
{
diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
index 7ad9d0afb1..08b3d5677b 100644
--- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java
+++ b/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.
*/
diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
index 2cc1e03dbe..771db67ce6 100644
--- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java
+++ b/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.
diff --git
a/test/unit/org/apache/cassandra/db/guardrails/GuardrailGroupByTest.java
b/test/unit/org/apache/cassandra/db/guardrails/GuardrailGroupByTest.java
new file mode 100644
index 0000000000..f25cba2829
--- /dev/null
+++ b/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);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
