Skip to content

[multistage] add calcite function catalog#9375

Merged
walterddr merged 6 commits intoapache:masterfrom
walterddr:pr_add_function_catalog
Sep 12, 2022
Merged

[multistage] add calcite function catalog#9375
walterddr merged 6 commits intoapache:masterfrom
walterddr:pr_add_function_catalog

Conversation

@walterddr
Copy link
Contributor

@walterddr walterddr commented Sep 10, 2022

Description

FunctionRegistry currently hosts lots of scalar functions. there are not registered with the Calcite catalog reader.

this PR registers

  • the functions with Calcite function catalog and
  • chained SqlOperator table with user-defined functions
  • clean up some bugs

TODO

  • support multiple functions overlead with the same number of arguments, but different parameter type.
  • support type casting and implicit type hoisting correctly.
  • consolidate number-based lookup and operand types lookup

@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2022

Codecov Report

Merging #9375 (0a5e656) into master (ff0a353) will increase coverage by 0.09%.
The diff coverage is 87.50%.

@@             Coverage Diff              @@
##             master    #9375      +/-   ##
============================================
+ Coverage     69.70%   69.80%   +0.09%     
- Complexity     4786     5094     +308     
============================================
  Files          1885     1885              
  Lines        100377   100387      +10     
  Branches      15275    15276       +1     
============================================
+ Hits          69972    70079     +107     
+ Misses        25454    25357      -97     
  Partials       4951     4951              
Flag Coverage Δ
integration1 26.17% <31.25%> (+0.04%) ⬆️
integration2 24.80% <18.75%> (+0.10%) ⬆️
unittests1 66.97% <87.50%> (+0.01%) ⬆️
unittests2 15.35% <56.25%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...che/pinot/query/planner/logical/RexExpression.java 76.19% <ø> (-3.18%) ⬇️
...a/org/apache/pinot/query/catalog/PinotCatalog.java 50.00% <50.00%> (+6.25%) ⬆️
...apache/pinot/common/function/FunctionRegistry.java 86.84% <83.33%> (-0.66%) ⬇️
.../java/org/apache/pinot/query/QueryEnvironment.java 82.14% <100.00%> (+0.66%) ⬆️
...org/apache/pinot/query/context/PlannerContext.java 92.30% <100.00%> (ø)
...pinot/query/parser/CalciteRexExpressionParser.java 51.06% <100.00%> (+1.06%) ⬆️
...ache/pinot/query/planner/logical/StagePlanner.java 93.85% <100.00%> (ø)
...mmon/request/context/predicate/NotInPredicate.java 87.50% <0.00%> (-6.25%) ⬇️
...tream/kafka20/server/KafkaDataServerStartable.java 72.91% <0.00%> (-6.25%) ⬇️
.../predicate/NotEqualsPredicateEvaluatorFactory.java 74.03% <0.00%> (-5.77%) ⬇️
... and 28 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kishoreg
Copy link
Member

Does it handle the case if a function is overloaded?

@walterddr
Copy link
Contributor Author

walterddr commented Sep 10, 2022

Does it handle the case if a function is overloaded?

the limitation is still the same as current behavior

  • we can handle overload with different number of arguments.
  • we can't have 2 functions overloaded with the same number of arguments. (this is a limitation of FunctionRegistry, not Calcite function catalog)

@walterddr walterddr force-pushed the pr_add_function_catalog branch from fd59bfe to 9b95e89 Compare September 11, 2022 22:30
public static void registerFunction(Method method, boolean nullableParameters) {
registerFunction(method.getName(), method, nullableParameters);

// Calcite ScalarFunctionImpl doesn't allow customized named functions. TODO: fix me.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the fix here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will need to follow up items in #8597

@walterddr walterddr merged commit 987480b into apache:master Sep 12, 2022
@Jackie-Jiang Jackie-Jiang added the multi-stage Related to the multi-stage query engine label Sep 13, 2022
@walterddr walterddr deleted the pr_add_function_catalog branch December 6, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multi-stage Related to the multi-stage query engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants