twalthr commented on code in PR #29333:
URL: https://github.com/apache/flink/pull/29333#discussion_r4192889591
##########
flink-table/flink-sql-gateway/src/test/java/org/apache/flink/table/gateway/service/application/ScriptExecutorITCase.java:
##########
@@ -151,7 +151,10 @@ void testParseStatementWithoutSemicolon() throws Exception
{
void testParseErrorPositionIsCorrect() throws Exception {
assertThat(runScript("error.q"))
.contains(
- "org.apache.flink.table.api.SqlParserException: SQL
parse failed. Encountered \")\" at line 26, column 1.");
+ "org.apache.flink.table.api.SqlParserException: SQL
parse failed. At line 26, column 1:\n"
Review Comment:
The error reason should still be close to the error message. The
visualization of the position is a nice to have, and since it spans multiple
lines, it should come last:
```
SQL parse failed. Encountered \")\" at line 26, column 1:
) WITH (
^
```
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/delegation/PlannerContext.java:
##########
@@ -172,11 +173,17 @@ public FlinkRelBuilder createRelBuilder() {
public FlinkPlannerImpl createFlinkPlanner() {
return new FlinkPlannerImpl(
- createFrameworkConfig(), this::createCatalogReader,
typeFactory, cluster);
+ createFrameworkConfig(),
+ createCalciteParser(),
Review Comment:
This seems to change the instance creation order. Can `createCalciteParser`
now be made private? Or are we instantiating the parser two times?
--
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]
