This is an automated email from the ASF dual-hosted git repository.
kfaraz 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 5a283964ca Improve SQL validation error messages (#12611)
5a283964ca is described below
commit 5a283964cab8ec6d19b5130e458d6653062252fc
Author: Adarsh Sanjeev <[email protected]>
AuthorDate: Mon Jun 6 16:14:28 2022 +0530
Improve SQL validation error messages (#12611)
Update the SQL validation error message to specify whether
the ingest is INSERT or REPLACE for better user experience.
---
.../org/apache/druid/sql/calcite/planner/DruidPlanner.java | 9 +++++----
.../org/apache/druid/sql/calcite/CalciteInsertDmlTest.java | 10 +++++-----
.../org/apache/druid/sql/calcite/CalciteReplaceDmlTest.java | 10 +++++-----
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
index 01b7b03d57..ff139c7153 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
@@ -689,12 +689,13 @@ public class DruidPlanner implements Closeable
*/
private String validateAndGetDataSourceForIngest(final SqlInsert insert)
throws ValidationException
{
+ final String operatorName = insert.getOperator().getName();
if (insert.isUpsert()) {
throw new ValidationException("UPSERT is not supported.");
}
if (insert.getTargetColumnList() != null) {
- throw new ValidationException("Ingestion with target column list is not
supported.");
+ throw new ValidationException(operatorName + " with target column list
is not supported.");
}
final SqlIdentifier tableIdentifier = (SqlIdentifier)
insert.getTargetTable();
@@ -702,7 +703,7 @@ public class DruidPlanner implements Closeable
if (tableIdentifier.names.isEmpty()) {
// I don't think this can happen, but include a branch for it just in
case.
- throw new ValidationException("Ingestion requires target table.");
+ throw new ValidationException(operatorName + " requires target table.");
} else if (tableIdentifier.names.size() == 1) {
// Unqualified name.
dataSource = Iterables.getOnlyElement(tableIdentifier.names);
@@ -715,13 +716,13 @@ public class DruidPlanner implements Closeable
dataSource = tableIdentifier.names.get(1);
} else {
throw new ValidationException(
- StringUtils.format("Cannot ingest into [%s] because it is not a
Druid datasource.", tableIdentifier)
+ StringUtils.format("Cannot %s into [%s] because it is not a Druid
datasource.", operatorName, tableIdentifier)
);
}
}
try {
- IdUtils.validateId("Ingestion dataSource", dataSource);
+ IdUtils.validateId(operatorName + " dataSource", dataSource);
}
catch (IllegalArgumentException e) {
throw new ValidationException(e.getMessage());
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 e718cc2bdf..0a14ea3e53 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
@@ -133,7 +133,7 @@ public class CalciteInsertDmlTest extends
CalciteIngestionDmlTest
{
testIngestionQuery()
.sql("INSERT INTO \"in/valid\" SELECT dim1, dim2 FROM foo PARTITIONED
BY ALL TIME")
- .expectValidationError(SqlPlanningException.class, "Ingestion
dataSource cannot contain the '/' character.")
+ .expectValidationError(SqlPlanningException.class, "INSERT dataSource
cannot contain the '/' character.")
.verify();
}
@@ -142,7 +142,7 @@ public class CalciteInsertDmlTest extends
CalciteIngestionDmlTest
{
testIngestionQuery()
.sql("INSERT INTO dst (foo, bar) SELECT dim1, dim2 FROM foo
PARTITIONED BY ALL TIME")
- .expectValidationError(SqlPlanningException.class, "Ingestion with
target column list is not supported.")
+ .expectValidationError(SqlPlanningException.class, "INSERT with target
column list is not supported.")
.verify();
}
@@ -162,7 +162,7 @@ public class CalciteInsertDmlTest extends
CalciteIngestionDmlTest
.sql("INSERT INTO INFORMATION_SCHEMA.COLUMNS SELECT * FROM foo
PARTITIONED BY ALL TIME")
.expectValidationError(
SqlPlanningException.class,
- "Cannot ingest into [INFORMATION_SCHEMA.COLUMNS] because it is not
a Druid datasource."
+ "Cannot INSERT into [INFORMATION_SCHEMA.COLUMNS] because it is not
a Druid datasource."
)
.verify();
}
@@ -174,7 +174,7 @@ public class CalciteInsertDmlTest extends
CalciteIngestionDmlTest
.sql("INSERT INTO view.aview SELECT * FROM foo PARTITIONED BY ALL
TIME")
.expectValidationError(
SqlPlanningException.class,
- "Cannot ingest into [view.aview] because it is not a Druid
datasource."
+ "Cannot INSERT into [view.aview] because it is not a Druid
datasource."
)
.verify();
}
@@ -204,7 +204,7 @@ public class CalciteInsertDmlTest extends
CalciteIngestionDmlTest
.sql("INSERT INTO nonexistent.dst SELECT * FROM foo PARTITIONED BY ALL
TIME")
.expectValidationError(
SqlPlanningException.class,
- "Cannot ingest into [nonexistent.dst] because it is not a Druid
datasource."
+ "Cannot INSERT into [nonexistent.dst] because it is not a Druid
datasource."
)
.verify();
}
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 9c76cabfd8..fed8e0cf0c 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
@@ -379,7 +379,7 @@ public class CalciteReplaceDmlTest extends
CalciteIngestionDmlTest
{
testIngestionQuery()
.sql("REPLACE INTO \"in/valid\" OVERWRITE ALL SELECT dim1, dim2 FROM
foo PARTITIONED BY ALL TIME")
- .expectValidationError(SqlPlanningException.class, "Ingestion
dataSource cannot contain the '/' character.")
+ .expectValidationError(SqlPlanningException.class, "REPLACE dataSource
cannot contain the '/' character.")
.verify();
}
@@ -388,7 +388,7 @@ public class CalciteReplaceDmlTest extends
CalciteIngestionDmlTest
{
testIngestionQuery()
.sql("REPLACE INTO dst (foo, bar) OVERWRITE ALL SELECT dim1, dim2 FROM
foo PARTITIONED BY ALL TIME")
- .expectValidationError(SqlPlanningException.class, "Ingestion with
target column list is not supported.")
+ .expectValidationError(SqlPlanningException.class, "REPLACE with
target column list is not supported.")
.verify();
}
@@ -435,7 +435,7 @@ public class CalciteReplaceDmlTest extends
CalciteIngestionDmlTest
.sql("REPLACE INTO INFORMATION_SCHEMA.COLUMNS OVERWRITE ALL SELECT *
FROM foo PARTITIONED BY ALL TIME")
.expectValidationError(
SqlPlanningException.class,
- "Cannot ingest into [INFORMATION_SCHEMA.COLUMNS] because it is not
a Druid datasource."
+ "Cannot REPLACE into [INFORMATION_SCHEMA.COLUMNS] because it is
not a Druid datasource."
)
.verify();
}
@@ -447,7 +447,7 @@ public class CalciteReplaceDmlTest extends
CalciteIngestionDmlTest
.sql("REPLACE INTO view.aview OVERWRITE ALL SELECT * FROM foo
PARTITIONED BY ALL TIME")
.expectValidationError(
SqlPlanningException.class,
- "Cannot ingest into [view.aview] because it is not a Druid
datasource."
+ "Cannot REPLACE into [view.aview] because it is not a Druid
datasource."
)
.verify();
}
@@ -477,7 +477,7 @@ public class CalciteReplaceDmlTest extends
CalciteIngestionDmlTest
.sql("REPLACE INTO nonexistent.dst OVERWRITE ALL SELECT * FROM foo
PARTITIONED BY ALL TIME")
.expectValidationError(
SqlPlanningException.class,
- "Cannot ingest into [nonexistent.dst] because it is not a Druid
datasource."
+ "Cannot REPLACE into [nonexistent.dst] because it is not a Druid
datasource."
)
.verify();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
