[
https://issues.apache.org/jira/browse/CALCITE-5909?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17752849#comment-17752849
]
Ran Tao edited comment on CALCITE-5909 at 8/10/23 3:55 PM:
-----------------------------------------------------------
[~thomas.rebele] thanks for suggestions. however, this problem is not the
reason of ide, caching, multi-thread or data.
I checked this problem carefully and in-depth. In fact, this case is wrong
indeed. The reason why gradle build is normal is because this SqlParserTest was
moved into the src/main directory in calcite-1.28, that is, it will not be
executed as a unit test. But inheriting its subclasses such as BabelParserTest,
ServerParserTest can execute it indirectly. But in calcite-1.28, for this case,
it is restricted and cannot be executed in subclasses, that is, it cannot be
included in the build or test phase in the end. Here are some important changes:
*<= calcite-1.27.0:* SqlParserTest is in `core/test` dir (this case is right
and can be covered by gradle build process)
*calcite-1.28.0 ~ calcite-1.33.0:* move SqlParserTest to `testkit/main` dir
(SqlParserTest not in test dir but it can be coverd by BabelParserTest,
ServerParserTest indirectly. all cases in SqlParserTest run success except
testNoUnintendedNewReservedKeywords, because this case has
`not-run-for-subclasses` limitation, so it can't be covered in gradle build
process, but this case is right.)
*>= calcite-1.34.0:* this case is wrong, because the keywords are changed in
these released version, but not update the static field of
SqlParserTest.KEYWORDS. because of the reason above we can't found this failure
in the build or ci process.
The solution:
1. fix this case. I have opened a
[PR|https://github.com/apache/calcite/pull/3361] to fix it. but
[calcite-5564|https://issues.apache.org/jira/browse/CALCITE-5564] introduce
PERCENTILE_CONT/PERCENTILE_DISC，to let it be NonReservedKeywords, however
PERCENTILE_CONT/PERCENTILE_DISC is reserved keywords from sql-2008. it blocked
my PR.
2.Only fix this case is not enough, it will be failed if we not cover it in
calcite build process. I have try 2 ways. first way is re-move SqlParserTest to
test directory, it's very difficult, because many src/main classes use it. If
move it to test dir, these classes can not reference it. second way is remove
the `not-run-for-subclasses` this limitation, it meets some other problems,
however, it looks like can be solved (I hope get some feedbacks or contexts).
In terms of priority, I think PERCENTILE_CONT, PERCENTILE_DISC keywords need to
be solved first.
[~julianhyde] If you have time, pls help to check it. [~shenlang] thanks for
report this issue.
was (Author: lemonjing):
[~thomas.rebele] thanks for suggestions. however, this problem is not the
reason of ide, caching, multi-thread or data.
I checked this problem carefully and in-depth. In fact, this case is wrong
indeed. The reason why gradle build is normal is because this SqlParserTest was
moved into the src/main directory in calcite-1.28, that is, it will not be
executed as a unit test. But inheriting its subclasses such as BabelParserTest,
ServerParserTest can execute it indirectly. But in calcite-1.28, for this case,
it is restricted and cannot be executed in subclasses, that is, it cannot be
included in the build or test phase in the end. Here are some important changes:
*<= calcite-1.27.0:* SqlParserTest is in `core/test` dir (this case is right
and can be covered by gradle build process)
*calcite-1.28.0 ~ calcite-1.33.0:* move SqlParserTest to `testkit/main` dir
(SqlParserTest not in test dir but it can be coverd by BabelParserTest,
ServerParserTest indirectly. all cases in SqlParserTest run success except
testNoUnintendedNewReservedKeywords, because this case has
`not-run-for-subclasses` limitation, so it can't be covered in gradle build
process, but this case is right.)
*>= calcite-1.34.0:* this case is wrong, because the keywords are changed in
these released version, but not update the static field of
SqlParserTest.KEYWORDS. because of the reason above we can't found this failure
in the build or ci process.
The solution:
1. fix this case. I have opened a
[PR|https://github.com/apache/calcite/pull/3361] to fix it. but
calcite-5564introduce PERCENTILE_CONT/PERCENTILE_DISC，to let it be
NonReservedKeywords, however PERCENTILE_CONT/PERCENTILE_DISC is reserved
keywords from sql-2008. it blocked my PR.
2.Only fix this case is not enough, it will be failed if we not cover it in
calcite build process. I have try 2 ways. first way is re-move SqlParserTest to
test directory, it's very difficult, because many src/main classes use it. If
move it to test dir, these classes can not reference it. second way is remove
the `not-run-for-subclasses` this limitation, it meets some other problems,
however, it looks like can be solved (I hope get some feedbacks or contexts).
In terms of priority, I think PERCENTILE_CONT, PERCENTILE_DISC keywords need to
be solved first.
[~julianhyde] If you have time, pls help to check it. [~shenlang] thanks for
report this issue.
> Sometimes SqlParserTest.testNoUnintendedNewReservedKeywords fails in the IDE
> but passes when run from the command line
> ----------------------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-5909
> URL: https://issues.apache.org/jira/browse/CALCITE-5909
> Project: Calcite
> Issue Type: Bug
> Components: tests
> Reporter: LakeShen
> Priority: Minor
> Labels: pull-request-available
> Attachments: image-2023-08-08-23-32-55-466.png
>
>
> When I run the SqlParserTest,the testNoUnintendedNewReservedKeywords method
> failed,the exception like this:
> {code:java}
> java.lang.AssertionError: The parser has at least one new reserved keyword.
> Are you sure it should be reserved? Difference: {code}
> The picture like this:
> !image-2023-08-08-23-32-55-466.png|width=1543,height=496!
> I could fix this problem.More importantly, why is this method failing, but
> the Calcite pipeline is passing? I think we should look at something we missed
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
