You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2022/04/06 22:13:01 UTC

[GitHub] [cassandra] dcapwell commented on a diff in pull request #1543: CASSANDRA-17509

dcapwell commented on code in PR #1543:
URL: https://github.com/apache/cassandra/pull/1543#discussion_r844457743


##########
src/java/org/apache/cassandra/db/guardrails/Guardrails.java:
##########
@@ -456,6 +461,18 @@ public void setUserTimestampsEnabled(boolean enabled)
         DEFAULT_CONFIG.setUserTimestampsEnabled(enabled);
     }
 
+    @Override
+    public boolean getGroupByEnabled()

Review Comment:
   same as other comment, should this be `is` rather than `get`?



##########
src/java/org/apache/cassandra/config/GuardrailsOptions.java:
##########
@@ -332,6 +332,20 @@ public void setUserTimestampsEnabled(boolean enabled)
                                   x -> config.user_timestamps_enabled = x);
     }
 
+    @Override
+    public boolean getGroupByEnabled()

Review Comment:
   should this be `is` rather than `get`, since `boolean` rather than `Boolean`?



##########
src/java/org/apache/cassandra/db/guardrails/Guardrails.java:
##########
@@ -129,6 +129,11 @@
                     state -> !CONFIG_PROVIDER.getOrCreate(state).getUserTimestampsEnabled(),
                     "User provided timestamps (USING TIMESTAMP)");
 
+    public static final DisableFlag groupByEnabled =
+    new DisableFlag("group_by",
+                    state -> !CONFIG_PROVIDER.getOrCreate(state).getGroupByEnabled(),

Review Comment:
   outside the scope of this issue:
   
   Rather than `DisableFlag` maybe we make it `EnabledFlag` and undo the `!` prefix every usage uses...



##########
src/java/org/apache/cassandra/db/guardrails/Guardrails.java:
##########
@@ -129,6 +129,11 @@
                     state -> !CONFIG_PROVIDER.getOrCreate(state).getUserTimestampsEnabled(),
                     "User provided timestamps (USING TIMESTAMP)");
 
+    public static final DisableFlag groupByEnabled =
+    new DisableFlag("group_by",
+                    state -> !CONFIG_PROVIDER.getOrCreate(state).getGroupByEnabled(),
+                    "GROUP BY functionality");

Review Comment:
   for me: `GROUP BY functionality is not allowed` will be the error message



-- 
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