dssysolyatin commented on code in PR #4620:
URL: https://github.com/apache/calcite/pull/4620#discussion_r2515800009
##########
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:
his test checks hint propagation. Your changes shouldn't affect anything
here. Double-check every Shuttle that extends RelShuttleImpl. There's probably
one in the test.
Also please add a note to the release notes for the next release. If someone
extends `RelShuttleImpl` and depends on `RelShuttleImpl.visit(RelNode other)`
for `Snapshot`, they will need to update their code to override the new method.
This is one of example of poor code design.
##########
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:
This test checks hint propagation. Your changes shouldn't affect anything
here. Double-check every Shuttle that extends RelShuttleImpl. There's probably
one in the test.
Also please add a note to the release notes for the next release. If someone
extends `RelShuttleImpl` and depends on `RelShuttleImpl.visit(RelNode other)`
for `Snapshot`, they will need to update their code to override the new method.
This is one of example of poor code design.
--
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]
