somu-imply commented on code in PR #14534:
URL: https://github.com/apache/druid/pull/14534#discussion_r1254564338
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskQueryMaker.java:
##########
@@ -136,15 +136,24 @@ public QueryResponse<Object[]> runQuery(final DruidQuery
druidQuery)
.orElse(jsonMapper.writeValueAsString(DEFAULT_SEGMENT_GRANULARITY));
}
catch (JsonProcessingException e) {
- throw new IAE("Unable to deserialize the insert granularity. Please
retry the query with a valid "
- + "segment graularity");
+ // This isn't populated by the user in the query context. If there was
an issue with the query granularity
+ // entered by the user, it should have been flagged earlier
+ throw DruidException.forPersona(DruidException.Persona.DEVELOPER)
Review Comment:
Might be a good idea to change this to a DeveloperDefensiveException that
can be used at other places as well similar to Forbidden. But this is fine as
well
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/MSQTaskSqlEngine.java:
##########
@@ -62,6 +61,7 @@ public class MSQTaskSqlEngine implements SqlEngine
.addAll(NativeSqlEngine.SYSTEM_CONTEXT_PARAMETERS)
.add(MultiStageQueryContext.CTX_DESTINATION)
.add(QueryKitUtils.CTX_TIME_COLUMN_NAME)
+ .add(MSQTaskQueryMaker.USER_KEY)
Review Comment:
Is this intentional ?
##########
sql/src/main/java/org/apache/druid/sql/calcite/external/ExternalOperatorConversion.java:
##########
@@ -106,28 +106,72 @@ public ExternalTableSpec apply(
}
final RowSignature rowSignature;
if (columns != null) {
- rowSignature = Columns.convertSignature(columns);
+ try {
+ rowSignature = Columns.convertSignature(columns);
+ }
+ catch (IAE e) {
+ throw new ArgumentAndException("columns", e);
+ }
} else {
- rowSignature = jsonMapper.readValue(sigValue, RowSignature.class);
+ try {
+ rowSignature = jsonMapper.readValue(sigValue, RowSignature.class);
+ }
+ catch (JsonProcessingException e) {
+ throw new ArgumentAndException("rowSignature", e);
+ }
}
String inputSrcStr = CatalogUtils.getString(args, INPUT_SOURCE_PARAM);
- InputSource inputSource = jsonMapper.readValue(inputSrcStr,
InputSource.class);
+ InputSource inputSource;
+ try {
+ inputSource = jsonMapper.readValue(inputSrcStr, InputSource.class);
+ }
+ catch (JsonProcessingException e) {
+ throw new ArgumentAndException("rowSignature", e);
+ }
+ InputFormat inputFormat;
+ try {
+ inputFormat = jsonMapper.readValue(CatalogUtils.getString(args,
INPUT_FORMAT_PARAM), InputFormat.class);
+ }
+ catch (JsonProcessingException e) {
+ throw new ArgumentAndException("inputFormat", e);
+ }
return new ExternalTableSpec(
inputSource,
- jsonMapper.readValue(CatalogUtils.getString(args,
INPUT_FORMAT_PARAM), InputFormat.class),
+ inputFormat,
rowSignature,
inputSource::getTypes
);
}
- catch (JsonProcessingException e) {
- throw DruidException.forPersona(DruidException.Persona.USER)
- .ofCategory(DruidException.Category.INVALID_INPUT)
- .build(e, e.getMessage());
+ catch (ArgumentAndException e) { // We can triage out the error to one
of the argument passed to the EXTERN function
Review Comment:
This would require to update the test case in
`testErrorWithUnableToConstructColumnSignatureWithExtern` where the string
would change I believe
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/util/MultiStageQueryContext.java:
##########
@@ -197,10 +196,7 @@ public static int getMaxNumTasks(final QueryContext
queryContext)
public static Object getDestination(final QueryContext queryContext)
{
- return queryContext.get(
- CTX_DESTINATION,
- DEFAULT_DESTINATION
- );
+ return queryContext.get(CTX_DESTINATION);
Review Comment:
Was this intentional ?
--
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]
