Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Adarsh Sanjeev 2022-06-06 16:14:28 +05:30 committed by GitHub
parent abf0e0a159
commit 5a283964ca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 15 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidPlanner.java
View File

@ -689,12 +689,13 @@ public class DruidPlanner implements Closeable
*/ */
private String validateAndGetDataSourceForIngest(final SqlInsert insert) throws ValidationException private String validateAndGetDataSourceForIngest(final SqlInsert insert) throws ValidationException
{ {
final String operatorName = insert.getOperator().getName();
if (insert.isUpsert()) { if (insert.isUpsert()) {
throw new ValidationException("UPSERT is not supported."); throw new ValidationException("UPSERT is not supported.");
} }
if (insert.getTargetColumnList() != null) { 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(); final SqlIdentifier tableIdentifier = (SqlIdentifier) insert.getTargetTable();
@ -702,7 +703,7 @@ public class DruidPlanner implements Closeable
if (tableIdentifier.names.isEmpty()) { if (tableIdentifier.names.isEmpty()) {
// I don't think this can happen, but include a branch for it just in case. // 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) { } else if (tableIdentifier.names.size() == 1) {
// Unqualified name. // Unqualified name.
dataSource = Iterables.getOnlyElement(tableIdentifier.names); dataSource = Iterables.getOnlyElement(tableIdentifier.names);
@ -715,13 +716,13 @@ public class DruidPlanner implements Closeable
dataSource = tableIdentifier.names.get(1); dataSource = tableIdentifier.names.get(1);
} else { } else {
throw new ValidationException( 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 { try {
IdUtils.validateId("Ingestion dataSource", dataSource); IdUtils.validateId(operatorName + " dataSource", dataSource);
} }
catch (IllegalArgumentException e) { catch (IllegalArgumentException e) {
throw new ValidationException(e.getMessage()); throw new ValidationException(e.getMessage());

10
sql/src/test/java/org/apache/druid/sql/calcite/CalciteInsertDmlTest.java
View File

@ -133,7 +133,7 @@ public class CalciteInsertDmlTest extends CalciteIngestionDmlTest
{ {
testIngestionQuery() testIngestionQuery()
.sql("INSERT INTO \"in/valid\" SELECT dim1, dim2 FROM foo PARTITIONED BY ALL TIME") .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(); .verify();
} }
@ -142,7 +142,7 @@ public class CalciteInsertDmlTest extends CalciteIngestionDmlTest
{ {
testIngestionQuery() testIngestionQuery()
.sql("INSERT INTO dst (foo, bar) SELECT dim1, dim2 FROM foo PARTITIONED BY ALL TIME") .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(); .verify();
} }
@ -162,7 +162,7 @@ public class CalciteInsertDmlTest extends CalciteIngestionDmlTest
.sql("INSERT INTO INFORMATION_SCHEMA.COLUMNS SELECT * FROM foo PARTITIONED BY ALL TIME") .sql("INSERT INTO INFORMATION_SCHEMA.COLUMNS SELECT * FROM foo PARTITIONED BY ALL TIME")
.expectValidationError( .expectValidationError(
SqlPlanningException.class, 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(); .verify();
} }
@ -174,7 +174,7 @@ public class CalciteInsertDmlTest extends CalciteIngestionDmlTest
.sql("INSERT INTO view.aview SELECT * FROM foo PARTITIONED BY ALL TIME") .sql("INSERT INTO view.aview SELECT * FROM foo PARTITIONED BY ALL TIME")
.expectValidationError( .expectValidationError(
SqlPlanningException.class, 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(); .verify();
} }
@ -204,7 +204,7 @@ public class CalciteInsertDmlTest extends CalciteIngestionDmlTest
.sql("INSERT INTO nonexistent.dst SELECT * FROM foo PARTITIONED BY ALL TIME") .sql("INSERT INTO nonexistent.dst SELECT * FROM foo PARTITIONED BY ALL TIME")
.expectValidationError( .expectValidationError(
SqlPlanningException.class, 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(); .verify();
} }

10
sql/src/test/java/org/apache/druid/sql/calcite/CalciteReplaceDmlTest.java
View File

@ -379,7 +379,7 @@ public class CalciteReplaceDmlTest extends CalciteIngestionDmlTest
{ {
testIngestionQuery() testIngestionQuery()
.sql("REPLACE INTO \"in/valid\" OVERWRITE ALL SELECT dim1, dim2 FROM foo PARTITIONED BY ALL TIME") .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(); .verify();
} }
@ -388,7 +388,7 @@ public class CalciteReplaceDmlTest extends CalciteIngestionDmlTest
{ {
testIngestionQuery() testIngestionQuery()
.sql("REPLACE INTO dst (foo, bar) OVERWRITE ALL SELECT dim1, dim2 FROM foo PARTITIONED BY ALL TIME") .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(); .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") .sql("REPLACE INTO INFORMATION_SCHEMA.COLUMNS OVERWRITE ALL SELECT * FROM foo PARTITIONED BY ALL TIME")
.expectValidationError( .expectValidationError(
SqlPlanningException.class, 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(); .verify();
} }
@ -447,7 +447,7 @@ public class CalciteReplaceDmlTest extends CalciteIngestionDmlTest
.sql("REPLACE INTO view.aview OVERWRITE ALL SELECT * FROM foo PARTITIONED BY ALL TIME") .sql("REPLACE INTO view.aview OVERWRITE ALL SELECT * FROM foo PARTITIONED BY ALL TIME")
.expectValidationError( .expectValidationError(
SqlPlanningException.class, 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(); .verify();
} }
@ -477,7 +477,7 @@ public class CalciteReplaceDmlTest extends CalciteIngestionDmlTest
.sql("REPLACE INTO nonexistent.dst OVERWRITE ALL SELECT * FROM foo PARTITIONED BY ALL TIME") .sql("REPLACE INTO nonexistent.dst OVERWRITE ALL SELECT * FROM foo PARTITIONED BY ALL TIME")
.expectValidationError( .expectValidationError(
SqlPlanningException.class, 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(); .verify();
} }