gianm commented on code in PR #17937:
URL: https://github.com/apache/druid/pull/17937#discussion_r2053435592
##########
docs/api-reference/sql-api.md:
##########
@@ -99,6 +103,22 @@ The request body takes the following properties:
}
```
+##### Text Format Request body
+
+Druid also allows you to submit SQL queries in text format which is simpler
than above JSON format.
+To do this, just leave the `Content-Type` request header blank or set it to
anything other than `application/json`.
+If there are multiple `Content-Type` headers, the **first** one is used.
+
+In this format, the whole request body is treated as a single SQL query string.
+
+For response, the `resultFormat` is always `object` with the HTTP response
header `Content-Type: application/json`.
+
+If you want more control over the query context or response format, use the
above JSON format request body instead.
+
+```commandline
+echo 'SELECT 1' | curl -X POST "http://ROUTER_IP:ROUTER_PORT/druid/v2/sql";
--data @-
Review Comment:
When I do this example, I get a warning:
```
Note: Unnecessary use of -X or --request, POST is already inferred.
```
I think `-X POST` is not necessary if you also have `--data`.
##########
sql/src/main/java/org/apache/druid/sql/http/SqlQuery.java:
##########
@@ -200,4 +208,44 @@ public SqlQuery withQueryContext(Map<String, Object>
newContext)
{
return new SqlQuery(query, resultFormat, header, typesHeader,
sqlTypesHeader, newContext, parameters);
}
+
+ /**
+ * For BROKERs to use.
+ *
+ * Brokers use com.sun.jersey upon Jetty for RESTful API, however jersey
internally has sepcial handling for x-www-form-urlencoded,
+ * it's not able to get the data from the stream of HttpServletRequest for
such content type.
+ * So we use HttpContext to get the request entity/string instead of using
HttpServletRequest.
+ */
+ public static SqlQuery from(HttpContext httpContext)
+ {
+ MediaType contentType = httpContext.getRequest().getMediaType();
+ if (MediaType.APPLICATION_JSON_TYPE.equals(contentType)) {
+ return httpContext.getRequest().getEntity(SqlQuery.class);
+ } else {
Review Comment:
IMO, we should respect the content-type set by the user, which means this
branch should be `else if (MediaType.TEXT_PLAIN_TYPE.equals(contentType))`, and
the final `else` branch should be an error about the content type being
unsupported. Currently the `/druid/v2/sql` endpoint sends `415 Unsupported
Media Type` for anything other than `application/json`.
If we don't respect the content type, it will be difficult or impossible to
add support for other content types later. It will also be potentially
confusing, if a user expects us to handle a particular content type in the
expected way for that content type, rather than interpreting it as a SQL string.
To keep a nice curl integration, we could add a branch for
`application/x-www-form-urlencoded`, and urldecode the response before
interpreting it as a SQL query string. Users would need to do something like:
```
echo 'SELECT 1 = 1' | curl http://localhost:8888/druid/v2/sql/
--data-urlencode @-
```
Which we would get as:
```
SELECT+1+%3D+1%0A
```
This, I think, respects the integrity of the content type while also having
a nice curl integration.
##########
integration-tests/pom.xml:
##########
@@ -418,6 +418,16 @@
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpcore</artifactId>
+ <version>4.4.11</version>
Review Comment:
These are both present in the master pom, so you should be able to omit the
`<version>` to inherit the version from the main pom's `<dependencyManagement>`.
##########
sql/src/main/java/org/apache/druid/sql/http/SqlQuery.java:
##########
@@ -200,4 +208,44 @@ public SqlQuery withQueryContext(Map<String, Object>
newContext)
{
return new SqlQuery(query, resultFormat, header, typesHeader,
sqlTypesHeader, newContext, parameters);
}
+
+ /**
+ * For BROKERs to use.
+ *
+ * Brokers use com.sun.jersey upon Jetty for RESTful API, however jersey
internally has sepcial handling for x-www-form-urlencoded,
Review Comment:
"special" (spelling)
--
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]
