dssysolyatin commented on PR #4620: URL: https://github.com/apache/calcite/pull/4620#issuecomment-3545763351
> Can we add a test to ensure that when a new RelNode is added @xuzifu666 I don’t think what you did in your last commit matches what @xiedeyantu meant. It is not so easy to write such test, although it should be possible using reflection. But before writing such a test, you need to clearly define the specification. For example, should `RelShuttle` handle both `TableFunctionScan` and `LogicalTableFunctionScan` in the same `visit(TableFunctionScan)` method (which already exists), or should `LogicalTableFunctionScan` have its own `visit(LogicalTableFunctionScan)` method? > as a temporary fix for the current omission Make sure we don’t add a new "temporary fix" with every release that includes breaking changes related to this topic. Based on HintCollector, I can say that there's an issue with `Window` and `TableFunctionScan`. For example, `RelShuttle.visit(TableFunctionScan)` exists, but `TableFunctionScan` doesn’t implement `accept` for it. So it falls back to `RelShuttle.visit(RelNode other)`. This is yet another breaking change. -- 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]
