Skip to content
Permalink
Browse files
Improve error messages from SQL REPLACE syntax (#12523)
- Add user friendly error messages for missing or incorrect OVERWRITE clause for REPLACE SQL query
- Move validation of missing OVERWRITE clause at code level instead of parser for custom error message
  • Loading branch information
adarshsanjeev committed May 17, 2022
1 parent fdfecfd commit 0fd4f1e3863f8f33bec69fc77dcb1ea118a42ed4
Showing 4 changed files with 28 additions and 6 deletions.
@@ -28,7 +28,7 @@ SqlNode DruidSqlReplaceEof() :
// Using fully qualified name for Pair class, since Calcite also has a same class name being used in the Parser.jj
org.apache.druid.java.util.common.Pair<Granularity, String> partitionedBy = new org.apache.druid.java.util.common.Pair(null, null);
final Pair<SqlNodeList, SqlNodeList> p;
final SqlNode replaceTimeQuery;
SqlNode replaceTimeQuery = null;
}
{
<REPLACE> { s = span(); }
@@ -41,8 +41,9 @@ SqlNode DruidSqlReplaceEof() :
}
}
]
<OVERWRITE>
replaceTimeQuery = ReplaceTimeQuery()
[
<OVERWRITE> 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
// DruidSqlInsert constructor so that we can return a custom error message.
@@ -61,7 +61,7 @@ public DruidSqlReplace(
@Nonnull SqlInsert insertNode,
@Nullable Granularity partitionedBy,
@Nullable String partitionedByStringForUnparse,
@Nonnull SqlNode replaceTimeQuery
@Nullable SqlNode replaceTimeQuery
) throws ParseException
{
super(
@@ -71,8 +71,11 @@ public DruidSqlReplace(
insertNode.getSource(),
insertNode.getTargetColumnList()
);
if (replaceTimeQuery == null) {
throw new ParseException("Missing time chunk information in OVERWRITE clause for REPLACE, set it to OVERWRITE WHERE <__time based condition> or set it to overwrite the entire table with OVERWRITE ALL.");
}
if (partitionedBy == null) {
throw new ParseException("REPLACE statements must specify PARTITIONED BY clause explictly");
throw new ParseException("REPLACE statements must specify PARTITIONED BY clause explicitly");
}
this.partitionedBy = partitionedBy;

@@ -899,7 +899,7 @@ static ParsedNodes handleReplace(SqlExplain explain, DruidSqlReplace druidSqlRep

SqlNode replaceTimeQuery = druidSqlReplace.getReplaceTimeQuery();
if (replaceTimeQuery == null) {
throw new ValidationException("Missing time chunk information in DELETE WHERE clause for replace.");
throw new ValidationException("Missing time chunk information in OVERWRITE clause for REPLACE, set it to OVERWRITE WHERE <__time based condition> or set it to overwrite the entire table with OVERWRITE ALL.");
}

Granularity ingestionGranularity = druidSqlReplace.getPartitionedBy();
@@ -368,6 +368,24 @@ public void testReplaceUsingColumnList()
.verify();
}

@Test
public void testReplaceWithoutOverwriteClause()
{
testIngestionQuery()
.sql("REPLACE INTO dst SELECT * FROM foo PARTITIONED BY ALL TIME")
.expectValidationError(SqlPlanningException.class, "Missing time chunk information in OVERWRITE clause for REPLACE, set it to OVERWRITE WHERE <__time based condition> or set it to overwrite the entire table with OVERWRITE ALL.")
.verify();
}

@Test
public void testReplaceWithoutCompleteOverwriteClause()
{
testIngestionQuery()
.sql("REPLACE INTO dst OVERWRITE SELECT * FROM foo PARTITIONED BY ALL TIME")
.expectValidationError(SqlPlanningException.class, "Missing time chunk information in OVERWRITE clause for REPLACE, set it to OVERWRITE WHERE <__time based condition> or set it to overwrite the entire table with OVERWRITE ALL.")
.verify();
}

@Test
public void testReplaceIntoSystemTable()
{

0 comments on commit 0fd4f1e

Please sign in to comment.