[multistage] clean up function signature registered into Calcite catalog
- Dominant language
- Java
- Stars
- 6.1k
- Forks
- 1.5k
- Avg merge
- 2d 55m
- Merged PRs (30d)
- 182
Description
follow up on #11726, several issues occurs to the function registries.
Goal
===
After this issue is fixed, any function should be able to appear in any part of the user SQL without the need to check whether it is a v1 function, a v2 function or supported by both (with some exceptions such as `arrayToMv` that only make sense on leaf)
Background
===
Planner
---
1. `PinotOperatorTable` is used to determine the operators supported by Pinot (Transform/Agg and other functions registered here)
2. `CalciteSchema` is built from `CalciteSchemaBuilder.asRootSchema` method, which reads `FunctionRegistry.FUNCTION_MAP` and registers them into calcite catalog
Runtime
---
1. `TransformFunctionFactory/Type` and `AggregateFunctionFactory/Type` provide concrete impls of the `SqlOperator` parsed out during the planner phase;
2. `FunctionRegistry.FUNCTION_INFO_MAP` provides concrete impls of the `SqlOperator` parsed out during the planner phase that are not covered by `TransformFunction`s
- as of now, there's no `TransformFunction` in the intermediate stage, so `FunctionRegistry.FUNCTION_INFO_MAP`, provides all the implementations.
Issue
===
during the planning phase, all the registered functions (transform/agg/scalar) are all parsed out uniformly and can be moved around stages, so the problem is
1. if a SqlOperator only exists in `TransformFunction` but not `ScalarFunction` impl, we cannot use that on non-leaf stages (majorly fixed by #11726)
2. if a SqlOperator is built-in to `SqlStdOperatorTable` we should not re-register the signature, instead we should just keep using the signature coming from Calcite
- we do today b/c we want to catch all the behavioral differences between us and Calcite early, but eventually we should support all behaviors unless there's a reason to override that
3. if the 2 implementations are different between transform and scalar, we should only register the intersection (to ensure that no matter how the planner choose to optimize the query, the concrete impl always exist on both transform and scalar)
- we do allow scalar function to have signature that transform doesn't have b/c of `ScalarTransformFunctionWrapper`, if we decided to do that then all transform function should have equivalent ScalarFunction annotated version (the otherway around is not necesarily true)
Tasks
===
- [ ] do not register elements that are already in SqlStdOperatorTable
- [ ] de-register those that are generated but user should never directly write (for example: `col > '123'` should not be allow explicitly rewritten as `greater_than(col, '123')` in user SQL)
- noted that this deregister only refers to the `PinotOperatorTable` and `CalciteSchema`, it should still register concrete impls to `FunctionRegistry` and `Transform/AggFunctionFactory`s
- [ ] check for dual register signature for `PinotOperatorTable` and `CalciteSchema`, they should not be allowed
Contributor guide
Assessment
This issue has not been assessed yet.