imply-cheddar commented on code in PR #13564:
URL: https://github.com/apache/druid/pull/13564#discussion_r1049214948
##########
sql/src/main/java/org/apache/druid/sql/http/SqlResource.java:
##########
@@ -320,4 +181,207 @@ public Response cancelQuery(
return Response.status(Status.FORBIDDEN).build();
}
}
+
+ /**
+ * The SqlResource only generates metrics and doesn't keep track of
aggregate counts of successful/failed/interrupted
+ * queries, so this implementation is effectively just a noop.
+ */
+ private static class SqlResourceQueryMetricCounter implements
QueryResource.QueryMetricCounter
+ {
+ @Override
+ public void incrementSuccess()
+ {
+
+ }
+
+ @Override
+ public void incrementFailed()
+ {
+
+ }
+
+ @Override
+ public void incrementInterrupted()
+ {
+
+ }
+
+ @Override
+ public void incrementTimedOut()
+ {
+
+ }
+ }
+
+ private class SqlResourceResultPusher extends ResultPusher
+ {
+ private final String sqlQueryId;
+ private final HttpStatement stmt;
+ private final SqlQuery sqlQuery;
+
+ public SqlResourceResultPusher(
+ AsyncContext asyncContext,
+ String sqlQueryId,
+ HttpStatement stmt,
+ SqlQuery sqlQuery
+ )
+ {
+ super(
+ (HttpServletResponse) asyncContext.getResponse(),
+ SqlResource.this.jsonMapper,
+ SqlResource.this.responseContextConfig,
+ SqlResource.this.selfNode,
+ SqlResource.QUERY_METRIC_COUNTER,
+ sqlQueryId,
+ MediaType.APPLICATION_JSON_TYPE
+ );
+ this.sqlQueryId = sqlQueryId;
+ this.stmt = stmt;
+ this.sqlQuery = sqlQuery;
+ }
+
+ @Override
+ public ResultsWriter start()
+ {
+ return new ResultsWriter()
+ {
+ private ResultSet thePlan;
+
+ @Override
+ @Nullable
+ @SuppressWarnings({"unchecked", "rawtypes"})
+ public QueryResponse<Object> start(HttpServletResponse response)
+ {
+ response.setHeader(SQL_QUERY_ID_RESPONSE_HEADER, sqlQueryId);
+
+ final QueryResponse<Object[]> retVal;
+ try {
+ thePlan = stmt.plan();
+ retVal = thePlan.run();
+ }
+ catch (RelOptPlanner.CannotPlanException e) {
+ recordFailure(e);
+ final SqlPlanningException wrappedException = new
SqlPlanningException(
+ SqlPlanningException.PlanningError.UNSUPPORTED_SQL_ERROR,
+ e.getMessage()
+ );
+
+ writeErrorResponse(HttpServletResponse.SC_BAD_REQUEST, response,
wrappedException);
+ return null;
+ }
+ // There is a claim that Calcite sometimes throws a
java.lang.AssertionError, but we do not have a test that can
+ // reproduce it checked into the code (the best we have is something
that uses mocks to throw an Error, which is
+ // dubious at best). We keep this just in case, but it might be
best to remove it and see where the
+ // AssertionErrors are coming from and do something to ensure that
they don't actually make it out of Calcite
+ catch (AssertionError e) {
+ log.warn(e, "AssertionError killed query: %s", sqlQuery);
+ final QueryInterruptedException wrappedEx =
QueryInterruptedException.wrapIfNeeded(e);
Review Comment:
This is a hold-over from when this code caught a lot more stuff... I'll
switch it around to just do a `new QueryInterruptedException(e)` and work with
that.
--
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]
