From d5da19ae740147498006457f1aa974c3f2db192e Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Fri, 30 Jan 2026 09:16:13 +0530 Subject: [PATCH 01/19] added use_misprepare_statements_enabled flag (not implemented) --- conf/cassandra.yaml | 4 ++++ .../org/apache/cassandra/config/Config.java | 2 ++ .../cassandra/config/DatabaseDescriptor.java | 14 ++++++++++++++ .../cassandra/config/GuardrailsOptions.java | 6 ++++++ .../cassandra/db/guardrails/Guardrails.java | 9 +++++++++ .../db/guardrails/GuardrailsConfig.java | 8 ++++++++ .../cassandra/service/StorageService.java | 13 +++++++++++++ .../service/StorageServiceMBean.java | 10 ++++++++++ .../config/DatabaseDescriptorTest.java | 19 +++++++++++++++++++ 9 files changed, 85 insertions(+) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 5f3a935be3cf..6a2cf1fd6a7f 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -2531,6 +2531,10 @@ drop_compact_storage_enabled: false # This would also apply to system keyspaces. # maximum_replication_factor_warn_threshold: -1 # maximum_replication_factor_fail_threshold: -1 +# +# When true, allows the use of misprepared statements, only warns in the logs. +# When false, misprepared statements will result in an error to the client. +# use_misprepare_statements_enabled: false #role_name_policy: # # Implementation class of a validator. When not in form of FQCN, the diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 2f290d8182ed..553b078ec2ea 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -665,6 +665,8 @@ public static class SSTableConfig public volatile boolean use_statements_enabled = true; + public volatile boolean use_misprepare_statements_enabled = false; + /** * Optionally disable asynchronous UDF execution. * Disabling asynchronous UDF execution also implicitly disables the security-manager! diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index 809a44fdcc63..c24c632549f3 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -5474,6 +5474,11 @@ public static boolean getUseStatementsEnabled() return conf.use_statements_enabled; } + public static boolean getUseMispreparedStatementsEnabled() + { + return conf.use_misprepare_statements_enabled; + } + public static void setUseStatementsEnabled(boolean enabled) { if (enabled != conf.use_statements_enabled) @@ -5483,6 +5488,15 @@ public static void setUseStatementsEnabled(boolean enabled) } } + public static void setUseMispreparedStatementsEnabled(boolean enabled) + { + if (enabled != conf.use_misprepare_statements_enabled) + { + logger.info("Setting use_misprepare_statements_enabled to {}", enabled); + conf.use_misprepare_statements_enabled = enabled; + } + } + public static AccordSpec getAccord() { diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index 7443565d89e6..67561c7fb499 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -1329,6 +1329,12 @@ public boolean getVectorTypeEnabled() return config.vector_type_enabled; } + @Override + public boolean getMispreparedStatementsEnabled() + { + return config.use_misprepare_statements_enabled; + } + @Override public void setVectorTypeEnabled(boolean enabled) { diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 6738d9a7d950..73463a5782ff 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -613,6 +613,15 @@ public final class Guardrails implements GuardrailsMBean format("The keyspace %s has a replication factor of %s, above the %s threshold of %s.", what, value, isWarning ? "warning" : "failure", threshold)); + /** + * Guardrail on mis-prepared statements. + */ + public static final EnableFlag MispreparedStatementsEnabled = + new EnableFlag("misprepared_statements_enabled", + "Mis-prepared statements cause server-side memory exhaustion and high GC pressure by flooding the statement cache with non-reusable query entries", + state -> CONFIG_PROVIDER.getOrCreate(state).getMispreparedStatementsEnabled(), + "Mis-prepared statements"); + public static final MaxThreshold maximumAllowableTimestamp = new MaxThreshold("maximum_timestamp", "Timestamps too far in the future can lead to data that can't be easily overwritten", diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index 8e0d02025975..1040c914a44b 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -406,6 +406,12 @@ public interface GuardrailsConfig */ boolean getVectorTypeEnabled(); + /** + * @return Whether use misprepared statements is enabled. If not enabled, misprepared statements will fail + * otherwise warned. + */ + boolean getMispreparedStatementsEnabled(); + /** * Sets whether new columns can be created with vector type * @@ -509,6 +515,8 @@ public interface GuardrailsConfig void setIntersectFilteringQueryEnabled(boolean value); + + /** * @return A timestamp that if a user supplied timestamp is after will trigger a warning */ diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index dc9d9e3f680b..abc320d46162 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -4836,6 +4836,19 @@ public void setColumnIndexCacheSize(int cacheSizeInKB) logger.info("Updated column_index_cache_size to {}", cacheSizeInKB); } + @Override + public boolean getUseMispreparedStatementsEnabled() + { + return DatabaseDescriptor.getUseMispreparedStatementsEnabled(); + } + + @Override + public void setUseMispreparedStatementsEnabled(boolean enabled) + { + DatabaseDescriptor.setUseMispreparedStatementsEnabled(enabled); + logger.info("Updated use_misprepared_statements_enabled to {}", enabled); + } + /* * In CASSANDRA-17668, JMX setters that did not throw standard exceptions were deprecated in favor of ones that do. * For consistency purposes, the respective getter "getColumnIndexCacheSize" was also deprecated and replaced by diff --git a/src/java/org/apache/cassandra/service/StorageServiceMBean.java b/src/java/org/apache/cassandra/service/StorageServiceMBean.java index 9c4ae481ec11..f6136a288c5a 100644 --- a/src/java/org/apache/cassandra/service/StorageServiceMBean.java +++ b/src/java/org/apache/cassandra/service/StorageServiceMBean.java @@ -1097,6 +1097,16 @@ default int upgradeSSTables(String keyspaceName, boolean excludeCurrentVersion, @Deprecated(since = "5.0") public void setColumnIndexCacheSize(int cacheSizeInKB); + /** + * Returns whether use of misprepared statements is enabled + */ + public boolean getUseMispreparedStatementsEnabled(); + + /** + * Sets whether use of misprepared statements is enabled + */ + public void setUseMispreparedStatementsEnabled(boolean enabled); + /** Returns the threshold for skipping the column index when caching partition info **/ public int getColumnIndexCacheSizeInKiB(); diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java index 678f6746f1df..36230b4a944f 100644 --- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java +++ b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java @@ -781,6 +781,25 @@ public void testRowIndexSizeWarnEqAbort() DatabaseDescriptor.applyReadThresholdsValidations(conf); } + @Test + public void testMispreparedStatementsEnabled() { + boolean originalValue = DatabaseDescriptor.getUseMispreparedStatementsEnabled(); + try + { + DatabaseDescriptor.setUseMispreparedStatementsEnabled(true); + Assert.assertTrue("Value should be true after setting to true", + DatabaseDescriptor.getUseMispreparedStatementsEnabled()); + + DatabaseDescriptor.setUseMispreparedStatementsEnabled(false); + Assert.assertFalse("Value should be false after setting to false", + DatabaseDescriptor.getUseMispreparedStatementsEnabled()); + } + finally + { + DatabaseDescriptor.setUseMispreparedStatementsEnabled(originalValue); + } + } + @Test public void testRowIndexSizeWarnEnabledAbortDisabled() { From 33c321909f113a1282a34cb06149117226cccffd Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Sat, 31 Jan 2026 01:12:00 +0530 Subject: [PATCH 02/19] implemented use_misprepared_statements flag --- .idea/codeStyles/Project.xml | 315 ------------------ .idea/codeStyles/codeStyleConfig.xml | 5 - conf/cassandra.yaml | 3 - .../apache/cassandra/cql3/QueryProcessor.java | 47 +++ .../cql3/MispreparedStatementsTest.java | 256 ++++++++++++++ 5 files changed, 303 insertions(+), 323 deletions(-) delete mode 100644 .idea/codeStyles/Project.xml delete mode 100644 .idea/codeStyles/codeStyleConfig.xml create mode 100644 test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml deleted file mode 100644 index afcd5ec49778..000000000000 --- a/.idea/codeStyles/Project.xml +++ /dev/null @@ -1,315 +0,0 @@ - - - - \ No newline at end of file diff --git a/.idea/codeStyles/codeStyleConfig.xml b/.idea/codeStyles/codeStyleConfig.xml deleted file mode 100644 index 0f7bc519db61..000000000000 --- a/.idea/codeStyles/codeStyleConfig.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - - diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 6a2cf1fd6a7f..019841551ef9 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -197,9 +197,6 @@ batchlog_replay_throttle: 1024KiB # If using PasswordAuthenticator, CassandraRoleManager must also be used (see below) authenticator: class_name: AllowAllAuthenticator -# MutualTlsAuthenticator can be configured using the following configuration. One can add their own validator -# which implements MutualTlsCertificateValidator class and provide logic for extracting identity out of certificates -# and validating certificates. # class_name: org.apache.cassandra.auth.MutualTlsAuthenticator # parameters: # validator_class_name: org.apache.cassandra.auth.SpiffeCertificateValidator diff --git a/src/java/org/apache/cassandra/cql3/QueryProcessor.java b/src/java/org/apache/cassandra/cql3/QueryProcessor.java index 606da473056d..5f0cf7469c49 100644 --- a/src/java/org/apache/cassandra/cql3/QueryProcessor.java +++ b/src/java/org/apache/cassandra/cql3/QueryProcessor.java @@ -50,6 +50,7 @@ import org.apache.cassandra.cql3.functions.FunctionName; import org.apache.cassandra.cql3.functions.UDAggregate; import org.apache.cassandra.cql3.functions.UDFunction; +import org.apache.cassandra.cql3.restrictions.StatementRestrictions; import org.apache.cassandra.cql3.selection.ResultSetBuilder; import org.apache.cassandra.cql3.statements.BatchStatement; import org.apache.cassandra.cql3.statements.ModificationStatement; @@ -64,6 +65,7 @@ import org.apache.cassandra.db.ReadResponse; import org.apache.cassandra.db.SinglePartitionReadQuery; import org.apache.cassandra.db.SystemKeyspace; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.partitions.PartitionIterator; import org.apache.cassandra.db.partitions.PartitionIterators; @@ -495,6 +497,11 @@ public static Prepared parseAndPrepare(String query, ClientState clientState, bo CQLStatement statement = raw.prepare(clientState); statement.validate(clientState); + if (!isInternal) + { + checkMispreparedGuardrail(statement, clientState); + } + // Set CQL string for AlterSchemaStatement as this is used to serialize the transformation // in the cluster metadata log if (statement instanceof AlterSchemaStatement) @@ -513,6 +520,46 @@ public static Prepared parseAndPrepare(String query, ClientState clientState, bo return res; } + private static void checkMispreparedGuardrail(CQLStatement statement, ClientState clientState) + { + if (clientState.isInternal || !Guardrails.MispreparedStatementsEnabled.enabled(clientState)) + { + return; + } + + if (isMisprepared(statement)) + { + Guardrails.MispreparedStatementsEnabled.ensureEnabled(clientState); + } + } + + private static boolean isMisprepared(CQLStatement statement) + { + if (!statement.getBindVariables().isEmpty()) + return false; + + if (statement instanceof BatchStatement) + { + for (ModificationStatement subStmt : ((BatchStatement) statement).getStatements()) + { + if (isMisprepared(subStmt)) + return true; + } + return false; + } + + StatementRestrictions restrictions = null; + if (statement instanceof SelectStatement) + restrictions = ((SelectStatement) statement).getRestrictions(); + else if (statement instanceof ModificationStatement) + restrictions = ((ModificationStatement) statement).getRestrictions(); + + return (restrictions != null && + (restrictions.hasPartitionKeyRestrictions() + || restrictions.hasClusteringColumnsRestrictions() + || restrictions.hasNonPrimaryKeyRestrictions())); + } + public static UntypedResultSet executeInternal(String query, Object... values) { Prepared prepared = prepareInternal(query); diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java new file mode 100644 index 000000000000..6a104abed98a --- /dev/null +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -0,0 +1,256 @@ +/* + * 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.cql3; + +import java.net.InetSocketAddress; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.auth.AuthenticatedUser; +import org.apache.cassandra.auth.CassandraAuthorizer; +import org.apache.cassandra.auth.PasswordAuthenticator; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.service.ClientState; + +import static org.junit.Assert.assertTrue; + +public class MispreparedStatementsTest extends CQLTester +{ + + private static final ClientState state = ClientState.forExternalCalls(new InetSocketAddress("127.0.0.1", 9042)); + + @BeforeClass + public static void setupGlobalConfig() + { + DatabaseDescriptor.setAuthenticator(new PasswordAuthenticator()); + DatabaseDescriptor.setAuthorizer(new CassandraAuthorizer()); + DatabaseDescriptor.setUseMispreparedStatementsEnabled(false); + } + + @Before + public void setUp() + { + AuthenticatedUser nonSuperUser = new AuthenticatedUser("regular-user") + { + @Override + public boolean isSuper() + { + return false; + } + + @Override + public boolean isSystem() + { + return false; + } + + @Override + public boolean isAnonymous() + { + return false; + } + + @Override + public boolean canLogin() + { + return true; + } + }; + + state.login(nonSuperUser); + state.setKeyspace(KEYSPACE); + } + + @After + public void tearDown() + { + DatabaseDescriptor.setUseMispreparedStatementsEnabled(false); + } + + @Test + public void testSelectGuardrail() + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + String query = String.format("SELECT * FROM %s WHERE id = 1", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error for mis-prepared SELECT statement"); + } + catch (Exception e) + { + assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } + + + @Test + public void testModificationGuardrail() + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + String fullName = KEYSPACE + '.' + currentTable(); + String query = String.format("UPDATE %s SET val = 'new_name' WHERE id = 1", fullName); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error"); + } + catch (Exception e) + { + assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } + + + @Test + public void testBatchGuardrail() + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + String tableName = currentTable(); + String batchWithLiterals = String.format("BEGIN BATCH " + "INSERT INTO %s.%s (id, val) VALUES (1, 'v1'); " + "UPDATE %s.%s SET val = 'v2' WHERE id = 2; " + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName); + try + { + QueryProcessor.instance.prepare(batchWithLiterals, state); + Assert.fail("Expected guardrail error for BATCH with literals"); + } + catch (Exception e) + { + assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } + + @Test + public void testInWhereClause() throws Throwable + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + String fullTableName = KEYSPACE + '.' + currentTable(); + String query = String.format("SELECT * FROM %s WHERE id IN (1, 2, 3)", fullTableName); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error for regular user with IN clause literals"); + } + catch (Exception e) + { + assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } + + + @Test + public void testInternalBypass() + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + ClientState internalState = ClientState.forInternalCalls(); + QueryProcessor.instance.prepare("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", internalState); + } + + @Test + public void testSuperUserBypass() + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + AuthenticatedUser superUser = new AuthenticatedUser("super-user") + { + + @Override + public boolean isSuper() + { + return true; + } + + @Override + public boolean isSystem() + { + return false; + } + + @Override + public boolean isAnonymous() + { + return false; + } + + @Override + public boolean canLogin() + { + return true; + } + }; + + ClientState superUserState = ClientState.forExternalCalls(new InetSocketAddress("127.0.0.1", 9042)); + superUserState.login(superUser); + QueryProcessor.instance.prepare("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", superUserState); + } + + @Test + public void testSelectAllPasses() + { + createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + String query = String.format("SELECT * FROM %s", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + } + catch (Exception e) + { + Assert.fail("Did not expect guardrail error for SELECT * statement"); + } + } + + @Test + public void testGuardrailDisabledAllowsLiterals() + { + DatabaseDescriptor.setUseMispreparedStatementsEnabled(true); + createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + String query = String.format("SELECT * FROM %s WHERE id = 1", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + } + catch (Exception e) + { + Assert.fail("Guardrail should NOT have triggered because it is disabled (set to true)"); + } + } + + @Test + public void testGuardrailDisabledAllowsBatchLiterals() + { + DatabaseDescriptor.setUseMispreparedStatementsEnabled(true); + createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + String tableName = currentTable(); + String batchWithLiterals = String.format("BEGIN BATCH " + + "INSERT INTO %s.%s (id, val) VALUES (1, 'v1'); " + + "UPDATE %s.%s SET val = 'v2' WHERE id = 2; " + + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName); + try + { + QueryProcessor.instance.prepare(batchWithLiterals, state); + } + catch (Exception e) + { + Assert.fail("Batch with literals should be allowed when guardrail is disabled"); + } + } +} + From ae61b88dabb472a5b33d5ad64af3fb9d8becd3e5 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Sat, 31 Jan 2026 01:37:37 +0530 Subject: [PATCH 03/19] added warning to client --- .../apache/cassandra/cql3/QueryProcessor.java | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/cql3/QueryProcessor.java b/src/java/org/apache/cassandra/cql3/QueryProcessor.java index 5f0cf7469c49..f8f99c09d713 100644 --- a/src/java/org/apache/cassandra/cql3/QueryProcessor.java +++ b/src/java/org/apache/cassandra/cql3/QueryProcessor.java @@ -90,6 +90,7 @@ import org.apache.cassandra.schema.SchemaConstants; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.ClientWarn; import org.apache.cassandra.service.QueryState; import org.apache.cassandra.service.StorageService; import org.apache.cassandra.service.pager.QueryPager; @@ -103,6 +104,7 @@ import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.JVMStabilityInspector; import org.apache.cassandra.utils.MD5Digest; +import org.apache.cassandra.utils.NoSpamLogger; import org.apache.cassandra.utils.ObjectSizes; import org.apache.cassandra.utils.concurrent.Future; import org.apache.cassandra.utils.concurrent.FutureCombiner; @@ -529,7 +531,23 @@ private static void checkMispreparedGuardrail(CQLStatement statement, ClientStat if (isMisprepared(statement)) { - Guardrails.MispreparedStatementsEnabled.ensureEnabled(clientState); + if (Guardrails.MispreparedStatementsEnabled.enabled(clientState)) + { + Guardrails.MispreparedStatementsEnabled.ensureEnabled(clientState); + } + else + { + String msg = "Performance Tip: This query contains literal values in the WHERE clause. " + + "Using '?' placeholders (bind markers) is much more efficient. " + + "It allows the server to cache this query once and reuse it for different values, " + + "reducing memory usage and improving speed."; + + ClientWarn.instance.warn(msg); + + NoSpamLogger.log(logger, NoSpamLogger.Level.WARN, 1, TimeUnit.MINUTES, + "Client {} prepared a non-reusable query: {}", + clientState.getRemoteAddress(), statement); + } } } From c83bf22051491dbd1b8808cb998522806487078c Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Sat, 31 Jan 2026 01:42:53 +0530 Subject: [PATCH 04/19] cleanup --- conf/cassandra.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 019841551ef9..6a2cf1fd6a7f 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -197,6 +197,9 @@ batchlog_replay_throttle: 1024KiB # If using PasswordAuthenticator, CassandraRoleManager must also be used (see below) authenticator: class_name: AllowAllAuthenticator +# MutualTlsAuthenticator can be configured using the following configuration. One can add their own validator +# which implements MutualTlsCertificateValidator class and provide logic for extracting identity out of certificates +# and validating certificates. # class_name: org.apache.cassandra.auth.MutualTlsAuthenticator # parameters: # validator_class_name: org.apache.cassandra.auth.SpiffeCertificateValidator From ba80e42777e815375aa18db8101d19aa1fd84dc3 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Sat, 31 Jan 2026 02:27:47 +0530 Subject: [PATCH 05/19] additional cleanup --- conf/cassandra.yaml | 2 +- src/java/org/apache/cassandra/cql3/QueryProcessor.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 6a2cf1fd6a7f..82e0fe8fff8a 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -2533,7 +2533,7 @@ drop_compact_storage_enabled: false # maximum_replication_factor_fail_threshold: -1 # # When true, allows the use of misprepared statements, only warns in the logs. -# When false, misprepared statements will result in an error to the client. +# When false, misprepared statements will result in an error to the client. Default is true. # use_misprepare_statements_enabled: false #role_name_policy: diff --git a/src/java/org/apache/cassandra/cql3/QueryProcessor.java b/src/java/org/apache/cassandra/cql3/QueryProcessor.java index f8f99c09d713..b3484ced9087 100644 --- a/src/java/org/apache/cassandra/cql3/QueryProcessor.java +++ b/src/java/org/apache/cassandra/cql3/QueryProcessor.java @@ -524,7 +524,7 @@ public static Prepared parseAndPrepare(String query, ClientState clientState, bo private static void checkMispreparedGuardrail(CQLStatement statement, ClientState clientState) { - if (clientState.isInternal || !Guardrails.MispreparedStatementsEnabled.enabled(clientState)) + if (clientState.isInternal) { return; } From 4caf9adc70e61bdbf921b182adc9a9dc985d37d1 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Sat, 31 Jan 2026 02:33:49 +0530 Subject: [PATCH 06/19] fixed codestyle files --- .idea/codeStyles/Project.xml | 315 +++++++++++++++++++++++++++ .idea/codeStyles/codeStyleConfig.xml | 5 + 2 files changed, 320 insertions(+) create mode 100644 .idea/codeStyles/Project.xml create mode 100644 .idea/codeStyles/codeStyleConfig.xml diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml new file mode 100644 index 000000000000..afcd5ec49778 --- /dev/null +++ b/.idea/codeStyles/Project.xml @@ -0,0 +1,315 @@ + + + + \ No newline at end of file diff --git a/.idea/codeStyles/codeStyleConfig.xml b/.idea/codeStyles/codeStyleConfig.xml new file mode 100644 index 000000000000..0f7bc519db61 --- /dev/null +++ b/.idea/codeStyles/codeStyleConfig.xml @@ -0,0 +1,5 @@ + + + + From e282987799e53cc741b7abd2e0b83c4cb6f89bd5 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Sat, 31 Jan 2026 11:54:13 +0530 Subject: [PATCH 07/19] optimized test case --- .../cassandra/cql3/MispreparedStatementsTest.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java index 6a104abed98a..7a16335934a0 100644 --- a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -79,6 +79,7 @@ public boolean canLogin() state.login(nonSuperUser); state.setKeyspace(KEYSPACE); + createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); } @After @@ -90,7 +91,6 @@ public void tearDown() @Test public void testSelectGuardrail() { - createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); String query = String.format("SELECT * FROM %s WHERE id = 1", currentTable()); try { @@ -107,7 +107,6 @@ public void testSelectGuardrail() @Test public void testModificationGuardrail() { - createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); String fullName = KEYSPACE + '.' + currentTable(); String query = String.format("UPDATE %s SET val = 'new_name' WHERE id = 1", fullName); try @@ -125,7 +124,6 @@ public void testModificationGuardrail() @Test public void testBatchGuardrail() { - createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); String tableName = currentTable(); String batchWithLiterals = String.format("BEGIN BATCH " + "INSERT INTO %s.%s (id, val) VALUES (1, 'v1'); " + "UPDATE %s.%s SET val = 'v2' WHERE id = 2; " + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName); try @@ -142,7 +140,6 @@ public void testBatchGuardrail() @Test public void testInWhereClause() throws Throwable { - createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); String fullTableName = KEYSPACE + '.' + currentTable(); String query = String.format("SELECT * FROM %s WHERE id IN (1, 2, 3)", fullTableName); try @@ -160,7 +157,6 @@ public void testInWhereClause() throws Throwable @Test public void testInternalBypass() { - createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); ClientState internalState = ClientState.forInternalCalls(); QueryProcessor.instance.prepare("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", internalState); } @@ -168,7 +164,6 @@ public void testInternalBypass() @Test public void testSuperUserBypass() { - createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); AuthenticatedUser superUser = new AuthenticatedUser("super-user") { @@ -205,7 +200,6 @@ public boolean canLogin() @Test public void testSelectAllPasses() { - createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); String query = String.format("SELECT * FROM %s", currentTable()); try { @@ -221,7 +215,6 @@ public void testSelectAllPasses() public void testGuardrailDisabledAllowsLiterals() { DatabaseDescriptor.setUseMispreparedStatementsEnabled(true); - createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); String query = String.format("SELECT * FROM %s WHERE id = 1", currentTable()); try { @@ -237,7 +230,6 @@ public void testGuardrailDisabledAllowsLiterals() public void testGuardrailDisabledAllowsBatchLiterals() { DatabaseDescriptor.setUseMispreparedStatementsEnabled(true); - createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); String tableName = currentTable(); String batchWithLiterals = String.format("BEGIN BATCH " + "INSERT INTO %s.%s (id, val) VALUES (1, 'v1'); " + From 58250945a5e3d668008669266392062ebb0ccb75 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Sat, 31 Jan 2026 12:40:09 +0530 Subject: [PATCH 08/19] Expand guardrail coverage for mis-prepared CQL statements across full CRUD, IN, and LWT operations --- .../cql3/MispreparedStatementsTest.java | 149 +++++++++++++++++- 1 file changed, 142 insertions(+), 7 deletions(-) diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java index 7a16335934a0..5f45610959f7 100644 --- a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -79,7 +79,7 @@ public boolean canLogin() state.login(nonSuperUser); state.setKeyspace(KEYSPACE); - createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + createTable("CREATE TABLE %s (id int, description text, name text, PRIMARY KEY (id, name))"); } @After @@ -89,7 +89,7 @@ public void tearDown() } @Test - public void testSelectGuardrail() + public void testSelectWithPartitionKey() { String query = String.format("SELECT * FROM %s WHERE id = 1", currentTable()); try @@ -103,12 +103,73 @@ public void testSelectGuardrail() } } + @Test + public void testSelectWithClusteringKey() + { + String query = String.format("SELECT * FROM %s WHERE name = 'v1' ALLOW FILTERING", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error for mis-prepared SELECT statement"); + } + catch (Exception e) + { + assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } + + @Test + public void testInsertJsonGuardrail() + { + String query = String.format("INSERT INTO %s JSON '{\"id\": 1, \"name\": \"v1\"}'", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error for INSERT JSON literal"); + } + catch (Exception e) + { + assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } + + @Test + public void testUpdateWithIfConditionLiteralShouldFail() + { + String query = String.format("UPDATE %s SET description = 'v2' WHERE id = 1 AND name = 'v1' IF description = 'v0'", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error because ZERO bind markers are present"); + } + catch (Exception e) + { + assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } + + + @Test + public void testSelectWithFullPrimaryKey() + { + String query = String.format("SELECT * FROM %s WHERE id = 1 AND name = 'v1'", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error for mis-prepared SELECT statement"); + } + catch (Exception e) + { + assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); + } + + } @Test public void testModificationGuardrail() { String fullName = KEYSPACE + '.' + currentTable(); - String query = String.format("UPDATE %s SET val = 'new_name' WHERE id = 1", fullName); + String query = String.format("UPDATE %s SET description = 'new_description' WHERE id = 1 AND name = 'name'", fullName); try { QueryProcessor.instance.prepare(query, state); @@ -120,12 +181,55 @@ public void testModificationGuardrail() } } + @Test + public void testDeleteWithFullPrimaryKey() + { + String query = String.format("DELETE FROM %s WHERE id = 1 AND name = 'v1'", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error for DELETE with literals"); + } + catch (Exception e) + { + assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } + + @Test + public void testSelectWithRangeRestriction() + { + String query = String.format("SELECT * FROM %s WHERE id = 1 AND name > 'a'", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error for range query with literals"); + } + catch (Exception e) + { + assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } + + @Test + public void testProperlyPreparedWithBindMarkers() + { + String query = String.format("SELECT * FROM %s WHERE id = ? AND name = ?", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + } + catch (Exception e) + { + Assert.fail("Guardrail should NOT trigger for statements using bind markers (?)"); + } + } @Test public void testBatchGuardrail() { String tableName = currentTable(); - String batchWithLiterals = String.format("BEGIN BATCH " + "INSERT INTO %s.%s (id, val) VALUES (1, 'v1'); " + "UPDATE %s.%s SET val = 'v2' WHERE id = 2; " + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName); + String batchWithLiterals = String.format("BEGIN BATCH " + "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1'; " + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName); try { QueryProcessor.instance.prepare(batchWithLiterals, state); @@ -138,7 +242,7 @@ public void testBatchGuardrail() } @Test - public void testInWhereClause() throws Throwable + public void testSelectInRestrictionOnPartitionKey() throws Throwable { String fullTableName = KEYSPACE + '.' + currentTable(); String query = String.format("SELECT * FROM %s WHERE id IN (1, 2, 3)", fullTableName); @@ -153,6 +257,37 @@ public void testInWhereClause() throws Throwable } } + @Test + public void testSelectInRestrictionOnClusteringKey() + { + String fullTableName = KEYSPACE + '.' + currentTable(); + String query = String.format("SELECT * FROM %s WHERE name IN ('a', 'b') ALLOW FILTERING", fullTableName); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error for regular user with IN clause literals"); + } + catch (Exception e) + { + assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } + + @Test + public void testSelectInRestrictionOnFullPrimaryKey() + { + String fullTableName = KEYSPACE + '.' + currentTable(); + String query = String.format("SELECT * FROM %s WHERE id IN (1, 2, 3) AND name in ('a', 'b')", fullTableName); + try + { + QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected guardrail error for regular user with IN clause literals"); + } + catch (Exception e) + { + assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); + } + } @Test public void testInternalBypass() @@ -232,8 +367,8 @@ public void testGuardrailDisabledAllowsBatchLiterals() DatabaseDescriptor.setUseMispreparedStatementsEnabled(true); String tableName = currentTable(); String batchWithLiterals = String.format("BEGIN BATCH " + - "INSERT INTO %s.%s (id, val) VALUES (1, 'v1'); " + - "UPDATE %s.%s SET val = 'v2' WHERE id = 2; " + + "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + + "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1'; " + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName); try { From 50ec4994e3b6395b275671e84761f30e00d0a0ac Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Sat, 31 Jan 2026 14:57:50 +0530 Subject: [PATCH 09/19] optimized queryprocessor and clubbed getter and setters --- conf/cassandra.yaml | 2 +- .../org/apache/cassandra/config/Config.java | 3 +- .../cassandra/config/DatabaseDescriptor.java | 19 +++++------ .../cassandra/config/GuardrailsOptions.java | 11 ++++++- .../apache/cassandra/cql3/QueryProcessor.java | 33 ++++++++++--------- .../cassandra/db/guardrails/Guardrails.java | 8 +++-- .../db/guardrails/GuardrailsConfig.java | 32 ++++++++++++++++-- .../cassandra/service/StorageService.java | 10 +++--- .../service/StorageServiceMBean.java | 4 +-- .../config/DatabaseDescriptorTest.java | 12 +++---- .../cql3/MispreparedStatementsTest.java | 8 ++--- .../GuardrailsConfigCommandsTest.java | 1 + 12 files changed, 92 insertions(+), 51 deletions(-) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 82e0fe8fff8a..4794b350b943 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -2534,7 +2534,7 @@ drop_compact_storage_enabled: false # # When true, allows the use of misprepared statements, only warns in the logs. # When false, misprepared statements will result in an error to the client. Default is true. -# use_misprepare_statements_enabled: false +# misprepare_statements_enabled: false #role_name_policy: # # Implementation class of a validator. When not in form of FQCN, the diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 553b078ec2ea..2383755309c1 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -665,8 +665,6 @@ public static class SSTableConfig public volatile boolean use_statements_enabled = true; - public volatile boolean use_misprepare_statements_enabled = false; - /** * Optionally disable asynchronous UDF execution. * Disabling asynchronous UDF execution also implicitly disables the security-manager! @@ -939,6 +937,7 @@ public static void setClientMode(boolean clientMode) public volatile boolean user_timestamps_enabled = true; public volatile boolean alter_table_enabled = true; public volatile boolean group_by_enabled = true; + public volatile boolean misprepared_statements_enabled = false; public volatile boolean bulk_load_enabled = true; public volatile boolean drop_truncate_table_enabled = true; public volatile boolean drop_keyspace_enabled = true; diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index c24c632549f3..37f0c05ed987 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -5474,11 +5474,6 @@ public static boolean getUseStatementsEnabled() return conf.use_statements_enabled; } - public static boolean getUseMispreparedStatementsEnabled() - { - return conf.use_misprepare_statements_enabled; - } - public static void setUseStatementsEnabled(boolean enabled) { if (enabled != conf.use_statements_enabled) @@ -5488,16 +5483,20 @@ public static void setUseStatementsEnabled(boolean enabled) } } - public static void setUseMispreparedStatementsEnabled(boolean enabled) + public static boolean getMispreparedStatementsEnabled() { - if (enabled != conf.use_misprepare_statements_enabled) + return conf.misprepared_statements_enabled; + } + + public static void setMispreparedStatementsEnabled(boolean enabled) + { + if (enabled != conf.misprepared_statements_enabled) { - logger.info("Setting use_misprepare_statements_enabled to {}", enabled); - conf.use_misprepare_statements_enabled = enabled; + logger.info("Setting misprepare_statements_enabled to {}", enabled); + conf.misprepared_statements_enabled = enabled; } } - public static AccordSpec getAccord() { return conf.accord; diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index 67561c7fb499..e209019169ba 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -1332,7 +1332,16 @@ public boolean getVectorTypeEnabled() @Override public boolean getMispreparedStatementsEnabled() { - return config.use_misprepare_statements_enabled; + return config.misprepared_statements_enabled; + } + + + public void setMispreparedStatementsEnabled(boolean enabled) + { + updatePropertyWithLogging("use_misprepare_statements_enabled", + enabled, + () -> config.misprepared_statements_enabled, + x -> config.misprepared_statements_enabled = x); } @Override diff --git a/src/java/org/apache/cassandra/cql3/QueryProcessor.java b/src/java/org/apache/cassandra/cql3/QueryProcessor.java index b3484ced9087..36e0bb9445b0 100644 --- a/src/java/org/apache/cassandra/cql3/QueryProcessor.java +++ b/src/java/org/apache/cassandra/cql3/QueryProcessor.java @@ -43,6 +43,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.cassandra.auth.AuthenticatedUser; import org.apache.cassandra.concurrent.ImmediateExecutor; import org.apache.cassandra.concurrent.ScheduledExecutors; import org.apache.cassandra.config.DatabaseDescriptor; @@ -134,6 +135,12 @@ public class QueryProcessor implements QueryHandler // counters. Callers of processStatement are responsible for correctly notifying metrics public static final CQLMetrics metrics = new CQLMetrics(); + + private static final String msg = "Performance Tip: This query contains literal values in the WHERE clause. " + + "Using '?' placeholders (bind markers) is much more efficient. " + + "It allows the server to cache this query once and reuse it for different values, " + + "reducing memory usage and improving speed."; + // Paging size to use when preloading prepared statements. public static final int PRELOAD_PREPARED_STATEMENTS_FETCH_SIZE = 5000; @@ -499,7 +506,7 @@ public static Prepared parseAndPrepare(String query, ClientState clientState, bo CQLStatement statement = raw.prepare(clientState); statement.validate(clientState); - if (!isInternal) + if (!isInternal && !clientState.isInternal) { checkMispreparedGuardrail(statement, clientState); } @@ -524,26 +531,20 @@ public static Prepared parseAndPrepare(String query, ClientState clientState, bo private static void checkMispreparedGuardrail(CQLStatement statement, ClientState clientState) { - if (clientState.isInternal) - { + AuthenticatedUser user = clientState.getUser(); + if (user != null && user.isSuper()) return; - } - if (isMisprepared(statement)) + if (Guardrails.mispreparedStatementsEnabled.enabled(clientState)) { - if (Guardrails.MispreparedStatementsEnabled.enabled(clientState)) - { - Guardrails.MispreparedStatementsEnabled.ensureEnabled(clientState); - } - else + if (isMisprepared(statement)) + Guardrails.mispreparedStatementsEnabled.ensureEnabled(clientState); + } + else + { + if (isMisprepared(statement)) { - String msg = "Performance Tip: This query contains literal values in the WHERE clause. " + - "Using '?' placeholders (bind markers) is much more efficient. " + - "It allows the server to cache this query once and reuse it for different values, " + - "reducing memory usage and improving speed."; - ClientWarn.instance.warn(msg); - NoSpamLogger.log(logger, NoSpamLogger.Level.WARN, 1, TimeUnit.MINUTES, "Client {} prepared a non-reusable query: {}", clientState.getRemoteAddress(), statement); diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 73463a5782ff..5771c628c848 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -614,9 +614,13 @@ public final class Guardrails implements GuardrailsMBean what, value, isWarning ? "warning" : "failure", threshold)); /** - * Guardrail on mis-prepared statements. + * Prevents heap exhaustion caused by the anti-pattern of preparing queries with + * hardcoded literals instead of bind markers. This prevents filling the statement cache with non-reusable entries. + * + * @see CASSANDRA-21139 */ - public static final EnableFlag MispreparedStatementsEnabled = + + public static final EnableFlag mispreparedStatementsEnabled = new EnableFlag("misprepared_statements_enabled", "Mis-prepared statements cause server-side memory exhaustion and high GC pressure by flooding the statement cache with non-reusable query entries", state -> CONFIG_PROVIDER.getOrCreate(state).getMispreparedStatementsEnabled(), diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index 1040c914a44b..e91442959f64 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -407,11 +407,39 @@ public interface GuardrailsConfig boolean getVectorTypeEnabled(); /** - * @return Whether use misprepared statements is enabled. If not enabled, misprepared statements will fail - * otherwise warned. + *

