Adding support to execute functions during query compilation #5406
Conversation
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
Outdated
Show resolved
Hide resolved
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
Outdated
Show resolved
Hide resolved
| for (ScalarFunctionType value : values) { | ||
| String upperCaseFunctionName = value.getName().toUpperCase(); | ||
| _scalarFunctions.put(upperCaseFunctionName, value); | ||
| _scalarFunctions.put(upperCaseFunctionName.replace("_", ""), value); |
There was a problem hiding this comment.
What's the purpose of this? Flexibility to support variations of function names?
There was a problem hiding this comment.
e.g. presto use _ in function names. It provides more robustness from user perspective.
pinot-common/src/main/java/org/apache/pinot/common/function/ScalarFunctionType.java
Outdated
Show resolved
Hide resolved
|
|
||
| ScalarFunctionType scalarFunctionType = ScalarFunctionType.getScalarFunctionType(funcName); | ||
| switch (scalarFunctionType) { | ||
| case NOW: |
There was a problem hiding this comment.
Will this approach scale, as number of scalar functions increase? For example, each one would need to be added here. What do you think about modelling this as a query rewrite phase that goes over all scalars and evaluates them?
There was a problem hiding this comment.
I think we will need to move to function registry/invoker model
There was a problem hiding this comment.
yes, this is to just ensure that the logic of detection if a function can be evaluated at query compile time works.
|
For this PR, please focus on logic to figure out if any function can be evaluated at compile time. The evaluation itself will move into FunctionRegistry/Function Invoker |
xiangfu0
force-pushed
the
time-functions-in-query
branch
7 times, most recently
from
55c9814
to
1368749
Compare
| * @param funcExpr | ||
| * @return true if all arguments are literals | ||
| */ | ||
| private static boolean isCompileTimeEvaluationPossible(Expression funcExpr) { |
There was a problem hiding this comment.
Instead of this, why don't we just have a constant value function registry where all compile time evaluated functions are registered. So this check then becomes if the function is part of the constant value function registry.
There was a problem hiding this comment.
One thing here is that the function could be used in transform functions in query/ingestion field conversion. Ideally we should be able to evaluate any transform function with literal here.
xiangfu0
force-pushed
the
time-functions-in-query
branch
from
1368749
to
9a4e8ca
Compare
xiangfu0
requested review from
siddharthteotia,
Jackie-Jiang,
jackjlli and
mcvsubbu
pinot-common/src/main/java/org/apache/pinot/common/function/DateTimeFunctions.java
Outdated
Show resolved
Hide resolved
| if (funcSqlNode.getOperator().getKind() == SqlKind.OTHER_FUNCTION) { | ||
| funcName = funcSqlNode.getOperator().getName(); | ||
| } | ||
| if (funcName.equalsIgnoreCase(SqlKind.COUNT.toString()) && (funcSqlNode.getFunctionQuantifier() != null) |
There was a problem hiding this comment.
i didn't quite follow why there is special casing for DISTINCTCOUNT ?
There was a problem hiding this comment.
this is a special handling for the case of COUNT(DISTINCT A).
|
Will we eventually move towards also allowing functions which have column names? |
do you mean function name contains a column name? |
xiangfu0
force-pushed
the
time-functions-in-query
branch
from
9a4e8ca
to
cc89fe3
Compare
Adding support to execute functions during query compilation
pinot-coretopinot-commonNow that below queries are supported.
SELECT * FROM T where ts < now()SELECT * FROM T where date < toDateTime(now(), 'yyyy-MM-dd z')now()should be evaluated at the time of the query compilation. The logic introduced detects any function that hasThis PR introduces
now()anddatetimeFormat(String, String)as sample functions. There will be another PR that will add support for many other DateTime functions such as https://prestodb.io/docs/current/functions/datetime.html