mirror of https://github.com/apache/druid.git
Better error message for date_trunc operators (#15759)
IAEs are not bubbled up and show up as a runtime failure to the user which are not helpful. See https://apachedruidworkspace.slack.com/archives/C0303FDCZEZ/p1706185796975109 for one such example. This change will fix that.
This commit is contained in:
Abhishek Agarwal
GitHub
parent
f58fd5b75f
commit
989a8f7874
|
|
@ -19,6 +19,12 @@
|
|||
|
||||
package org.apache.druid.error;
|
||||
|
||||
/**
|
||||
* This exception class should be used instead of
|
||||
* {@link org.apache.druid.java.util.common.ISE} or {@link org.apache.druid.java.util.common.IAE} when processing is
|
||||
* to be halted during planning. There is wiring in place to bubble up the error message to the user when wrapped
|
||||
* in this exception class.
|
||||
*/
|
||||
public class InvalidSqlInput extends InvalidInput
|
||||
{
|
||||
public static DruidException exception(String msg, Object... args)
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ import org.apache.calcite.sql.SqlFunctionCategory;
|
|||
import org.apache.calcite.sql.SqlOperator;
|
||||
import org.apache.calcite.sql.type.SqlTypeFamily;
|
||||
import org.apache.calcite.sql.type.SqlTypeName;
|
||||
import org.apache.druid.java.util.common.IAE;
|
||||
import org.apache.druid.error.InvalidSqlInput;
|
||||
import org.apache.druid.java.util.common.StringUtils;
|
||||
import org.apache.druid.math.expr.Expr;
|
||||
import org.apache.druid.segment.column.RowSignature;
|
||||
|
|
@ -67,6 +67,7 @@ public class DateTruncOperatorConversion implements SqlOperatorConversion
|
|||
.operatorBuilder("DATE_TRUNC")
|
||||
.operandTypes(SqlTypeFamily.CHARACTER, SqlTypeFamily.TIMESTAMP)
|
||||
.requiredOperandCount(2)
|
||||
.literalOperands(0)
|
||||
.returnTypeCascadeNullable(SqlTypeName.TIMESTAMP)
|
||||
.functionCategory(SqlFunctionCategory.TIMEDATE)
|
||||
.build();
|
||||
|
|
@ -92,15 +93,15 @@ public class DateTruncOperatorConversion implements SqlOperatorConversion
|
|||
final DruidExpression arg = inputExpressions.get(1);
|
||||
final Expr truncTypeExpr = plannerContext.parseExpression(inputExpressions.get(0).getExpression());
|
||||
|
||||
if (!truncTypeExpr.isLiteral()) {
|
||||
throw new IAE("Operator[%s] truncType must be a literal", calciteOperator().getName());
|
||||
}
|
||||
|
||||
final String truncType = (String) truncTypeExpr.getLiteralValue();
|
||||
final Period truncPeriod = TRUNC_PERIOD_MAP.get(StringUtils.toLowerCase(truncType));
|
||||
|
||||
if (truncPeriod == null) {
|
||||
throw new IAE("Operator[%s] cannot truncate to[%s]", calciteOperator().getName(), truncType);
|
||||
throw InvalidSqlInput.exception(
|
||||
"Operator[%s] cannot truncate to[%s]",
|
||||
calciteOperator().getName(),
|
||||
truncType
|
||||
);
|
||||
}
|
||||
|
||||
return DruidExpression.ofFunctionCall(
|
||||
|
|
|
|||
|
|
@ -239,6 +239,17 @@ public abstract class QueryHandler extends SqlStatementHandler.BaseStatementHand
|
|||
if (de != null) {
|
||||
throw de;
|
||||
}
|
||||
|
||||
// Exceptions during rule evaluations could be wrapped inside a RuntimeException by VolcanoRuleCall class.
|
||||
// This block will extract a user-friendly message from the exception chain.
|
||||
if (e.getMessage() != null
|
||||
&& e.getCause() != null
|
||||
&& e.getCause().getMessage() != null
|
||||
&& e.getMessage().startsWith("Error while applying rule")) {
|
||||
throw DruidException.forPersona(DruidException.Persona.ADMIN)
|
||||
.ofCategory(DruidException.Category.UNCATEGORIZED)
|
||||
.build(e, "%s", e.getCause().getMessage());
|
||||
}
|
||||
throw DruidPlanner.translateException(e);
|
||||
}
|
||||
catch (Exception e) {
|
||||
|
|
@ -681,9 +692,6 @@ public abstract class QueryHandler extends SqlStatementHandler.BaseStatementHand
|
|||
private DruidException buildSQLPlanningError(RelOptPlanner.CannotPlanException exception)
|
||||
{
|
||||
String errorMessage = handlerContext.plannerContext().getPlanningError();
|
||||
if (null == errorMessage && exception instanceof UnsupportedSQLQueryException) {
|
||||
errorMessage = exception.getMessage();
|
||||
}
|
||||
if (errorMessage == null) {
|
||||
throw DruidException.forPersona(DruidException.Persona.OPERATOR)
|
||||
.ofCategory(DruidException.Category.UNSUPPORTED)
|
||||
|
|
|
|||
|
|
@ -1,38 +0,0 @@
|
|||
/*
|
||||
* Licensed to the Apache Software Foundation (ASF) under one
|
||||
* or more contributor license agreements. See the NOTICE file
|
||||
* distributed with this work for additional information
|
||||
* regarding copyright ownership. The ASF licenses this file
|
||||
* to you under the Apache License, Version 2.0 (the
|
||||
* "License"); you may not use this file except in compliance
|
||||
* with the License. You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
|
||||
package org.apache.druid.sql.calcite.planner;
|
||||
|
||||
import org.apache.calcite.plan.RelOptPlanner;
|
||||
import org.apache.druid.java.util.common.StringUtils;
|
||||
|
||||
/**
|
||||
* This class is different from {@link RelOptPlanner.CannotPlanException} in that the error
|
||||
* messages are user-friendly unlike its parent class. This exception class should be used instead of
|
||||
* {@link org.apache.druid.java.util.common.ISE} or {@link org.apache.druid.java.util.common.IAE} when processing is
|
||||
* to be halted during planning. Similarly, Druid planner can catch this exception and know that the error
|
||||
* can be directly exposed to end-users.
|
||||
*/
|
||||
public class UnsupportedSQLQueryException extends RelOptPlanner.CannotPlanException
|
||||
{
|
||||
public UnsupportedSQLQueryException(String formatText, Object... arguments)
|
||||
{
|
||||
super(StringUtils.nonStrictFormat(formatText, arguments));
|
||||
}
|
||||
}
|
||||
|
|
@ -14759,6 +14759,30 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
|
|||
)
|
||||
);
|
||||
}
|
||||
@Test
|
||||
public void testGroupByDateTrunc()
|
||||
{
|
||||
testQuery(
|
||||
"select DATE_TRUNC('HOUR', __time), COUNT(*) from druid.foo group by DATE_TRUNC('HOUR', __time)",
|
||||
ImmutableList.of(
|
||||
Druids.newTimeseriesQueryBuilder()
|
||||
.dataSource(CalciteTests.DATASOURCE1)
|
||||
.intervals(querySegmentSpec(Filtration.eternity()))
|
||||
.granularity(Granularities.HOUR)
|
||||
.aggregators(aggregators(new CountAggregatorFactory("a0")))
|
||||
.context(QUERY_CONTEXT_DEFAULT)
|
||||
.build()
|
||||
),
|
||||
ImmutableList.of(
|
||||
new Object[]{946684800000L, 1L},
|
||||
new Object[]{946771200000L, 1L},
|
||||
new Object[]{946857600000L, 1L},
|
||||
new Object[]{978307200000L, 1L},
|
||||
new Object[]{978393600000L, 1L},
|
||||
new Object[]{978480000000L, 1L}
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLatestByOnStringColumnWithoutMaxBytesSpecified()
|
||||
|
|
|
|||
|
|
@ -98,7 +98,6 @@ import org.apache.druid.sql.calcite.planner.PlannerConfig;
|
|||
import org.apache.druid.sql.calcite.planner.PlannerContext;
|
||||
import org.apache.druid.sql.calcite.planner.PlannerFactory;
|
||||
import org.apache.druid.sql.calcite.planner.PlannerResult;
|
||||
import org.apache.druid.sql.calcite.planner.UnsupportedSQLQueryException;
|
||||
import org.apache.druid.sql.calcite.run.NativeSqlEngine;
|
||||
import org.apache.druid.sql.calcite.schema.DruidSchemaCatalog;
|
||||
import org.apache.druid.sql.calcite.util.CalciteTestBase;
|
||||
|
|
@ -1399,12 +1398,12 @@ public class SqlResourceTest extends CalciteTestBase
|
|||
}
|
||||
|
||||
/**
|
||||
* This test is for {@link UnsupportedSQLQueryException} exceptions that are thrown by druid rules during query
|
||||
* This test is for {@link org.apache.druid.error.InvalidSqlInput} exceptions that are thrown by druid rules during query
|
||||
* planning. e.g. doing max aggregation on string type. The test checks that the API returns correct error messages
|
||||
* for such planning errors.
|
||||
*/
|
||||
@Test
|
||||
public void testCannotConvert_UnsupportedSQLQueryException() throws Exception
|
||||
public void testCannotConvert_InvalidSQL() throws Exception
|
||||
{
|
||||
// max(string) unsupported
|
||||
ErrorResponse errorResponse = postSyncForException(
|
||||
|
|
|
|||
Loading…
Reference in New Issue