[ 
https://issues.apache.org/jira/browse/CALCITE-7640?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Darpan Lunagariya (e6data computing) updated CALCITE-7640:
----------------------------------------------------------
    Description: 
h2. Background

CALCITE-7631 introduced {{RexImplementorTable}}, an interface for providing 
implementors for custom functions defined through a {{SqlOperatorTable}}. As 
part of that change, {{RexImpTable}} became the built-in implementation of 
{{RexImplementorTable}}.

The scalar code-generation path was updated to use this composable table. 
{{RexToLixTranslator}}, and therefore {{RexExecutorImpl}}, can now use a custom 
{{RexImplementorTable}} chained ahead of the built-ins.

The aggregate path was not updated. {{EnumerableAggregate}} and related 
aggregate code still resolve implementors directly from the {{RexImpTable}} 
singleton. Therefore, custom aggregate operators contributed through a 
{{SqlOperatorTable}} cannot be planned or executed by the enumerable engine 
unless they are known to the built-in table.

h2. Problem

The aggregate path consults the built-in singleton in two places:

* *Planning time*: the {{EnumerableAggregate}} constructor checks each 
{{AggregateCall}} against {{RexImpTable.INSTANCE}} and throws 
{{InvalidRelException}} if no implementor is found. {{EnumerableAggregateRule}} 
catches this and returns {{null}}, so aggregates unknown to the built-ins are 
rejected during planning.
* *Code-generation time*: {{AggImpState}} also resolves aggregate implementors 
from {{RexImpTable.INSTANCE}}.

Because both paths hard-code the singleton, a chained {{RexImplementorTable}} 
containing custom aggregate implementors is ignored.

Implementor availability is a planner-dependent decision. It depends on the 
active implementor table, which is available through the planner context, not 
through the {{EnumerableAggregate}} constructor alone.

h2. Proposed Change

Make aggregate implementor resolution use the composable 
{{RexImplementorTable}}, defaulting to the built-ins when no custom table is 
registered.

* Add {{RexImplementorTables.of(RelOptCluster)}}. It returns the 
{{RexImplementorTable}} registered in the planner {{Context}}, or 
{{RexImpTable.instance()}} if none is registered.
* Move the aggregate implementor availability check from the 
{{EnumerableAggregate}} constructor into {{EnumerableAggregateRule.convert}}, 
where the planner context is available. If the active table has no implementor, 
the rule returns {{null}} and the planner may try another convention.
* Keep only table-independent structural validation in the 
{{EnumerableAggregate}} constructor.
* Thread the resolved {{RexImplementorTable}} into aggregate code generation. 
{{AggImpState}} should receive the table from its callers, including 
{{EnumerableAggregate}}, {{EnumerableSortedAggregate}}, {{EnumerableWindow}}, 
and the interpreter's {{AggregateNode}}, using 
{{RexImplementorTables.of(getCluster())}}.

With this change, planning and code generation use the same implementor table. 
If the enumerable aggregate rule accepts an aggregate, code generation can 
resolve the same implementor.

{{RelOptCluster}} itself is not changed; it is only used to access the planner 
{{Context}}.

h3. Backward Compatibility

If no custom table is registered, {{RexImplementorTables.of(RelOptCluster)}} 
returns {{RexImpTable.instance()}}, so existing behavior is unchanged.

h2. Example

{code:sql}
SELECT g, MY_CUSTOM_AGG(x) AS c
FROM (VALUES ('a', 1), ('a', 2), ('b', 3)) AS t (g, x)
GROUP BY g
{code}

Here, {{MY_CUSTOM_AGG}} is contributed through a {{SqlOperatorTable}}, and its 
implementation is provided by a chained {{RexImplementorTable}}. This pattern 
can support custom aggregate functions such as {{APPROX_TOP_K}}, 
{{APPROX_COUNT_DISTINCT}}, and other approximate aggregates whose execution 
strategy is provided by the implementor.

h2. Testing

Add end-to-end enumerable tests for a custom aggregate implementor, asserting 
the actual result values.

The tests should cover:

* SQL execution through the {{Frameworks}} API.
* A directly built planner using a registered custom {{RexImplementorTable}}.
* A negative case where an unknown aggregate is rejected when no custom 
implementor table is registered.

  was:
h2. Background

CALCITE-7631 introduced {{RexImplementorTable}}, a composable SPI that is the 
code-generation counterpart of {{SqlOperatorTable}}, and threaded it through 
scalar code generation ({{RexToLixTranslator}}) and constant reduction 
({{RexExecutorImpl}}). An extension can now supply implementors for its own 
scalar operators and chain them ahead of the built-ins.

The aggregate path was left out of that change. {{EnumerableAggregate}} still 
resolves aggregate implementors only through the {{RexImpTable}} singleton, so 
a custom aggregate operator which is contributed through a {{SqlOperatorTable}} 
rather than registered as a schema function, cannot be planned or executed by 
the enumerable engine, even when a {{RexImplementorTable}} that knows it is in 
scope.

h2. Problem

The built-in singleton is consulted in two places on the aggregate path:

* *Planning time*: the {{EnumerableAggregate}} constructor pre-checks every 
{{AggregateCall}} against {{RexImpTable.INSTANCE}} and throws 
{{InvalidRelException}} when there is no implementor. 
{{EnumerableAggregateRule}} catches that and returns {{null}}, so an aggregate 
the built-ins do not know is rejected during planning (ultimately 
{{CannotPlanException}}).
* *Code-generation time*: {{AggImpState}} resolves the implementor from 
{{RexImpTable.INSTANCE}} as well.

Because both hard-code the singleton, a chained {{RexImplementorTable}} 
carrying a custom aggregate implementor is ignored. This is inconsistent with 
scalar operators, which CALCITE-7631 already made pluggable.

The deeper issue is that the constructor conflates two unrelated validations:
# table-independent structural checks ({{DISTINCT}} / {{WITHIN DISTINCT}} are 
never supported by the enumerable aggregate regardless of any table), which 
legitimately belong to the node.
# implementor availability against a specific registry, which is a planning 
decision and depends on which table is in play.

