[GitHub] [cassandra] adelapena commented on a change in pull request #1453: added enable for ALLOW FILTERING property

[GitBox <gi...@apache.org>]

adelapena commented on a change in pull request #1453:
URL: https://github.com/apache/cassandra/pull/1453#discussion_r816039354



##########
File path: src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
##########
@@ -231,7 +233,8 @@ public void authorize(ClientState state) throws InvalidRequestException, Unautho
 
     public void validate(ClientState state) throws InvalidRequestException
     {
-        // Nothing to do, all validation has been done by RawStatement.prepare()
+        if (parameters.allowFiltering && !SchemaConstants.isSystemKeyspace(table.keyspace))
+            checkTrue(DatabaseDescriptor.getGuardrailsConfig().getAllowFilteringEnabled(), "ALLOW FILTERING statement is prohibited");

Review comment:
       This isn't verifying the `ClientState` nor the global enabling of guardrails. Instead of directly verifying the config property we should use the guardrail itself:
   ```suggestion
               Guardrails.allowFilteringEnabled.ensureEnabled(state);
   ```
   There is a note about this [here](https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/guardrails/GuardrailsConfig.java#L26-L28).
   
   Unfortunately that makes the new tests fail, because `CQLTester` uses an internal query that is excluded from guardrails checks. Probably it would be easier to add a new test class extending `GuardrailTester`, which contains utilities for this. That class would be very similar to, for example, `GuardrailUserTimestampsTest`, which is also used to test another `DisabledFlag`s.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org