+ * A statement is considered "mis-prepared" if it contains hardcoded literal values + * instead of bind markers. This is a performance anti-pattern because it prevents + * query plan reuse and floods the server-side Prepared Statement Cache with + * unique entries, leading to heap exhaustion and high GC pressure. + *

+ * LWT and Batch Considerations: + * This check applies to both the {@code WHERE} clause and the {@code IF} conditions + * in Lightweight Transactions (LWT). For example, the following is considered + * mis-prepared because of the hardcoded literals: + * 

{@code
+     * UPDATE users SET description = 'v2' WHERE id = 1 AND name = 'v1' IF description = 'v0'
+     * }
+ * To be compliant, it should use bind markers ({@code ?}): + * 
{@code
+     * UPDATE users SET description = ? WHERE id = ? AND name = ? IF description = ?
+     * }
+ * For {@code BATCH} statements, if any individual statement within the batch is + * identified as mis-prepared, the entire batch triggers this guardrail. + + * + * @return true if the usage of mis-prepared statements is enabled, false otherwise. Returns true by default. + * {@code false} if mis-prepared statements should be strictly rejected, causing the query to fail. + * @see CASSANDRA-21139 */ + boolean getMispreparedStatementsEnabled(); + /** + * sets whether misprepared statements is enabled + */ + void setMispreparedStatementsEnabled(boolean enabled); + /** * Sets whether new columns can be created with vector type * diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index abc320d46162..d53836f626e6 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -4837,16 +4837,16 @@ public void setColumnIndexCacheSize(int cacheSizeInKB) } @Override - public boolean getUseMispreparedStatementsEnabled() + public boolean getMispreparedStatementsEnabled() { - return DatabaseDescriptor.getUseMispreparedStatementsEnabled(); + return DatabaseDescriptor.getMispreparedStatementsEnabled(); } @Override - public void setUseMispreparedStatementsEnabled(boolean enabled) + public void setMispreparedStatementsEnabled(boolean enabled) { - DatabaseDescriptor.setUseMispreparedStatementsEnabled(enabled); - logger.info("Updated use_misprepared_statements_enabled to {}", enabled); + DatabaseDescriptor.setMispreparedStatementsEnabled(enabled); + logger.info("Updated misprepared_statements_enabled to {}", enabled); } /* diff --git a/src/java/org/apache/cassandra/service/StorageServiceMBean.java b/src/java/org/apache/cassandra/service/StorageServiceMBean.java index f6136a288c5a..09db793ad840 100644 --- a/src/java/org/apache/cassandra/service/StorageServiceMBean.java +++ b/src/java/org/apache/cassandra/service/StorageServiceMBean.java @@ -1100,12 +1100,12 @@ default int upgradeSSTables(String keyspaceName, boolean excludeCurrentVersion, /** * Returns whether use of misprepared statements is enabled */ - public boolean getUseMispreparedStatementsEnabled(); + public boolean getMispreparedStatementsEnabled(); /** * Sets whether use of misprepared statements is enabled */ - public void setUseMispreparedStatementsEnabled(boolean enabled); + public void setMispreparedStatementsEnabled(boolean enabled); /** Returns the threshold for skipping the column index when caching partition info **/ public int getColumnIndexCacheSizeInKiB(); diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java index 36230b4a944f..c640f09dc476 100644 --- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java +++ b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java @@ -783,20 +783,20 @@ public void testRowIndexSizeWarnEqAbort() @Test public void testMispreparedStatementsEnabled() { - boolean originalValue = DatabaseDescriptor.getUseMispreparedStatementsEnabled(); + boolean originalValue = DatabaseDescriptor.getMispreparedStatementsEnabled(); try { - DatabaseDescriptor.setUseMispreparedStatementsEnabled(true); + DatabaseDescriptor.setMispreparedStatementsEnabled(true); Assert.assertTrue("Value should be true after setting to true", - DatabaseDescriptor.getUseMispreparedStatementsEnabled()); + DatabaseDescriptor.getMispreparedStatementsEnabled()); - DatabaseDescriptor.setUseMispreparedStatementsEnabled(false); + DatabaseDescriptor.setMispreparedStatementsEnabled(false); Assert.assertFalse("Value should be false after setting to false", - DatabaseDescriptor.getUseMispreparedStatementsEnabled()); + DatabaseDescriptor.getMispreparedStatementsEnabled()); } finally { - DatabaseDescriptor.setUseMispreparedStatementsEnabled(originalValue); + DatabaseDescriptor.setMispreparedStatementsEnabled(originalValue); } } diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java index 5f45610959f7..d185859e2ed0 100644 --- a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -44,7 +44,7 @@ public static void setupGlobalConfig() { DatabaseDescriptor.setAuthenticator(new PasswordAuthenticator()); DatabaseDescriptor.setAuthorizer(new CassandraAuthorizer()); - DatabaseDescriptor.setUseMispreparedStatementsEnabled(false); + DatabaseDescriptor.setMispreparedStatementsEnabled(false); } @Before @@ -85,7 +85,7 @@ public boolean canLogin() @After public void tearDown() { - DatabaseDescriptor.setUseMispreparedStatementsEnabled(false); + DatabaseDescriptor.setMispreparedStatementsEnabled(false); } @Test @@ -349,7 +349,7 @@ public void testSelectAllPasses() @Test public void testGuardrailDisabledAllowsLiterals() { - DatabaseDescriptor.setUseMispreparedStatementsEnabled(true); + DatabaseDescriptor.setMispreparedStatementsEnabled(true); String query = String.format("SELECT * FROM %s WHERE id = 1", currentTable()); try { @@ -364,7 +364,7 @@ public void testGuardrailDisabledAllowsLiterals() @Test public void testGuardrailDisabledAllowsBatchLiterals() { - DatabaseDescriptor.setUseMispreparedStatementsEnabled(true); + DatabaseDescriptor.setMispreparedStatementsEnabled(true); String tableName = currentTable(); String batchWithLiterals = String.format("BEGIN BATCH " + "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + diff --git a/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java b/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java index 37958178b7b0..21a46c41e7a4 100644 --- a/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java +++ b/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java @@ -209,6 +209,7 @@ private Set getConfigFieldNames() "intersect_filtering_query_enabled true\n" + "intersect_filtering_query_warned true\n" + "non_partition_restricted_index_query_enabled true\n" + + "misprepared_statements_enabled false\n" + "read_before_write_list_operations_enabled true\n" + "secondary_indexes_enabled true\n" + "simplestrategy_enabled true\n" + From 564a99d37ac90d938fe1750e2ed4282133860403 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Mon, 2 Feb 2026 12:44:16 +0530 Subject: [PATCH 10/19] added test case for literals in projections and bypassing guardrail for system keyspaces --- .../cql3/MispreparedStatementsTest.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java index d185859e2ed0..a09c77a6e829 100644 --- a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -379,5 +379,34 @@ public void testGuardrailDisabledAllowsBatchLiterals() Assert.fail("Batch with literals should be allowed when guardrail is disabled"); } } + + @Test + public void testSystemKeyspaceBypassForRegularUser() + { + String query = "SELECT * FROM system.local WHERE key = 'local'"; + try + { + QueryProcessor.instance.prepare(query, state); + } + catch (Exception e) + { + Assert.fail("Guardrail should NOT trigger for system keyspace queries"); + } + } + + @Test + public void testLiteralInProjectionIsAllowed() + { + String query = String.format("SELECT id, (text)'const_val' FROM %s WHERE id = ?", currentTable()); + try + { + QueryProcessor.instance.prepare(query, state); + } + catch (Exception e) + { + e.printStackTrace(); + Assert.fail("Guardrail should only trigger for literals in RESTRICTIONS (WHERE clause)"); + } + } } From e2071d18656b40bf57c480564298a28403cd9ede Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Mon, 2 Feb 2026 14:48:53 +0530 Subject: [PATCH 11/19] optimized query processing and new test cases --- .python-version | 1 + conf/cassandra.yaml | 4 +- .../org/apache/cassandra/config/Config.java | 2 +- .../cassandra/config/DatabaseDescriptor.java | 6 +- .../cassandra/config/GuardrailsOptions.java | 2 +- .../apache/cassandra/cql3/QueryProcessor.java | 66 ---- .../statements/ModificationStatement.java | 14 + .../cql3/statements/SelectStatement.java | 13 +- .../cassandra/db/guardrails/Guardrails.java | 13 + .../cql3/MispreparedStatementsTest.java | 292 ++++++------------ 10 files changed, 145 insertions(+), 268 deletions(-) create mode 100644 .python-version diff --git a/.python-version b/.python-version new file mode 100644 index 000000000000..2419ad5b0a32 --- /dev/null +++ b/.python-version @@ -0,0 +1 @@ +3.11.9 diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index 4794b350b943..b2851d9021c3 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -2532,9 +2532,9 @@ drop_compact_storage_enabled: false # maximum_replication_factor_warn_threshold: -1 # maximum_replication_factor_fail_threshold: -1 # -# When true, allows the use of misprepared statements, only warns in the logs. +# When true, allows the usage of misprepared statements, only warns in the logs. # When false, misprepared statements will result in an error to the client. Default is true. -# misprepare_statements_enabled: false +# misprepared_statements_enabled: true #role_name_policy: # # Implementation class of a validator. When not in form of FQCN, the diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 2383755309c1..4ef0208a93d2 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -937,7 +937,7 @@ public static void setClientMode(boolean clientMode) public volatile boolean user_timestamps_enabled = true; public volatile boolean alter_table_enabled = true; public volatile boolean group_by_enabled = true; - public volatile boolean misprepared_statements_enabled = false; + public volatile boolean misprepared_statements_enabled = true; public volatile boolean bulk_load_enabled = true; public volatile boolean drop_truncate_table_enabled = true; public volatile boolean drop_keyspace_enabled = true; diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index 37f0c05ed987..5b01d7163760 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -5490,11 +5490,7 @@ public static boolean getMispreparedStatementsEnabled() public static void setMispreparedStatementsEnabled(boolean enabled) { - if (enabled != conf.misprepared_statements_enabled) - { - logger.info("Setting misprepare_statements_enabled to {}", enabled); - conf.misprepared_statements_enabled = enabled; - } + conf.misprepared_statements_enabled = enabled; } public static AccordSpec getAccord() diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index e209019169ba..bf9c9348052e 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -1338,7 +1338,7 @@ public boolean getMispreparedStatementsEnabled() public void setMispreparedStatementsEnabled(boolean enabled) { - updatePropertyWithLogging("use_misprepare_statements_enabled", + updatePropertyWithLogging("misprepare_statements_enabled", enabled, () -> config.misprepared_statements_enabled, x -> config.misprepared_statements_enabled = x); diff --git a/src/java/org/apache/cassandra/cql3/QueryProcessor.java b/src/java/org/apache/cassandra/cql3/QueryProcessor.java index 36e0bb9445b0..1af40d0d4878 100644 --- a/src/java/org/apache/cassandra/cql3/QueryProcessor.java +++ b/src/java/org/apache/cassandra/cql3/QueryProcessor.java @@ -43,7 +43,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.cassandra.auth.AuthenticatedUser; import org.apache.cassandra.concurrent.ImmediateExecutor; import org.apache.cassandra.concurrent.ScheduledExecutors; import org.apache.cassandra.config.DatabaseDescriptor; @@ -51,7 +50,6 @@ import org.apache.cassandra.cql3.functions.FunctionName; import org.apache.cassandra.cql3.functions.UDAggregate; import org.apache.cassandra.cql3.functions.UDFunction; -import org.apache.cassandra.cql3.restrictions.StatementRestrictions; import org.apache.cassandra.cql3.selection.ResultSetBuilder; import org.apache.cassandra.cql3.statements.BatchStatement; import org.apache.cassandra.cql3.statements.ModificationStatement; @@ -66,7 +64,6 @@ import org.apache.cassandra.db.ReadResponse; import org.apache.cassandra.db.SinglePartitionReadQuery; import org.apache.cassandra.db.SystemKeyspace; -import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.partitions.PartitionIterator; import org.apache.cassandra.db.partitions.PartitionIterators; @@ -91,7 +88,6 @@ import org.apache.cassandra.schema.SchemaConstants; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.service.ClientState; -import org.apache.cassandra.service.ClientWarn; import org.apache.cassandra.service.QueryState; import org.apache.cassandra.service.StorageService; import org.apache.cassandra.service.pager.QueryPager; @@ -105,7 +101,6 @@ import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.JVMStabilityInspector; import org.apache.cassandra.utils.MD5Digest; -import org.apache.cassandra.utils.NoSpamLogger; import org.apache.cassandra.utils.ObjectSizes; import org.apache.cassandra.utils.concurrent.Future; import org.apache.cassandra.utils.concurrent.FutureCombiner; @@ -136,11 +131,6 @@ public class QueryProcessor implements QueryHandler public static final CQLMetrics metrics = new CQLMetrics(); - private static final String msg = "Performance Tip: This query contains literal values in the WHERE clause. " + - "Using '?' placeholders (bind markers) is much more efficient. " + - "It allows the server to cache this query once and reuse it for different values, " + - "reducing memory usage and improving speed."; - // Paging size to use when preloading prepared statements. public static final int PRELOAD_PREPARED_STATEMENTS_FETCH_SIZE = 5000; @@ -505,12 +495,6 @@ public static Prepared parseAndPrepare(String query, ClientState clientState, bo // Note: if 2 threads prepare the same query, we'll live so don't bother synchronizing CQLStatement statement = raw.prepare(clientState); statement.validate(clientState); - - if (!isInternal && !clientState.isInternal) - { - checkMispreparedGuardrail(statement, clientState); - } - // Set CQL string for AlterSchemaStatement as this is used to serialize the transformation // in the cluster metadata log if (statement instanceof AlterSchemaStatement) @@ -529,56 +513,6 @@ public static Prepared parseAndPrepare(String query, ClientState clientState, bo return res; } - private static void checkMispreparedGuardrail(CQLStatement statement, ClientState clientState) - { - AuthenticatedUser user = clientState.getUser(); - if (user != null && user.isSuper()) - return; - - if (Guardrails.mispreparedStatementsEnabled.enabled(clientState)) - { - if (isMisprepared(statement)) - Guardrails.mispreparedStatementsEnabled.ensureEnabled(clientState); - } - else - { - if (isMisprepared(statement)) - { - ClientWarn.instance.warn(msg); - NoSpamLogger.log(logger, NoSpamLogger.Level.WARN, 1, TimeUnit.MINUTES, - "Client {} prepared a non-reusable query: {}", - clientState.getRemoteAddress(), statement); - } - } - } - - private static boolean isMisprepared(CQLStatement statement) - { - if (!statement.getBindVariables().isEmpty()) - return false; - - if (statement instanceof BatchStatement) - { - for (ModificationStatement subStmt : ((BatchStatement) statement).getStatements()) - { - if (isMisprepared(subStmt)) - return true; - } - return false; - } - - StatementRestrictions restrictions = null; - if (statement instanceof SelectStatement) - restrictions = ((SelectStatement) statement).getRestrictions(); - else if (statement instanceof ModificationStatement) - restrictions = ((ModificationStatement) statement).getRestrictions(); - - return (restrictions != null && - (restrictions.hasPartitionKeyRestrictions() - || restrictions.hasClusteringColumnsRestrictions() - || restrictions.hasNonPrimaryKeyRestrictions())); - } - public static UntypedResultSet executeInternal(String query, Object... values) { Prepared prepared = prepareInternal(query); diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java index 118f2c1fa44f..c08e861e8ee0 100644 --- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java @@ -409,7 +409,21 @@ public void validate(ClientState state) throws InvalidRequestException checkFalse(isVirtual() && hasConditions(), "Conditional updates are not supported by virtual tables"); if (attrs.isTimestampSet()) + { Guardrails.userTimestampsEnabled.ensureEnabled(state); + } + // Misprepare guardrail shouldn't block system keyspaces + if (SchemaConstants.isSystemKeyspace(metadata.keyspace)) + return; + + boolean hasRestriction = restrictions != null && + restrictions.hasPartitionKeyRestrictions() || + restrictions.hasClusteringColumnsRestrictions() || + restrictions.hasNonPrimaryKeyRestrictions(); + if (getBindVariables().isEmpty() && hasRestriction) + { + Guardrails.onMisprepared(state); + } } public void validateDiskUsage(QueryOptions options, ClientState state) diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 1752fe0bc45e..562831dddf3d 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -339,8 +339,19 @@ public void authorize(ClientState state) throws InvalidRequestException, Unautho public void validate(ClientState state) throws InvalidRequestException { - if (parameters.allowFiltering && !SchemaConstants.isSystemKeyspace(table.keyspace)) + if (SchemaConstants.isSystemKeyspace(table.keyspace)) + return; + if (parameters.allowFiltering) Guardrails.allowFilteringEnabled.ensureEnabled(state); + boolean hasRestrictions = restrictions != null && + (restrictions.hasPartitionKeyRestrictions() || + restrictions.hasClusteringColumnsRestrictions() || + restrictions.hasNonPrimaryKeyRestrictions()); + if (getBindVariables().isEmpty() + && hasRestrictions) + { + Guardrails.onMisprepared(state); + } } @Override diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 5771c628c848..ef0e793404f3 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -40,6 +40,7 @@ import org.apache.cassandra.db.compaction.TimeWindowCompactionStrategy; import org.apache.cassandra.locator.InetAddressAndPort; import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.ClientWarn; import org.apache.cassandra.service.disk.usage.DiskUsageBroadcaster; import org.apache.cassandra.utils.JsonUtils; import org.apache.cassandra.utils.MBeanWrapper; @@ -54,6 +55,9 @@ public final class Guardrails implements GuardrailsMBean public static final String MBEAN_NAME = "org.apache.cassandra.db:type=Guardrails"; public static final GuardrailsConfigProvider CONFIG_PROVIDER = GuardrailsConfigProvider.instance; + + public static final String MISPREPARED_STATEMENT_WARN_MESSAGE = "Performance Tip: This query contains literal values in the WHERE clause. " + "Using '?' placeholders (bind markers) is much more efficient. " + "It allows the server to cache this query once and reuse it for different values, " + "reducing memory usage and improving speed."; + private static final GuardrailsOptions DEFAULT_CONFIG = DatabaseDescriptor.getGuardrailsConfig(); @VisibleForTesting @@ -1899,4 +1903,13 @@ private static long minimumTimestampAsRelativeMicros(@Nullable DurationSpec.Long ? Long.MIN_VALUE : (ClientState.getLastTimestampMicros() - duration.toMicroseconds()); } + + public static void onMisprepared(ClientState state) + { + mispreparedStatementsEnabled.ensureEnabled(state); + + // only warn if user ins't super user or a external call, these checks are handled by guardrail framework + ClientWarn.instance.warn(MISPREPARED_STATEMENT_WARN_MESSAGE); + mispreparedStatementsEnabled.warn(MISPREPARED_STATEMENT_WARN_MESSAGE); + } } diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java index a09c77a6e829..e5895e16f6a4 100644 --- a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -19,6 +19,7 @@ package org.apache.cassandra.cql3; import java.net.InetSocketAddress; +import java.util.List; import org.junit.After; import org.junit.Assert; @@ -30,14 +31,17 @@ import org.apache.cassandra.auth.CassandraAuthorizer; import org.apache.cassandra.auth.PasswordAuthenticator; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.db.guardrails.GuardrailViolatedException; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.ClientWarn; import static org.junit.Assert.assertTrue; public class MispreparedStatementsTest extends CQLTester { - private static final ClientState state = ClientState.forExternalCalls(new InetSocketAddress("127.0.0.1", 9042)); + private static final String TIP = "Using '?' placeholders (bind markers) is much more efficient"; @BeforeClass public static void setupGlobalConfig() @@ -79,6 +83,7 @@ public boolean canLogin() state.login(nonSuperUser); state.setKeyspace(KEYSPACE); + ClientWarn.instance.captureWarnings(); createTable("CREATE TABLE %s (id int, description text, name text, PRIMARY KEY (id, name))"); } @@ -91,209 +96,119 @@ public void tearDown() @Test public void testSelectWithPartitionKey() { - String query = String.format("SELECT * FROM %s WHERE id = 1", currentTable()); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error for mis-prepared SELECT statement"); - } - catch (Exception e) - { - assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id = 1", currentTable())); } @Test public void testSelectWithClusteringKey() { - String query = String.format("SELECT * FROM %s WHERE name = 'v1' ALLOW FILTERING", currentTable()); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error for mis-prepared SELECT statement"); - } - catch (Exception e) - { - assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailViolated(String.format("SELECT * FROM %s WHERE name = 'v1' ALLOW FILTERING", currentTable())); } @Test - public void testInsertJsonGuardrail() + public void testSelectWithFullPrimaryKey() { - String query = String.format("INSERT INTO %s JSON '{\"id\": 1, \"name\": \"v1\"}'", currentTable()); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error for INSERT JSON literal"); - } - catch (Exception e) - { - assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id = 1 AND name = 'v1'", currentTable())); } @Test - public void testUpdateWithIfConditionLiteralShouldFail() + public void testSelectWithRangeRestriction() { - String query = String.format("UPDATE %s SET description = 'v2' WHERE id = 1 AND name = 'v1' IF description = 'v0'", currentTable()); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error because ZERO bind markers are present"); - } - catch (Exception e) - { - assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id = 1 AND name > 'a'", currentTable())); } + @Test + public void testSelectInRestrictionOnPartitionKey() + { + assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id IN (1, 2, 3)", currentTable())); + } @Test - public void testSelectWithFullPrimaryKey() + public void testSelectInRestrictionOnClusteringKey() { - String query = String.format("SELECT * FROM %s WHERE id = 1 AND name = 'v1'", currentTable()); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error for mis-prepared SELECT statement"); - } - catch (Exception e) - { - assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailViolated(String.format("SELECT * FROM %s WHERE name IN ('a', 'b') ALLOW FILTERING", currentTable())); + } + @Test + public void testSelectInRestrictionOnFullPrimaryKey() + { + assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id IN (1, 2, 3) AND name in ('a', 'b')", currentTable())); } @Test - public void testModificationGuardrail() + public void testInsertJsonGuardrail() { - String fullName = KEYSPACE + '.' + currentTable(); - String query = String.format("UPDATE %s SET description = 'new_description' WHERE id = 1 AND name = 'name'", fullName); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error"); - } - catch (Exception e) - { - assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailViolated(String.format("INSERT INTO %s JSON '{\"id\": 1, \"name\": \"v1\"}'", currentTable())); } @Test - public void testDeleteWithFullPrimaryKey() + public void testUpdateWithPartitionKey() { - String query = String.format("DELETE FROM %s WHERE id = 1 AND name = 'v1'", currentTable()); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error for DELETE with literals"); - } - catch (Exception e) - { - assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailViolated(String.format("UPDATE %s SET description = 'new' WHERE id = 1 AND name = 'name'", KEYSPACE + '.' + currentTable())); } @Test - public void testSelectWithRangeRestriction() + public void testUpdateWithIfCondition() { - String query = String.format("SELECT * FROM %s WHERE id = 1 AND name > 'a'", currentTable()); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error for range query with literals"); - } - catch (Exception e) - { - assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailViolated(String.format("UPDATE %s SET description = 'v2' WHERE id = 1 AND name = 'v1' IF description = 'v0'", currentTable())); } @Test - public void testProperlyPreparedWithBindMarkers() + public void testDeleteWithFullPrimaryKey() { - String query = String.format("SELECT * FROM %s WHERE id = ? AND name = ?", currentTable()); - try - { - QueryProcessor.instance.prepare(query, state); - } - catch (Exception e) - { - Assert.fail("Guardrail should NOT trigger for statements using bind markers (?)"); - } + assertGuardrailViolated(String.format("DELETE FROM %s WHERE id = 1 AND name = 'v1'", currentTable())); } @Test public void testBatchGuardrail() { String tableName = currentTable(); - String batchWithLiterals = String.format("BEGIN BATCH " + "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1'; " + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName); - try - { - QueryProcessor.instance.prepare(batchWithLiterals, state); - Assert.fail("Expected guardrail error for BATCH with literals"); - } - catch (Exception e) - { - assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailViolated(String.format("BEGIN BATCH " + + "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + + "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1'; " + + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName)); } @Test - public void testSelectInRestrictionOnPartitionKey() throws Throwable + public void testMultiTableBatchGuardrail() { - String fullTableName = KEYSPACE + '.' + currentTable(); - String query = String.format("SELECT * FROM %s WHERE id IN (1, 2, 3)", fullTableName); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error for regular user with IN clause literals"); - } - catch (Exception e) - { - assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); - } + String table1 = currentTable(); + String table2 = createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); + String query = String.format("BEGIN BATCH " + + "UPDATE %s.%s SET description = 'v1' WHERE id = 1 AND name = 'n1'; " + + "INSERT INTO %s.%s (id, val) VALUES (2, 'v2'); " + + "APPLY BATCH", + KEYSPACE, table1, KEYSPACE, table2); + + DatabaseDescriptor.setMispreparedStatementsEnabled(false); + assertGuardrailViolated(query); + + DatabaseDescriptor.setMispreparedStatementsEnabled(true); + assertGuardrailPassed(query); } @Test - public void testSelectInRestrictionOnClusteringKey() + public void testProperlyPreparedWithBindMarkers() { - String fullTableName = KEYSPACE + '.' + currentTable(); - String query = String.format("SELECT * FROM %s WHERE name IN ('a', 'b') ALLOW FILTERING", fullTableName); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error for regular user with IN clause literals"); - } - catch (Exception e) - { - assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = ? AND name = ?", currentTable())); } @Test - public void testSelectInRestrictionOnFullPrimaryKey() + public void testSelectAllPasses() { - String fullTableName = KEYSPACE + '.' + currentTable(); - String query = String.format("SELECT * FROM %s WHERE id IN (1, 2, 3) AND name in ('a', 'b')", fullTableName); - try - { - QueryProcessor.instance.prepare(query, state); - Assert.fail("Expected guardrail error for regular user with IN clause literals"); - } - catch (Exception e) - { - assertTrue("Expected guardrail error, but got: " + e.getMessage(), e.getMessage().contains("Mis-prepared statements is not allowed")); - } + assertGuardrailPassed(String.format("SELECT * FROM %s", currentTable())); + } + + @Test + public void testLiteralInProjectionIsAllowed() + { + assertGuardrailPassed(String.format("SELECT id, (text)'const_val' FROM %s WHERE id = ?", currentTable())); } @Test public void testInternalBypass() { - ClientState internalState = ClientState.forInternalCalls(); - QueryProcessor.instance.prepare("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", internalState); + assertGuardrailPassed("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", ClientState.forInternalCalls()); } @Test @@ -301,7 +216,6 @@ public void testSuperUserBypass() { AuthenticatedUser superUser = new AuthenticatedUser("super-user") { - @Override public boolean isSuper() { @@ -326,39 +240,33 @@ public boolean canLogin() return true; } }; - ClientState superUserState = ClientState.forExternalCalls(new InetSocketAddress("127.0.0.1", 9042)); superUserState.login(superUser); - QueryProcessor.instance.prepare("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", superUserState); + assertGuardrailPassed("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", superUserState); } @Test - public void testSelectAllPasses() + public void testSystemKeyspaceBypassForRegularUser() { - String query = String.format("SELECT * FROM %s", currentTable()); - try - { - QueryProcessor.instance.prepare(query, state); - } - catch (Exception e) - { - Assert.fail("Did not expect guardrail error for SELECT * statement"); - } + assertGuardrailPassed("SELECT * FROM system.local WHERE key = 'local'"); } @Test public void testGuardrailDisabledAllowsLiterals() { DatabaseDescriptor.setMispreparedStatementsEnabled(true); - String query = String.format("SELECT * FROM %s WHERE id = 1", currentTable()); - try - { - QueryProcessor.instance.prepare(query, state); - } - catch (Exception e) - { - Assert.fail("Guardrail should NOT have triggered because it is disabled (set to true)"); - } + assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = 1", currentTable())); + } + + @Test + public void testWarningIsIssuedWhenGuardrailIsAllowed() + { + DatabaseDescriptor.setMispreparedStatementsEnabled(true); + ClientWarn.instance.captureWarnings(); + assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = 1", currentTable())); + List warnings = ClientWarn.instance.getWarnings(); + Assert.assertNotNull(warnings); + assertTrue(warnings.stream().anyMatch(w -> w.contains("Using '?' placeholders (bind markers) is much more efficient"))); } @Test @@ -366,47 +274,47 @@ public void testGuardrailDisabledAllowsBatchLiterals() { DatabaseDescriptor.setMispreparedStatementsEnabled(true); String tableName = currentTable(); - String batchWithLiterals = String.format("BEGIN BATCH " + - "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + - "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1'; " + - "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName); - try - { - QueryProcessor.instance.prepare(batchWithLiterals, state); - } - catch (Exception e) - { - Assert.fail("Batch with literals should be allowed when guardrail is disabled"); - } + assertGuardrailPassed(String.format("BEGIN BATCH " + + "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + + "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1'; " + + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName)); } - @Test - public void testSystemKeyspaceBypassForRegularUser() + private void assertGuardrailViolated(String query) { - String query = "SELECT * FROM system.local WHERE key = 'local'"; try { + ClientWarn.instance.captureWarnings(); QueryProcessor.instance.prepare(query, state); + Assert.fail("Expected GuardrailViolatedException for query: " + query); } catch (Exception e) { - Assert.fail("Guardrail should NOT trigger for system keyspace queries"); + assertTrue(e instanceof GuardrailViolatedException); + List warnings = ClientWarn.instance.getWarnings(); + if (warnings != null) + { + assertTrue("Performance tip should not be issued when blocking", + warnings.stream().noneMatch(w -> w.contains(Guardrails.MISPREPARED_STATEMENT_WARN_MESSAGE))); + } + assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed")); } } - @Test - public void testLiteralInProjectionIsAllowed() + private void assertGuardrailPassed(String query) + { + assertGuardrailPassed(query, state); + } + + private void assertGuardrailPassed(String query, ClientState clientState) { - String query = String.format("SELECT id, (text)'const_val' FROM %s WHERE id = ?", currentTable()); try { - QueryProcessor.instance.prepare(query, state); + QueryProcessor.instance.prepare(query, clientState); } catch (Exception e) { - e.printStackTrace(); - Assert.fail("Guardrail should only trigger for literals in RESTRICTIONS (WHERE clause)"); + Assert.fail("Expected guardrail to pass, but got: " + e.getMessage()); } } -} - +} \ No newline at end of file From 0304d2381e40ad65b652ffeeb95ceb082fde13f7 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Mon, 2 Feb 2026 14:54:57 +0530 Subject: [PATCH 12/19] remove redundant variable declaration --- .../org/apache/cassandra/cql3/MispreparedStatementsTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java index e5895e16f6a4..25f27515c842 100644 --- a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -41,7 +41,6 @@ public class MispreparedStatementsTest extends CQLTester { private static final ClientState state = ClientState.forExternalCalls(new InetSocketAddress("127.0.0.1", 9042)); - private static final String TIP = "Using '?' placeholders (bind markers) is much more efficient"; @BeforeClass public static void setupGlobalConfig() From 029259d397a90fdd0fd24596c0e3e402ab2d3a0f Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Mon, 2 Feb 2026 15:23:27 +0530 Subject: [PATCH 13/19] fixed GuardrailsConfigCommandsTest --- .../apache/cassandra/db/guardrails/Guardrails.java | 12 ++++++++++++ .../cassandra/db/guardrails/GuardrailsMBean.java | 7 +++++++ .../tools/nodetool/GuardrailsConfigCommandsTest.java | 2 +- 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index ef0e793404f3..2fd625926b86 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -722,6 +722,18 @@ public void setKeyspacesThreshold(int warn, int fail) DEFAULT_CONFIG.setKeyspacesThreshold(warn, fail); } + @Override + public boolean getMispreparedStatementsEnabled() + { + return DEFAULT_CONFIG.getMispreparedStatementsEnabled(); + } + + @Override + public void setMispreparedStatementsEnabled(boolean enabled) + { + DEFAULT_CONFIG.setMispreparedStatementsEnabled(enabled); + } + @Override public int getTablesWarnThreshold() { diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java index e2e9cd74ddb9..31fb8d865def 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsMBean.java @@ -54,6 +54,13 @@ public interface GuardrailsMBean */ void setKeyspacesThreshold(int warn, int fail); + /** + * @return the use of misprepared statement is enabled + */ + boolean getMispreparedStatementsEnabled(); + + void setMispreparedStatementsEnabled(boolean enabled); + /** * @return The threshold to warn when creating more tables than threshold. * -1 means disabled. diff --git a/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java b/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java index 21a46c41e7a4..1aade56a0006 100644 --- a/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java +++ b/test/unit/org/apache/cassandra/tools/nodetool/GuardrailsConfigCommandsTest.java @@ -208,8 +208,8 @@ private Set getConfigFieldNames() "group_by_enabled true\n" + "intersect_filtering_query_enabled true\n" + "intersect_filtering_query_warned true\n" + + "misprepared_statements_enabled true\n" + "non_partition_restricted_index_query_enabled true\n" + - "misprepared_statements_enabled false\n" + "read_before_write_list_operations_enabled true\n" + "secondary_indexes_enabled true\n" + "simplestrategy_enabled true\n" + From 739ff86ba66e61a71b1e67d584f0aace6206bf09 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Mon, 2 Feb 2026 21:26:49 +0530 Subject: [PATCH 14/19] remove explicit client warn message --- src/java/org/apache/cassandra/db/guardrails/Guardrails.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 2fd625926b86..19bb78623155 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -40,7 +40,6 @@ import org.apache.cassandra.db.compaction.TimeWindowCompactionStrategy; import org.apache.cassandra.locator.InetAddressAndPort; import org.apache.cassandra.service.ClientState; -import org.apache.cassandra.service.ClientWarn; import org.apache.cassandra.service.disk.usage.DiskUsageBroadcaster; import org.apache.cassandra.utils.JsonUtils; import org.apache.cassandra.utils.MBeanWrapper; @@ -1921,7 +1920,6 @@ public static void onMisprepared(ClientState state) mispreparedStatementsEnabled.ensureEnabled(state); // only warn if user ins't super user or a external call, these checks are handled by guardrail framework - ClientWarn.instance.warn(MISPREPARED_STATEMENT_WARN_MESSAGE); mispreparedStatementsEnabled.warn(MISPREPARED_STATEMENT_WARN_MESSAGE); } } From 7270f9184f81906b6ba1093d02043e02e42c0774 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Wed, 4 Feb 2026 13:39:52 +0530 Subject: [PATCH 15/19] fixed warning logic to not warn interal and super user --- .../cassandra/db/guardrails/Guardrails.java | 7 +- .../cql3/MispreparedStatementsTest.java | 81 +++++++++++++++---- 2 files changed, 70 insertions(+), 18 deletions(-) diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index 19bb78623155..f2424b84874f 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -1919,7 +1919,10 @@ public static void onMisprepared(ClientState state) { mispreparedStatementsEnabled.ensureEnabled(state); - // only warn if user ins't super user or a external call, these checks are handled by guardrail framework - mispreparedStatementsEnabled.warn(MISPREPARED_STATEMENT_WARN_MESSAGE); + // only warn if user ins't super user or a external call + if (!state.isInternal && !state.isSuper()) + { + mispreparedStatementsEnabled.warn(MISPREPARED_STATEMENT_WARN_MESSAGE); + } } } diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java index 25f27515c842..cde6b5d6456d 100644 --- a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -96,66 +96,77 @@ public void tearDown() public void testSelectWithPartitionKey() { assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id = 1", currentTable())); + assertNoWarnings(); } @Test public void testSelectWithClusteringKey() { assertGuardrailViolated(String.format("SELECT * FROM %s WHERE name = 'v1' ALLOW FILTERING", currentTable())); + assertNoWarnings(); } @Test public void testSelectWithFullPrimaryKey() { assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id = 1 AND name = 'v1'", currentTable())); + assertNoWarnings(); } @Test public void testSelectWithRangeRestriction() { assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id = 1 AND name > 'a'", currentTable())); + assertNoWarnings(); } @Test public void testSelectInRestrictionOnPartitionKey() { assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id IN (1, 2, 3)", currentTable())); + assertNoWarnings(); } @Test public void testSelectInRestrictionOnClusteringKey() { assertGuardrailViolated(String.format("SELECT * FROM %s WHERE name IN ('a', 'b') ALLOW FILTERING", currentTable())); + assertNoWarnings(); } @Test public void testSelectInRestrictionOnFullPrimaryKey() { assertGuardrailViolated(String.format("SELECT * FROM %s WHERE id IN (1, 2, 3) AND name in ('a', 'b')", currentTable())); + assertNoWarnings(); } @Test public void testInsertJsonGuardrail() { assertGuardrailViolated(String.format("INSERT INTO %s JSON '{\"id\": 1, \"name\": \"v1\"}'", currentTable())); + assertNoWarnings(); } @Test public void testUpdateWithPartitionKey() { assertGuardrailViolated(String.format("UPDATE %s SET description = 'new' WHERE id = 1 AND name = 'name'", KEYSPACE + '.' + currentTable())); + assertNoWarnings(); } @Test public void testUpdateWithIfCondition() { assertGuardrailViolated(String.format("UPDATE %s SET description = 'v2' WHERE id = 1 AND name = 'v1' IF description = 'v0'", currentTable())); + assertNoWarnings(); } @Test public void testDeleteWithFullPrimaryKey() { assertGuardrailViolated(String.format("DELETE FROM %s WHERE id = 1 AND name = 'v1'", currentTable())); + assertNoWarnings(); } @Test @@ -166,6 +177,7 @@ public void testBatchGuardrail() "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1'; " + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName)); + assertNoWarnings(); } @Test @@ -179,35 +191,36 @@ public void testMultiTableBatchGuardrail() "APPLY BATCH", KEYSPACE, table1, KEYSPACE, table2); - DatabaseDescriptor.setMispreparedStatementsEnabled(false); assertGuardrailViolated(query); - - DatabaseDescriptor.setMispreparedStatementsEnabled(true); - assertGuardrailPassed(query); + assertNoWarnings(); } @Test public void testProperlyPreparedWithBindMarkers() { assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = ? AND name = ?", currentTable())); + assertNoWarnings(); } @Test public void testSelectAllPasses() { assertGuardrailPassed(String.format("SELECT * FROM %s", currentTable())); + assertNoWarnings(); } @Test public void testLiteralInProjectionIsAllowed() { assertGuardrailPassed(String.format("SELECT id, (text)'const_val' FROM %s WHERE id = ?", currentTable())); + assertNoWarnings(); } @Test public void testInternalBypass() { assertGuardrailPassed("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", ClientState.forInternalCalls()); + assertNoWarnings(); } @Test @@ -242,12 +255,14 @@ public boolean canLogin() ClientState superUserState = ClientState.forExternalCalls(new InetSocketAddress("127.0.0.1", 9042)); superUserState.login(superUser); assertGuardrailPassed("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", superUserState); + assertNoWarnings(); } @Test public void testSystemKeyspaceBypassForRegularUser() { assertGuardrailPassed("SELECT * FROM system.local WHERE key = 'local'"); + assertNoWarnings(); } @Test @@ -255,6 +270,7 @@ public void testGuardrailDisabledAllowsLiterals() { DatabaseDescriptor.setMispreparedStatementsEnabled(true); assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = 1", currentTable())); + assertWarnings(); } @Test @@ -263,9 +279,8 @@ public void testWarningIsIssuedWhenGuardrailIsAllowed() DatabaseDescriptor.setMispreparedStatementsEnabled(true); ClientWarn.instance.captureWarnings(); assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = 1", currentTable())); - List warnings = ClientWarn.instance.getWarnings(); - Assert.assertNotNull(warnings); - assertTrue(warnings.stream().anyMatch(w -> w.contains("Using '?' placeholders (bind markers) is much more efficient"))); + assertWarnings(); + } @Test @@ -277,27 +292,61 @@ public void testGuardrailDisabledAllowsBatchLiterals() "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1'; " + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName)); + assertWarnings(); + } + + @Test + public void testBlockedStatementsDoNotEnterCache() + { + QueryProcessor.clearPreparedStatementsCache(); + int initialCacheSize = QueryProcessor.preparedStatementsCount(); + for (int i = 0; i < 10; i++) + { + String query = String.format("SELECT * FROM %s WHERE id = %d", currentTable(), i); + try + { + QueryProcessor.instance.prepare(query, state); + } + catch (Exception e) + { + // Expected: Guardrail throws exception + } + } + Assert.assertEquals("Blocked misprepared statements should not be added to the cache", initialCacheSize, QueryProcessor.preparedStatementsCount()); } private void assertGuardrailViolated(String query) { try { - ClientWarn.instance.captureWarnings(); QueryProcessor.instance.prepare(query, state); Assert.fail("Expected GuardrailViolatedException for query: " + query); } - catch (Exception e) + catch (GuardrailViolatedException e) { - assertTrue(e instanceof GuardrailViolatedException); - List warnings = ClientWarn.instance.getWarnings(); - if (warnings != null) - { - assertTrue("Performance tip should not be issued when blocking", - warnings.stream().noneMatch(w -> w.contains(Guardrails.MISPREPARED_STATEMENT_WARN_MESSAGE))); - } assertTrue(e.getMessage().contains("Mis-prepared statements is not allowed")); } + catch (Exception e) + { + Assert.fail(e.getMessage()); + } + } + + private void assertNoWarnings() + { + List warnings = ClientWarn.instance.getWarnings(); + if (warnings != null) + { + assertTrue("Expected performance tip warning was found", + warnings.stream().noneMatch(w -> w.contains(Guardrails.MISPREPARED_STATEMENT_WARN_MESSAGE))); + } + } + + private void assertWarnings() + { + List warnings = ClientWarn.instance.getWarnings(); + assertTrue("Expedted performance tip warning was not found", + warnings.stream().anyMatch(w -> w.contains(Guardrails.MISPREPARED_STATEMENT_WARN_MESSAGE))); } private void assertGuardrailPassed(String query) From 586af836b062a5d5b80d3fbaf797c2f50f2dd804 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Wed, 4 Feb 2026 16:35:20 +0530 Subject: [PATCH 16/19] added test to config initial value of misprepared_statements_enable is true --- .../config/DatabaseDescriptorTest.java | 2 + .../cql3/MispreparedStatementsTest.java | 63 +++++++++++++------ 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java index c640f09dc476..88d583d57f52 100644 --- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java +++ b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java @@ -793,6 +793,8 @@ public void testMispreparedStatementsEnabled() { DatabaseDescriptor.setMispreparedStatementsEnabled(false); Assert.assertFalse("Value should be false after setting to false", DatabaseDescriptor.getMispreparedStatementsEnabled()); + + Assert.assertTrue("Default value of misprepared_statement_enabled must be true", originalValue); } finally { diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java index cde6b5d6456d..2cb437e5dca1 100644 --- a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -83,7 +83,7 @@ public boolean canLogin() state.login(nonSuperUser); state.setKeyspace(KEYSPACE); ClientWarn.instance.captureWarnings(); - createTable("CREATE TABLE %s (id int, description text, name text, PRIMARY KEY (id, name))"); + createTable("CREATE TABLE %s (id int, description text, age int, name text, PRIMARY KEY (id, name, age))"); } @After @@ -113,6 +113,26 @@ public void testSelectWithFullPrimaryKey() assertNoWarnings(); } + @Test + public void testSelectWithWhereNonPrimaryKeyColumn() + { + assertGuardrailViolated(String.format("SELECT * FROM %s WHERE description = 'foo' ALLOW FILTERING", currentTable())); + assertNoWarnings(); + } + + + @Test + public void testSelectWithCompositeRestriction() + { + assertGuardrailViolated(String.format("SELECT sum(id) from %s WHERE (name, age) = ('a', 1) ALLOW FILTERING", currentTable())); + } + + @Test + public void testSelectWithFunction() + { + assertGuardrailViolated(String.format("SELECT sum(id) from %s where name = 'v1' ALLOW FILTERING", currentTable())); + } + @Test public void testSelectWithRangeRestriction() { @@ -141,6 +161,20 @@ public void testSelectInRestrictionOnFullPrimaryKey() assertNoWarnings(); } + @Test + public void testDDLStatementsBypass() + { + assertGuardrailPassed("CREATE TABLE IF NOT EXISTS test_table (id INT PRIMARY KEY)"); + assertNoWarnings(); + } + + @Test + public void testWhereLiteralWithLike() + { + assertGuardrailViolated(String.format("SELECT * FROM %s WHERE name LIKE 'prefix%%' ALLOW FILTERING", currentTable())); + assertNoWarnings(); + } + @Test public void testInsertJsonGuardrail() { @@ -151,14 +185,14 @@ public void testInsertJsonGuardrail() @Test public void testUpdateWithPartitionKey() { - assertGuardrailViolated(String.format("UPDATE %s SET description = 'new' WHERE id = 1 AND name = 'name'", KEYSPACE + '.' + currentTable())); + assertGuardrailViolated(String.format("UPDATE %s SET description = 'new' WHERE id = 1 AND name = 'name' AND age = 1", KEYSPACE + '.' + currentTable())); assertNoWarnings(); } @Test public void testUpdateWithIfCondition() { - assertGuardrailViolated(String.format("UPDATE %s SET description = 'v2' WHERE id = 1 AND name = 'v1' IF description = 'v0'", currentTable())); + assertGuardrailViolated(String.format("UPDATE %s SET description = 'v2' WHERE id = 1 AND name = 'v1' AND age = 1 IF description = 'v0'", currentTable())); assertNoWarnings(); } @@ -170,23 +204,12 @@ public void testDeleteWithFullPrimaryKey() } @Test - public void testBatchGuardrail() - { - String tableName = currentTable(); - assertGuardrailViolated(String.format("BEGIN BATCH " + - "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + - "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1'; " + - "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName)); - assertNoWarnings(); - } - - @Test - public void testMultiTableBatchGuardrail() + public void testMultiTableBatchGuardrailAllLiteral() { String table1 = currentTable(); String table2 = createTable("CREATE TABLE %s (id int PRIMARY KEY, val text)"); String query = String.format("BEGIN BATCH " + - "UPDATE %s.%s SET description = 'v1' WHERE id = 1 AND name = 'n1'; " + + "UPDATE %s.%s SET description = 'v1' WHERE id = 1 AND name = 'n1' AND age = 1; " + "INSERT INTO %s.%s (id, val) VALUES (2, 'v2'); " + "APPLY BATCH", KEYSPACE, table1, KEYSPACE, table2); @@ -289,8 +312,8 @@ public void testGuardrailDisabledAllowsBatchLiterals() DatabaseDescriptor.setMispreparedStatementsEnabled(true); String tableName = currentTable(); assertGuardrailPassed(String.format("BEGIN BATCH " + - "INSERT INTO %s.%s (id, description, name) VALUES (1, 'v1', 'v1'); " + - "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1'; " + + "INSERT INTO %s.%s (id, description, name, age) VALUES (1, 'v1', 'v1', 1); " + + "UPDATE %s.%s SET description = 'v2' WHERE id = 2 AND name = 'v1' AND age = 1; " + "APPLY BATCH", KEYSPACE, tableName, KEYSPACE, tableName)); assertWarnings(); } @@ -307,7 +330,7 @@ public void testBlockedStatementsDoNotEnterCache() { QueryProcessor.instance.prepare(query, state); } - catch (Exception e) + catch (GuardrailViolatedException e) { // Expected: Guardrail throws exception } @@ -345,7 +368,7 @@ private void assertNoWarnings() private void assertWarnings() { List warnings = ClientWarn.instance.getWarnings(); - assertTrue("Expedted performance tip warning was not found", + assertTrue("Expected performance tip warning was not found", warnings.stream().anyMatch(w -> w.contains(Guardrails.MISPREPARED_STATEMENT_WARN_MESSAGE))); } From 6dbbc906beeb3cd90fbcc17f34ae046bb5dbe87a Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Thu, 5 Feb 2026 13:02:41 +0530 Subject: [PATCH 17/19] fix warning times --- .python-version | 1 - .../apache/cassandra/config/GuardrailsOptions.java | 2 +- .../org/apache/cassandra/cql3/CQLStatement.java | 1 + .../org/apache/cassandra/cql3/QueryProcessor.java | 2 +- .../apache/cassandra/db/guardrails/Guardrails.java | 13 +++---------- .../cassandra/db/guardrails/GuardrailsConfig.java | 2 -- .../apache/cassandra/service/StorageService.java | 1 - .../cassandra/cql3/MispreparedStatementsTest.java | 3 --- 8 files changed, 6 insertions(+), 19 deletions(-) delete mode 100644 .python-version diff --git a/.python-version b/.python-version deleted file mode 100644 index 2419ad5b0a32..000000000000 --- a/.python-version +++ /dev/null @@ -1 +0,0 @@ -3.11.9 diff --git a/src/java/org/apache/cassandra/config/GuardrailsOptions.java b/src/java/org/apache/cassandra/config/GuardrailsOptions.java index bf9c9348052e..ccef9ea0ad64 100644 --- a/src/java/org/apache/cassandra/config/GuardrailsOptions.java +++ b/src/java/org/apache/cassandra/config/GuardrailsOptions.java @@ -1338,7 +1338,7 @@ public boolean getMispreparedStatementsEnabled() public void setMispreparedStatementsEnabled(boolean enabled) { - updatePropertyWithLogging("misprepare_statements_enabled", + updatePropertyWithLogging("misprepared_statements_enabled", enabled, () -> config.misprepared_statements_enabled, x -> config.misprepared_statements_enabled = x); diff --git a/src/java/org/apache/cassandra/cql3/CQLStatement.java b/src/java/org/apache/cassandra/cql3/CQLStatement.java index db9896361ecf..042aade6a3f4 100644 --- a/src/java/org/apache/cassandra/cql3/CQLStatement.java +++ b/src/java/org/apache/cassandra/cql3/CQLStatement.java @@ -24,6 +24,7 @@ import org.apache.cassandra.audit.AuditLogContext; import org.apache.cassandra.cql3.functions.Function; +import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.QueryState; import org.apache.cassandra.transport.Dispatcher; diff --git a/src/java/org/apache/cassandra/cql3/QueryProcessor.java b/src/java/org/apache/cassandra/cql3/QueryProcessor.java index 1af40d0d4878..606da473056d 100644 --- a/src/java/org/apache/cassandra/cql3/QueryProcessor.java +++ b/src/java/org/apache/cassandra/cql3/QueryProcessor.java @@ -130,7 +130,6 @@ public class QueryProcessor implements QueryHandler // counters. Callers of processStatement are responsible for correctly notifying metrics public static final CQLMetrics metrics = new CQLMetrics(); - // Paging size to use when preloading prepared statements. public static final int PRELOAD_PREPARED_STATEMENTS_FETCH_SIZE = 5000; @@ -495,6 +494,7 @@ public static Prepared parseAndPrepare(String query, ClientState clientState, bo // Note: if 2 threads prepare the same query, we'll live so don't bother synchronizing CQLStatement statement = raw.prepare(clientState); statement.validate(clientState); + // Set CQL string for AlterSchemaStatement as this is used to serialize the transformation // in the cluster metadata log if (statement instanceof AlterSchemaStatement) diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index f2424b84874f..e59645210c85 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -619,15 +619,13 @@ public final class Guardrails implements GuardrailsMBean /** * Prevents heap exhaustion caused by the anti-pattern of preparing queries with * hardcoded literals instead of bind markers. This prevents filling the statement cache with non-reusable entries. - * - * @see CASSANDRA-21139 */ public static final EnableFlag mispreparedStatementsEnabled = new EnableFlag("misprepared_statements_enabled", - "Mis-prepared statements cause server-side memory exhaustion and high GC pressure by flooding the statement cache with non-reusable query entries", + "misprepared statements cause server-side memory exhaustion and high GC pressure by flooding the statement cache with non-reusable query entries", state -> CONFIG_PROVIDER.getOrCreate(state).getMispreparedStatementsEnabled(), - "Mis-prepared statements"); + "misprepared statements"); public static final MaxThreshold maximumAllowableTimestamp = new MaxThreshold("maximum_timestamp", @@ -1918,11 +1916,6 @@ private static long minimumTimestampAsRelativeMicros(@Nullable DurationSpec.Long public static void onMisprepared(ClientState state) { mispreparedStatementsEnabled.ensureEnabled(state); - - // only warn if user ins't super user or a external call - if (!state.isInternal && !state.isSuper()) - { - mispreparedStatementsEnabled.warn(MISPREPARED_STATEMENT_WARN_MESSAGE); - } + mispreparedStatementsEnabled.warn(MISPREPARED_STATEMENT_WARN_MESSAGE); } } diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index e91442959f64..b428a8cdabc2 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -543,8 +543,6 @@ public interface GuardrailsConfig void setIntersectFilteringQueryEnabled(boolean value); - - /** * @return A timestamp that if a user supplied timestamp is after will trigger a warning */ diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index d53836f626e6..b1a6db5e85a2 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -4846,7 +4846,6 @@ public boolean getMispreparedStatementsEnabled() public void setMispreparedStatementsEnabled(boolean enabled) { DatabaseDescriptor.setMispreparedStatementsEnabled(enabled); - logger.info("Updated misprepared_statements_enabled to {}", enabled); } /* diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java index 2cb437e5dca1..38c07be8e709 100644 --- a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -243,7 +243,6 @@ public void testLiteralInProjectionIsAllowed() public void testInternalBypass() { assertGuardrailPassed("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", ClientState.forInternalCalls()); - assertNoWarnings(); } @Test @@ -278,7 +277,6 @@ public boolean canLogin() ClientState superUserState = ClientState.forExternalCalls(new InetSocketAddress("127.0.0.1", 9042)); superUserState.login(superUser); assertGuardrailPassed("SELECT * FROM " + KEYSPACE + '.' + currentTable() + " WHERE id = 1", superUserState); - assertNoWarnings(); } @Test @@ -300,7 +298,6 @@ public void testGuardrailDisabledAllowsLiterals() public void testWarningIsIssuedWhenGuardrailIsAllowed() { DatabaseDescriptor.setMispreparedStatementsEnabled(true); - ClientWarn.instance.captureWarnings(); assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = 1", currentTable())); assertWarnings(); From 8a70ced10ac94b936baf05d25d088e72e12c2237 Mon Sep 17 00:00:00 2001 From: Rishabh Saraswat Date: Tue, 10 Feb 2026 13:01:52 +0530 Subject: [PATCH 18/19] Apply suggestion from @bschoening Co-authored-by: Brad Schoening <5796692+bschoening@users.noreply.github.com> --- src/java/org/apache/cassandra/db/guardrails/Guardrails.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index e59645210c85..da1c433957cd 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -55,7 +55,7 @@ public final class Guardrails implements GuardrailsMBean public static final GuardrailsConfigProvider CONFIG_PROVIDER = GuardrailsConfigProvider.instance; - public static final String MISPREPARED_STATEMENT_WARN_MESSAGE = "Performance Tip: This query contains literal values in the WHERE clause. " + "Using '?' placeholders (bind markers) is much more efficient. " + "It allows the server to cache this query once and reuse it for different values, " + "reducing memory usage and improving speed."; + public static final String MISPREPARED_STATEMENT_WARN_MESSAGE = "This query contains only literal values in the WHERE clause. " + "Using one or more '?' placeholder values (bind markers) allow a prepared statement to be reused."; private static final GuardrailsOptions DEFAULT_CONFIG = DatabaseDescriptor.getGuardrailsConfig(); From 5570c30d30d4f218ab749ebb1f1a8db9f6c61ab4 Mon Sep 17 00:00:00 2001 From: omniCoder77 Date: Tue, 10 Feb 2026 13:47:04 +0530 Subject: [PATCH 19/19] new test case with bind marker and literal --- src/java/org/apache/cassandra/cql3/CQLStatement.java | 1 - .../org/apache/cassandra/db/guardrails/Guardrails.java | 4 ++-- .../cassandra/db/guardrails/GuardrailsConfig.java | 3 +-- .../cassandra/cql3/MispreparedStatementsTest.java | 10 +++++++++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/java/org/apache/cassandra/cql3/CQLStatement.java b/src/java/org/apache/cassandra/cql3/CQLStatement.java index 042aade6a3f4..db9896361ecf 100644 --- a/src/java/org/apache/cassandra/cql3/CQLStatement.java +++ b/src/java/org/apache/cassandra/cql3/CQLStatement.java @@ -24,7 +24,6 @@ import org.apache.cassandra.audit.AuditLogContext; import org.apache.cassandra.cql3.functions.Function; -import org.apache.cassandra.db.guardrails.Guardrails; import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.QueryState; import org.apache.cassandra.transport.Dispatcher; diff --git a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java index da1c433957cd..b856993d1f99 100644 --- a/src/java/org/apache/cassandra/db/guardrails/Guardrails.java +++ b/src/java/org/apache/cassandra/db/guardrails/Guardrails.java @@ -617,13 +617,13 @@ public final class Guardrails implements GuardrailsMBean what, value, isWarning ? "warning" : "failure", threshold)); /** - * Prevents heap exhaustion caused by the anti-pattern of preparing queries with + * Prevents cache overflow and eviction caused by the anti-pattern of preparing queries with * hardcoded literals instead of bind markers. This prevents filling the statement cache with non-reusable entries. */ public static final EnableFlag mispreparedStatementsEnabled = new EnableFlag("misprepared_statements_enabled", - "misprepared statements cause server-side memory exhaustion and high GC pressure by flooding the statement cache with non-reusable query entries", + "misprepared statements create non-reusable query entries and cause cache overflow", state -> CONFIG_PROVIDER.getOrCreate(state).getMispreparedStatementsEnabled(), "misprepared statements"); diff --git a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java index b428a8cdabc2..ce2d4f460693 100644 --- a/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java +++ b/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java @@ -408,7 +408,7 @@ public interface GuardrailsConfig /** *

- * A statement is considered "mis-prepared" if it contains hardcoded literal values + * A statement is considered "mis-prepared" if it contains only hardcoded liter values and without any bind markers * instead of bind markers. This is a performance anti-pattern because it prevents * query plan reuse and floods the server-side Prepared Statement Cache with * unique entries, leading to heap exhaustion and high GC pressure. @@ -430,7 +430,6 @@ public interface GuardrailsConfig * * @return true if the usage of mis-prepared statements is enabled, false otherwise. Returns true by default. * {@code false} if mis-prepared statements should be strictly rejected, causing the query to fail. - * @see CASSANDRA-21139 */ boolean getMispreparedStatementsEnabled(); diff --git a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java index 38c07be8e709..f6752ff202f3 100644 --- a/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java +++ b/test/unit/org/apache/cassandra/cql3/MispreparedStatementsTest.java @@ -219,12 +219,20 @@ public void testMultiTableBatchGuardrailAllLiteral() } @Test - public void testProperlyPreparedWithBindMarkers() + public void testSucceedWithOnlyBindMarkers() { assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = ? AND name = ?", currentTable())); assertNoWarnings(); } + @Test + public void testSucceedWithOneBindMarkerOneLiteral() + { + + assertGuardrailPassed(String.format("SELECT * FROM %s WHERE id = ? AND name = 'v1'", currentTable())); + assertNoWarnings(); + } + @Test public void testSelectAllPasses() {