asolimando commented on code in PR #4303:
URL: https://github.com/apache/calcite/pull/4303#discussion_r2048347926
##########
core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java:
##########
@@ -288,8 +290,16 @@ private CoreRules() {}
/** Rule that pushes a {@link LogicalFilter}
* past a {@link LogicalTableFunctionScan}. */
public static final FilterTableFunctionTransposeRule
- FILTER_TABLE_FUNCTION_TRANSPOSE =
- FilterTableFunctionTransposeRule.Config.DEFAULT.toRule();
+ LOGICAL_FILTER_LOGICAL_TABLE_FUNCTION_TRANSPOSE = Config.DEFAULT
+ .withOperandFor(LogicalFilter.class, LogicalTableFunctionScan.class)
Review Comment:
Maybe I am missing something here, but both `LogicalFilter` and
`LogicalTableFunctionScan` are subclasses of `Filter` and `TableFunctionScan`
(respectively), why do we need to duplicate the rule matching condition here? I
don't recall us doing it for other Calcite rules which most probably have the
same problem (I recall a few changes lifting the Logical operator to the parent
class some time ago which didn't require any introduction of new methods nor
deprecating anything, but I can't find it at the moment).
If we provide the rule operand matching for `X` won't it work and get
applied to `LogicalX`? In that case you don't need to introduce the `LOGICAL_`
elements.
--
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]
