JingGe commented on code in PR #24317:
URL: https://github.com/apache/flink/pull/24317#discussion_r1495629504
##########
flink-table/flink-sql-parser/pom.xml:
##########
@@ -252,6 +252,7 @@ under the License.
it under ${project.build.directory} where all freemarker
templates are. -->
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
+ <!-- Uncomment it back once fix for
CALCITE-6266 comes to Flink with Calcite upgrade
Review Comment:
I would suggest creating a dedicated Flink jira ticket for the follow-up
cleanup task, since not only the pom but also the Parser.jj file need to be
reverted.
##########
flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/FlinkSqlParserImplTest.java:
##########
@@ -107,6 +107,10 @@ void testExplainAsDot() {}
void testStringAgg() {}
// END
+ @Disabled
Review Comment:
Would you like to help me understand the reason to add this test and
disabled?
##########
flink-table/flink-sql-parser/src/main/codegen/templates/Parser.jj:
##########
@@ -0,0 +1,8469 @@
+/*
Review Comment:
Keeping a copy of Parser.jj is very risky. Have you considered `Extending
the Parser`[1]?
[1] https://calcite.apache.org/docs/adapter.html#extending-the-parser
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
