abhishekagarwal87 commented on code in PR #14483:
URL: https://github.com/apache/druid/pull/14483#discussion_r1246192197
##########
sql/src/main/java/org/apache/druid/sql/calcite/run/SqlResults.java:
##########
@@ -220,41 +221,51 @@ private static DateTime coerceDateTime(Object value,
SqlTypeName sqlType)
} else if (value instanceof DateTime) {
dateTime = (DateTime) value;
} else {
- throw cannotCoerce(value, sqlType);
+ throw cannotCoerce(value, sqlType, fieldName);
}
return dateTime;
}
private static String coerceUsingObjectMapper(
final ObjectMapper jsonMapper,
final Object value,
- final SqlTypeName sqlTypeName
+ final SqlTypeName sqlTypeName,
+ final String fieldName
)
{
try {
return jsonMapper.writeValueAsString(value);
}
catch (JsonProcessingException e) {
- throw cannotCoerce(e, value, sqlTypeName);
+ throw cannotCoerce(e, value, sqlTypeName, fieldName);
}
}
- private static IllegalStateException cannotCoerce(
+ private static DruidException cannotCoerce(
final Throwable t,
final Object value,
- final SqlTypeName sqlTypeName
+ final SqlTypeName sqlTypeName,
+ final String fieldName
)
{
- return new ISE(t, "Cannot coerce [%s] to [%s]", value == null ? "null" :
value.getClass().getName(), sqlTypeName);
+ return DruidException.forPersona(DruidException.Persona.DEVELOPER)
+ .ofCategory(DruidException.Category.INVALID_INPUT)
+ .build(
+ t,
+ "Cannot coerce field [%s] class of [%s] to [%s]",
+ fieldName,
+ value == null ? "null" :
value.getClass().getName(),
Review Comment:
```suggestion
value == null ? "unknown" :
value.getClass().getName(),
```
##########
sql/src/test/java/org/apache/druid/sql/calcite/run/SqlResultsTest.java:
##########
@@ -222,6 +224,17 @@ public void testCoerceString()
assertCannotCoerce(new Object(), SqlTypeName.VARCHAR);
}
+ @Test
+ public void testCoerceOfArrayOfPrimitives()
+ {
+ try {
+ assertCoerce("", new byte[1], SqlTypeName.BIGINT);
+ Assert.fail("Should throw an exception");
+ }
+ catch (Exception e) {
+ Assert.assertEquals("Cannot coerce field [fieldName] class of [[B] to
[BIGINT]", e.getMessage());
Review Comment:
weren't we going to replace `[[B]` with a more friendly name such as binary?
##########
sql/src/main/java/org/apache/druid/sql/calcite/run/SqlResults.java:
##########
@@ -220,41 +221,51 @@ private static DateTime coerceDateTime(Object value,
SqlTypeName sqlType)
} else if (value instanceof DateTime) {
dateTime = (DateTime) value;
} else {
- throw cannotCoerce(value, sqlType);
+ throw cannotCoerce(value, sqlType, fieldName);
}
return dateTime;
}
private static String coerceUsingObjectMapper(
final ObjectMapper jsonMapper,
final Object value,
- final SqlTypeName sqlTypeName
+ final SqlTypeName sqlTypeName,
+ final String fieldName
)
{
try {
return jsonMapper.writeValueAsString(value);
}
catch (JsonProcessingException e) {
- throw cannotCoerce(e, value, sqlTypeName);
+ throw cannotCoerce(e, value, sqlTypeName, fieldName);
}
}
- private static IllegalStateException cannotCoerce(
+ private static DruidException cannotCoerce(
final Throwable t,
final Object value,
- final SqlTypeName sqlTypeName
+ final SqlTypeName sqlTypeName,
+ final String fieldName
)
{
- return new ISE(t, "Cannot coerce [%s] to [%s]", value == null ? "null" :
value.getClass().getName(), sqlTypeName);
+ return DruidException.forPersona(DruidException.Persona.DEVELOPER)
+ .ofCategory(DruidException.Category.INVALID_INPUT)
+ .build(
+ t,
+ "Cannot coerce field [%s] class of [%s] to [%s]",
Review Comment:
How about including an action message here? @clintropolis - any suggestions?
##########
sql/src/main/java/org/apache/druid/sql/calcite/run/SqlResults.java:
##########
@@ -220,41 +221,51 @@ private static DateTime coerceDateTime(Object value,
SqlTypeName sqlType)
} else if (value instanceof DateTime) {
dateTime = (DateTime) value;
} else {
- throw cannotCoerce(value, sqlType);
+ throw cannotCoerce(value, sqlType, fieldName);
}
return dateTime;
}
private static String coerceUsingObjectMapper(
final ObjectMapper jsonMapper,
final Object value,
- final SqlTypeName sqlTypeName
+ final SqlTypeName sqlTypeName,
+ final String fieldName
)
{
try {
return jsonMapper.writeValueAsString(value);
}
catch (JsonProcessingException e) {
- throw cannotCoerce(e, value, sqlTypeName);
+ throw cannotCoerce(e, value, sqlTypeName, fieldName);
}
}
- private static IllegalStateException cannotCoerce(
+ private static DruidException cannotCoerce(
final Throwable t,
final Object value,
- final SqlTypeName sqlTypeName
+ final SqlTypeName sqlTypeName,
+ final String fieldName
)
{
- return new ISE(t, "Cannot coerce [%s] to [%s]", value == null ? "null" :
value.getClass().getName(), sqlTypeName);
+ return DruidException.forPersona(DruidException.Persona.DEVELOPER)
Review Comment:
Hmm. I would say that this is for user persona since its an invalid input.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