Only the second is the defect: hard-coding {{RexImpTable.INSTANCE}} for it 
bakes in "the built-in table is the only table"; the closed-world assumption 
CALCITE-7631 removed for scalars. (The sibling {{EnumerableSortedAggregate}} 
performs no such check in its constructor.)

h2. Proposed change

Resolve aggregate implementors through the composable table, defaulting to the 
built-ins:

* Add {{RexImplementorTables.of(RelOptCluster)}}, which returns the 
{{RexImplementorTable}} registered on the planner {{Context}}, or 
{{RexImpTable.instance()}} when none is registered -- the single place the 
default is named.
* Move the implementor-availability check out of the {{EnumerableAggregate}} 
constructor into {{EnumerableAggregateRule.convert}}, where the planner (hence 
the injected table) is available. The rule declines (returns {{null}}) when the 
active table has no implementor, so the planner can try another convention. The 
constructor keeps only the table-independent structural checks.
* Thread the resolved table into aggregate code generation: {{AggImpState}} 
takes a {{RexImplementorTable}}, supplied by its callers 
({{EnumerableAggregate}}, {{EnumerableSortedAggregate}}, {{EnumerableWindow}} 
and the interpreter's {{AggregateNode}}) from 
{{RexImplementorTables.of(getCluster())}}.

Both the planning gate and code generation then read the same table, so "the 
rule accepted this aggregate" implies "code generation can generate it". 
{{RelOptCluster}} itself is not changed; it is only used to reach the planner 
{{Context}}.

h3. Registration

An extension supplies the table on the planner {{Context}} once, at planner 
creation:

{code:java}
FrameworkConfig config = Frameworks.newConfigBuilder()
    .operatorTable(
        SqlOperatorTables.chain(SqlStdOperatorTable.instance(),
            SqlOperatorTables.of(myAggFunction)))
    .context(
        Contexts.of(
            RexImplementorTables.chain(customTable, RexImpTable.instance())))
    // ... rules, schema ...
    .build();
{code}

The same applies to a directly built {{VolcanoPlanner(Contexts.of(...))}} and 
to the JDBC path (a {{CalcitePrepareImpl}} subclass overriding 
{{createPlanner}} to populate {{externalContext}}, wired via 
{{Driver.withPrepareFactory}}).

h3. Backward compatibility

With no registration the accessor resolves to {{RexImpTable.instance()}}, so 
existing behaviour is unchanged. The former 3-argument {{AggImpState}} 
constructor is retained (deprecated) and delegates to the built-in table.

h2. Example

{code:sql}
SELECT g, MY_CUSTOM_AGG(x) AS c
FROM (VALUES ('a', 1), ('a', 2), ('b', 3)) AS t (g, x)
GROUP BY g
{code}

where {{MY_CUSTOM_AGG}} is contributed through a {{SqlOperatorTable}} and its 
implementor through a chained {{RexImplementorTable}}. Today this fails to 
plan; after the change it plans as an {{EnumerableAggregate}} and executes, 
returning {{(a, 2), (b, 1)}}.

h2. Testing

End-to-end enumerable execution of a custom aggregate implementor, asserting 
the actual result values -- driven both through the {{Frameworks}} API (real 
SQL) and a directly built planner -- plus the negative case: an unknown 
aggregate is rejected when no custom table is registered.


> Enumerable engine should execute aggregates whose implementor comes from a 
> custom RexImplementorTable"
> ------------------------------------------------------------------------------------------------------
>
>                 Key: CALCITE-7640
>                 URL: https://issues.apache.org/jira/browse/CALCITE-7640
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>    Affects Versions: 1.42.0
>            Reporter: Darpan Lunagariya (e6data computing)
>            Assignee: Darpan Lunagariya (e6data computing)
>            Priority: Minor
>              Labels: pull-request-available
>
> h2. Background
> CALCITE-7631 introduced {{RexImplementorTable}}, an interface for providing 
> implementors for custom functions defined through a {{SqlOperatorTable}}. As 
> part of that change, {{RexImpTable}} became the built-in implementation of 
> {{RexImplementorTable}}.
> The scalar code-generation path was updated to use this composable table. 
> {{RexToLixTranslator}}, and therefore {{RexExecutorImpl}}, can now use a 
> custom {{RexImplementorTable}} chained ahead of the built-ins.
> The aggregate path was not updated. {{EnumerableAggregate}} and related 
> aggregate code still resolve implementors directly from the {{RexImpTable}} 
> singleton. Therefore, custom aggregate operators contributed through a 
> {{SqlOperatorTable}} cannot be planned or executed by the enumerable engine 
> unless they are known to the built-in table.
> h2. Problem
> The aggregate path consults the built-in singleton in two places:
> * *Planning time*: the {{EnumerableAggregate}} constructor checks each 
> {{AggregateCall}} against {{RexImpTable.INSTANCE}} and throws 
> {{InvalidRelException}} if no implementor is found. 
> {{EnumerableAggregateRule}} catches this and returns {{null}}, so aggregates 
> unknown to the built-ins are rejected during planning.
> * *Code-generation time*: {{AggImpState}} also resolves aggregate 
> implementors from {{RexImpTable.INSTANCE}}.
> Because both paths hard-code the singleton, a chained {{RexImplementorTable}} 
> containing custom aggregate implementors is ignored.
> Implementor availability is a planner-dependent decision. It depends on the 
> active implementor table, which is available through the planner context, not 
> through the {{EnumerableAggregate}} constructor alone.
> h2. Proposed Change
> Make aggregate implementor resolution use the composable 
> {{RexImplementorTable}}, defaulting to the built-ins when no custom table is 
> registered.
> * Add {{RexImplementorTables.of(RelOptCluster)}}. It returns the 
> {{RexImplementorTable}} registered in the planner {{Context}}, or 
> {{RexImpTable.instance()}} if none is registered.
> * Move the aggregate implementor availability check from the 
> {{EnumerableAggregate}} constructor into {{EnumerableAggregateRule.convert}}, 
> where the planner context is available. If the active table has no 
> implementor, the rule returns {{null}} and the planner may try another 
> convention.
> * Keep only table-independent structural validation in the 
> {{EnumerableAggregate}} constructor.
> * Thread the resolved {{RexImplementorTable}} into aggregate code generation. 
> {{AggImpState}} should receive the table from its callers, including 
> {{EnumerableAggregate}}, {{EnumerableSortedAggregate}}, {{EnumerableWindow}}, 
> and the interpreter's {{AggregateNode}}, using 
> {{RexImplementorTables.of(getCluster())}}.
> With this change, planning and code generation use the same implementor 
> table. If the enumerable aggregate rule accepts an aggregate, code generation 
> can resolve the same implementor.
> {{RelOptCluster}} itself is not changed; it is only used to access the 
> planner {{Context}}.
> h3. Backward Compatibility
> If no custom table is registered, {{RexImplementorTables.of(RelOptCluster)}} 
> returns {{RexImpTable.instance()}}, so existing behavior is unchanged.
> h2. Example
> {code:sql}
> SELECT g, MY_CUSTOM_AGG(x) AS c
> FROM (VALUES ('a', 1), ('a', 2), ('b', 3)) AS t (g, x)
> GROUP BY g
> {code}
> Here, {{MY_CUSTOM_AGG}} is contributed through a {{SqlOperatorTable}}, and 
> its implementation is provided by a chained {{RexImplementorTable}}. This 
> pattern can support custom aggregate functions such as {{APPROX_TOP_K}}, 
> {{APPROX_COUNT_DISTINCT}}, and other approximate aggregates whose execution 
> strategy is provided by the implementor.
> h2. Testing
> Add end-to-end enumerable tests for a custom aggregate implementor, asserting 
> the actual result values.
> The tests should cover:
> * SQL execution through the {{Frameworks}} API.
> * A directly built planner using a registered custom {{RexImplementorTable}}.
> * A negative case where an unknown aggregate is rejected when no custom 
> implementor table is registered.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to