[
https://issues.apache.org/jira/browse/CALCITE-5727?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Benchao Li updated CALCITE-5727:
--------------------------------
Description:
This is discovered while I'm review another
[PR|https://github.com/apache/calcite/pull/3193#discussion_r1204998877]. And
there are already some tests that using {{{}checkUnchanged{}}}, but the xml
file still contains {{planAfter}} section, such as
{{{}RelOptRulesTest#testNoReduceAverage{}}}.
IMO, there is no need to keep {{planAfter}} for {{{}checkUnchanged{}}}, and it
do more harm than good to keep it. Even we can make {{planAfter}} the same with
{{{}planBefore{}}}, it will bring burden for reading (because we'll identify
the difference of planAfter and planBefore). And it will be more harmful when
{{planAfter}} is different from {{{}planBefore{}}}.
was:This is discovered while I'm review another
[PR|https://github.com/apache/calcite/pull/3193#discussion_r1204998877]. And
there are already some tests that using {{checkUnchanged}}, but the xml file
still contains {{planAfter}} section, such as
{{RelOptRulesTest#testNoReduceAverage}}.
> RelOptFixture#checkUnchanged should assert planAfter is not present
> -------------------------------------------------------------------
>
> Key: CALCITE-5727
> URL: https://issues.apache.org/jira/browse/CALCITE-5727
> Project: Calcite
> Issue Type: Improvement
> Components: tests
> Affects Versions: 1.34.0
> Reporter: Benchao Li
> Assignee: Benchao Li
> Priority: Minor
> Labels: pull-request-available
> Fix For: 1.35.0
>
>
> This is discovered while I'm review another
> [PR|https://github.com/apache/calcite/pull/3193#discussion_r1204998877]. And
> there are already some tests that using {{{}checkUnchanged{}}}, but the xml
> file still contains {{planAfter}} section, such as
> {{{}RelOptRulesTest#testNoReduceAverage{}}}.
> IMO, there is no need to keep {{planAfter}} for {{{}checkUnchanged{}}}, and
> it do more harm than good to keep it. Even we can make {{planAfter}} the same
> with {{{}planBefore{}}}, it will bring burden for reading (because we'll
> identify the difference of planAfter and planBefore). And it will be more
> harmful when {{planAfter}} is different from {{{}planBefore{}}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
