yashmayya commented on code in PR #19216:
URL: https://github.com/apache/pinot/pull/19216#discussion_r3801067512


##########
pinot-common/src/main/java/org/apache/pinot/common/function/sql/PinotSqlFunction.java:
##########
@@ -28,12 +28,25 @@
 /// Pinot custom SqlFunction to be registered into SqlOperatorTable.
 public class PinotSqlFunction extends SqlFunction {
   private final boolean _deterministic;
+  private final boolean _volatile;
 
   public PinotSqlFunction(String name, SqlReturnTypeInference 
returnTypeInference,
-      SqlOperandTypeChecker operandTypeChecker, boolean deterministic) {
+      SqlOperandTypeChecker operandTypeChecker, boolean deterministic, boolean 
isVolatile) {

Review Comment:
   Can't use `volatile` for the parameter — it's a reserved keyword, so 
`boolean volatile` doesn't compile.
   
   Took what I think is the intent (parameter and field should match, like 
`deterministic`/`_deterministic`) and went the other way instead: renamed the 
field to `_isVolatile` so it pairs with the existing `isVolatile` parameter. 
`_isXxx` is already used for booleans elsewhere in the codebase. Done in 
68b1fc6.
   
   Happy to switch to something else (`volatileFunction`?) if you'd prefer.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to