This is an automated email from the ASF dual-hosted git repository.
abhishek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 5063eca5b9 Add error message for incorrectly ordered clause in sql
(#12558)
5063eca5b9 is described below
commit 5063eca5b9592215afc9913f091b2acc03a5797b
Author: Adarsh Sanjeev <[email protected]>
AuthorDate: Mon May 23 12:41:18 2022 +0530
Add error message for incorrectly ordered clause in sql (#12558)
In the case that the clustered by is before the partitioned by for an sql
query, the error message is a bit confusing.
insert into foo select * from bar clustered by dim1 partitioned by all
Error: SQL parse failed
Encountered "PARTITIONED" at line 1, column 88.
Was expecting one of: <EOF> "," ... "ASC" ... "DESC" ... "NULLS" ... "."
... "NOT" ... "IN" ... "<" ... "<=" ... ">" ... ">=" ... "=" ... "<>" ... "!="
... "BETWEEN" ... "LIKE" ... "SIMILAR" ... "+" ... "-" ... "*" ... "/" ... "%"
... "||" ... "AND" ... "OR" ... "IS" ... "MEMBER" ... "SUBMULTISET" ...
"CONTAINS" ... "OVERLAPS" ... "EQUALS" ... "PRECEDES" ... "SUCCEEDS" ...
"IMMEDIATELY" ... "MULTISET" ... "[" ... "FORMAT" ... "(" ... Less...
org.apache.calcite.sql.parser.SqlParseException
This is a bit confusing and adding a check could be added to throw a more
user friendly message stating that the order should be reversed.
Add error message for incorrectly ordered clause in sql.
---
sql/src/main/codegen/includes/insert.ftl | 5 +++++
sql/src/main/codegen/includes/replace.ftl | 5 +++++
.../apache/druid/sql/calcite/CalciteInsertDmlTest.java | 13 +++++++++++++
.../druid/sql/calcite/CalciteReplaceDmlTest.java | 18 ++++++++++++++++++
4 files changed, 41 insertions(+)
diff --git a/sql/src/main/codegen/includes/insert.ftl
b/sql/src/main/codegen/includes/insert.ftl
index 3eac7d19b2..07898aaadc 100644
--- a/sql/src/main/codegen/includes/insert.ftl
+++ b/sql/src/main/codegen/includes/insert.ftl
@@ -36,6 +36,11 @@ SqlNode DruidSqlInsertEof() :
<CLUSTERED> <BY>
clusteredBy = ClusterItems()
]
+ {
+ if (clusteredBy != null && partitionedBy.lhs == null) {
+ throw new ParseException("CLUSTERED BY found before PARTITIONED BY. In
druid, the CLUSTERED BY clause has to be specified after the PARTITIONED BY
clause");
+ }
+ }
// EOF is also present in SqlStmtEof but EOF is a special case and a single
EOF can be consumed multiple times.
// The reason for adding EOF here is to ensure that we create a
DruidSqlInsert node after the syntax has been
// validated and throw SQL syntax errors before performing validations in
the DruidSqlInsert which can overshadow the
diff --git a/sql/src/main/codegen/includes/replace.ftl
b/sql/src/main/codegen/includes/replace.ftl
index 20c9aac9a8..ed9eee46de 100644
--- a/sql/src/main/codegen/includes/replace.ftl
+++ b/sql/src/main/codegen/includes/replace.ftl
@@ -56,6 +56,11 @@ SqlNode DruidSqlReplaceEof() :
<CLUSTERED> <BY>
clusteredBy = ClusterItems()
]
+ {
+ if (clusteredBy != null && partitionedBy.lhs == null) {
+ throw new ParseException("CLUSTERED BY found before PARTITIONED BY.
In druid, the CLUSTERED BY clause has to be specified after the PARTITIONED BY
clause");
+ }
+ }
// EOF is also present in SqlStmtEof but EOF is a special case and a
single EOF can be consumed multiple times.
// The reason for adding EOF here is to ensure that we create a
DruidSqlReplace node after the syntax has been
// validated and throw SQL syntax errors before performing validations in
the DruidSqlReplace which can overshadow the
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java
index 87c389a722..e718cc2bdf 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java
@@ -348,6 +348,19 @@ public class CalciteInsertDmlTest extends
CalciteIngestionDmlTest
.verify();
}
+ @Test
+ public void testInsertWithoutPartitionedByWithClusteredBy()
+ {
+ testIngestionQuery()
+ .sql(
+ "INSERT INTO druid.dst "
+ + "SELECT __time, FLOOR(m1) as floor_m1, dim1, CEIL(m2) as ceil_m2
FROM foo "
+ + "CLUSTERED BY 2, dim1 DESC, CEIL(m2)"
+ )
+ .expectValidationError(SqlPlanningException.class, "CLUSTERED BY found
before PARTITIONED BY. In druid, the CLUSTERED BY clause has to be specified
after the PARTITIONED BY clause")
+ .verify();
+ }
+
@Test
public void testInsertWithPartitionedByAndClusteredBy()
{
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 6877ce8042..9c76cabfd8 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
@@ -392,6 +392,24 @@ public class CalciteReplaceDmlTest extends
CalciteIngestionDmlTest
.verify();
}
+ @Test
+ public void testReplaceWithoutPartitionedBy()
+ {
+ testIngestionQuery()
+ .sql("REPLACE INTO dst OVERWRITE ALL SELECT __time, FLOOR(m1) as
floor_m1, dim1 FROM foo")
+ .expectValidationError(SqlPlanningException.class, "REPLACE statements
must specify PARTITIONED BY clause explicitly")
+ .verify();
+ }
+
+ @Test
+ public void testReplaceWithoutPartitionedByWithClusteredBy()
+ {
+ testIngestionQuery()
+ .sql("REPLACE INTO dst OVERWRITE ALL SELECT __time, FLOOR(m1) as
floor_m1, dim1 FROM foo CLUSTERED BY dim1")
+ .expectValidationError(SqlPlanningException.class, "CLUSTERED BY found
before PARTITIONED BY. In druid, the CLUSTERED BY clause has to be specified
after the PARTITIONED BY clause")
+ .verify();
+ }
+
@Test
public void testReplaceWithoutOverwriteClause()
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
