From 14c1aff150dd369a7b4fa87a64140ba2ed6c17f2 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Tue, 22 Aug 2023 12:47:49 +0200 Subject: [PATCH] Fix error messages relating to OVERWRITE keyword (#14870) OVERWRITE should not be a fully reserved keyword --- .../test/java/org/apache/druid/msq/exec/MSQReplaceTest.java | 3 ++- sql/src/main/codegen/config.fmpp | 4 ++++ sql/src/main/codegen/includes/replace.ftl | 5 ++++- .../org/apache/druid/sql/calcite/CalciteReplaceDmlTest.java | 5 +++-- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQReplaceTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQReplaceTest.java index f951d24f6cd..80d6719f121 100644 --- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQReplaceTest.java +++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQReplaceTest.java @@ -329,7 +329,8 @@ public class MSQReplaceTest extends MSQTestBase .setExpectedDataSource("foo1") .setQueryContext(context) .setExpectedValidationErrorMatcher(invalidSqlContains( - "Incorrect syntax near the keyword 'OVERWRITE'" + "Missing time chunk information in OVERWRITE clause for REPLACE. " + + "Use OVERWRITE WHERE <__time based condition> or OVERWRITE ALL to overwrite the entire table." )) .verifyPlanningErrors(); } diff --git a/sql/src/main/codegen/config.fmpp b/sql/src/main/codegen/config.fmpp index 0dfdef46b2b..87195131b57 100644 --- a/sql/src/main/codegen/config.fmpp +++ b/sql/src/main/codegen/config.fmpp @@ -67,6 +67,10 @@ data: { "PARTITIONED" ] + nonReservedKeywordsToAdd: [ + "OVERWRITE" + ] + # List of methods for parsing custom SQL statements. # Return type of method implementation should be 'SqlNode'. # Example: SqlShowDatabases(), SqlShowTables(). diff --git a/sql/src/main/codegen/includes/replace.ftl b/sql/src/main/codegen/includes/replace.ftl index 1af45cb7698..f3ea3a56761 100644 --- a/sql/src/main/codegen/includes/replace.ftl +++ b/sql/src/main/codegen/includes/replace.ftl @@ -43,7 +43,10 @@ SqlNode DruidSqlReplaceEof() : } ] [ - replaceTimeQuery = ReplaceTimeQuery() + + [ + replaceTimeQuery = ReplaceTimeQuery() + ] ] source = OrderedQueryOrExpr(ExprContext.ACCEPT_QUERY) // PARTITIONED BY is necessary, but is kept optional in the grammar. It is asserted that it is not missing in the diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteReplaceDmlTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteReplaceDmlTest.java index 9116e3e1211..2970330e82f 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteReplaceDmlTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteReplaceDmlTest.java @@ -440,8 +440,9 @@ public class CalciteReplaceDmlTest extends CalciteIngestionDmlTest { testIngestionQuery() .sql("REPLACE INTO dst OVERWRITE SELECT * FROM foo PARTITIONED BY ALL TIME") - .expectValidationError(invalidSqlContains( - "Incorrect syntax near the keyword 'OVERWRITE' at line 1, column 18." + .expectValidationError(invalidSqlIs( + "Missing time chunk information in OVERWRITE clause for REPLACE. " + + "Use OVERWRITE WHERE <__time based condition> or OVERWRITE ALL to overwrite the entire table." )) .verify(); }