Jackie-Jiang commented on code in PR #17208:
URL: https://github.com/apache/pinot/pull/17208#discussion_r2535831225
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -349,6 +350,15 @@ public FunctionInfo getFunctionInfo(int numArguments) {
return functionInfo != null ? functionInfo :
_functionInfoMap.get(VAR_ARG_KEY);
}
+ private boolean isDeterministic() {
+ for (FunctionInfo functionInfo : _functionInfoMap.values()) {
Review Comment:
This seems incorrect. E.g. `random()` is not deterministic, but `random(long
seed)` is. I don't see a good solution to it right now, but let's at least add
a TODO
##########
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java:
##########
@@ -2142,6 +2142,17 @@ public void testCompilationInvokedFunction() {
Assert.assertTrue(nowTs >= lowerBound);
Assert.assertTrue(nowTs <= upperBound);
+ query = "SELECT rand() FROM foo";
+ pinotQuery = compileToPinotQuery(query);
+ Expression randExpression = pinotQuery.getSelectList().get(0);
+ Assert.assertTrue(randExpression.isSetFunctionCall());
+ Assert.assertEquals(randExpression.getFunctionCall().getOperator(),
"rand");
+
Assert.assertTrue(randExpression.getFunctionCall().getOperands().isEmpty());
+
+ query = "SELECT rand(123) FROM foo";
+ pinotQuery = compileToPinotQuery(query);
Review Comment:
For MSE, will this fail to generate the constant?
##########
pinot-spi/src/main/java/org/apache/pinot/spi/annotations/ScalarFunction.java:
##########
@@ -62,5 +62,10 @@
*/
boolean isVarArg() default false;
+ /**
+ * Whether the scalar function should be treated as deterministic (eligible
for compile-time evaluation).
+ */
+ boolean deterministic() default true;
Review Comment:
`isDeterministic()` for consistency?
--
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]
