cryptoe commented on code in PR #14571:
URL: https://github.com/apache/druid/pull/14571#discussion_r1275748079
##########
sql/src/main/java/org/apache/druid/sql/http/SqlQuery.java:
##########
@@ -66,7 +66,7 @@ public SqlQuery(
)
{
this.query = Preconditions.checkNotNull(query, "query");
- this.resultFormat = resultFormat == null ? ResultFormat.OBJECT :
resultFormat;
+ this.resultFormat = resultFormat == null ?
ResultFormat.DEFAULT_RESULT_FORMAT : resultFormat;
Review Comment:
Why is this change needed ?
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/sql/resources/SqlStatementResource.java:
##########
@@ -164,6 +165,7 @@ public Response isEnabled(@Context final HttpServletRequest
request)
@Consumes(MediaType.APPLICATION_JSON)
public Response doPost(final SqlQuery sqlQuery, @Context final
HttpServletRequest req)
{
+ sqlQuery.getContext().put(MSQTaskQueryMaker.RESULT_FORMAT,
sqlQuery.getResultFormat().toString());
Review Comment:
Can we add a line here as to why we are doing this.
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/SqlMSQStatementResourcePostTest.java:
##########
@@ -327,12 +330,100 @@ public void testWithDurableStorage() throws IOException
Assert.assertEquals(rows,
SqlStatementResourceTest.getResultRowsFromResponse(resource.doGetResults(
sqlStatementResult.getQueryId(),
null,
+ null,
+ SqlStatementResourceTest.makeOkRequest()
+ )));
+
+ Assert.assertEquals(rows,
SqlStatementResourceTest.getResultRowsFromResponse(resource.doGetResults(
+ sqlStatementResult.getQueryId(),
+ 0L,
+ null,
+ SqlStatementResourceTest.makeOkRequest()
+ )));
+ }
+
+ @Test
+ public void testResultFormat() throws IOException
+ {
+ Map<String, Object> context = defaultAsyncContext();
+ context.put(MultiStageQueryContext.CTX_SELECT_DESTINATION,
MSQSelectDestination.DURABLESTORAGE.name());
+
+ SqlStatementResult sqlStatementResult = (SqlStatementResult)
resource.doPost(
+ new SqlQuery(
+ "select cnt,dim1 from foo",
+ ResultFormat.ARRAY,
Review Comment:
Lets add ut's with all result formats.
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/sql/SqlMSQStatementResourcePostTest.java:
##########
@@ -139,30 +140,32 @@ public void testMSQSelectQueryTest() throws IOException
@Test
public void nonSupportedModes()
{
-
SqlStatementResourceTest.assertExceptionMessage(
resource.doPost(new SqlQuery(
"select * from foo",
null,
false,
false,
false,
- ImmutableMap.of(),
Review Comment:
I think we should create a new sqlQuery object in the post request when you
are mutating the map. WDYT?
--
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]
