dssysolyatin commented on code in PR #4620:
URL: https://github.com/apache/calcite/pull/4620#discussion_r2517606385
##########
core/src/test/resources/org/apache/calcite/test/SqlHintsConverterTest.xml:
##########
@@ -373,7 +371,6 @@ TableScan:[[PROPERTIES inheritPath:[] options:{K1=v1,
K2=v2}], [PROPERTIES inher
<Resource name="hints">
<![CDATA[
Project:[[FAST_SNAPSHOT inheritPath:[] options:[PRODUCTS_TEMPORAL]]]
-Snapshot:[[FAST_SNAPSHOT inheritPath:[0, 1] options:[PRODUCTS_TEMPORAL]]]
Review Comment:
Undo your changes to
`core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java`,
because it already uses `instanceof Snapshot`. After that, it looks good to me.
Also it would be good to start a discussion about this compatibility issue
with the current `RexShuttle` (But I can do it by my own when I have time).
Personally, I feel that proper `RexShuttle` interface should have either
1. Have a single method RexShuttle.visit(RelNode other) OR
2. Have many specific methods like RelNode visit(Exchange exchange) and
RelNode visit(Sort sort),
but not both at the same time.
##########
core/src/test/resources/org/apache/calcite/test/SqlHintsConverterTest.xml:
##########
@@ -373,7 +371,6 @@ TableScan:[[PROPERTIES inheritPath:[] options:{K1=v1,
K2=v2}], [PROPERTIES inher
<Resource name="hints">
<![CDATA[
Project:[[FAST_SNAPSHOT inheritPath:[] options:[PRODUCTS_TEMPORAL]]]
-Snapshot:[[FAST_SNAPSHOT inheritPath:[0, 1] options:[PRODUCTS_TEMPORAL]]]
Review Comment:
Undo your changes to
`core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java`,
because it already uses `instanceof Snapshot`. After that, it looks good to me.
Also it would be good to start a discussion about this compatibility issue
with the current `RexShuttle` (But I can do it by my own when I have time).
Personally, I feel that proper `RexShuttle` interface should have either
1. Have a single method RelShuttle.visit(RelNode other) OR
2. Have many specific methods like RelNode visit(Exchange exchange) and
RelNode visit(Sort sort),
but not both at the same time.
--
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]
