darpan-e6 commented on code in PR #5074:
URL: https://github.com/apache/calcite/pull/5074#discussion_r3541414683
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableAggregate.java:
##########
@@ -67,12 +67,6 @@ public EnumerableAggregate(
throw new InvalidRelException(
"within-distinct aggregation not supported");
}
- AggImplementor implementor2 =
- RexImpTable.INSTANCE.get(aggCall.getAggregation(), false);
- if (implementor2 == null) {
- throw new InvalidRelException(
- "aggregation " + aggCall.getAggregation() + " not supported");
Review Comment:
The check is still needed, but it needs to use the active
RexImplementorTable from the planner context rather than RexImpTable.INSTANCE.
I moved it to EnumerableAggregateRule.convert because implementor availability
is a convention-conversion feasibility check: if the active table cannot
implement an aggregate, the rule should return null and let the planner try
another path.
Keeping this in the EnumerableAggregate constructor would make the
constructor depend on planner-context lookup. The constructor now keeps only
table-independent structural checks such as DISTINCT / WITHIN DISTINCT. Code
generation later resolves through the same active table via
RexImplementorTables.of(cluster).
--
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]
