[GitHub] [pinot] snleee commented on a diff in pull request #8711: Disable Groovy function by default

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

snleee commented on code in PR #8711:
URL: https://github.com/apache/pinot/pull/8711#discussion_r874105775


##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -1033,8 +1033,8 @@ private HandlerContext getHandlerContext(@Nullable TableConfig offlineTableConfi
       }
     }
 
-    // Disable Groovy if either offline or realtime table config disables Groovy
-    boolean disableGroovy = offlineTableDisableGroovyQuery | realtimeTableDisableGroovyQuery;
+    // Disable Groovy if both offline and realtime table config disable Groovy

Review Comment:
   I will improve this logic in that case. I was trying to handle `offline/ realtime only case`.
   
   We need an extra `null` check for the table configs.
   
   1. If both offline/realtime exists, allow if both got enabled.
   2. if only one exists, allow if the specific table got enabled.



-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org