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 <jmcken...@apache.org> 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: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org