LakshSingla commented on code in PR #15382:
URL: https://github.com/apache/druid/pull/15382#discussion_r1395187637


##########
docs/querying/query-from-deep-storage.md:
##########
@@ -89,10 +89,10 @@ Submitting a query from deep storage uses the same syntax 
as any other Druid SQL
 
 Generally, the request body fields are the same between the `sql` and 
`sql/statements` endpoints.
 
-There are additional context parameters for `sql/statements` specifically: 
+Apart from the context parameters mentioned 
[here](../multi-stage-query/reference.md#context-parameters) there are 
additional context parameters for `sql/statements` specifically: 

Review Comment:
   ```suggestion
   Apart from the context parameters mentioned 
[here](../multi-stage-query/reference.md#context-parameters) there are 
additional context parameters for `sql/statements`: 
   ```



##########
docs/querying/query-from-deep-storage.md:
##########
@@ -182,12 +182,14 @@ Only the user who submitted a query can retrieve the 
results for the query.
 Use the following endpoint to retrieve results:
 
 ```
-GET 
https://ROUTER:8888/druid/v2/sql/statements/QUERYID/results?page=PAGENUMBER&size=RESULT_SIZE&timeout=TIMEOUT_MS
+GET 
https://ROUTER:8888/druid/v2/sql/statements/QUERYID/results?page=PAGENUMBER&resultFormat=FORMAT
 ```
 
 Results are returned in JSON format.
 
-You can use the optional `page`, `size`, and `timeout` parameters to refine 
your results. You can retrieve the `page` information for your results by 
fetching the status of the completed query.
+You can use the optional `page`, `resultFormat` parameters to refine your 
results. 
+* You can retrieve the `page` information for your results by fetching the 
status of the completed query.
+* For `resultFormat` the following options are supported 
`arrayLines`,`objectLines`,`array`,`object`,`csv`. Default value is `object`. 
More documentation present [here](../api-reference/sql-api.md#request-body). 

Review Comment:
   ```suggestion
   * For `resultFormat` the following options are supported 
`arrayLines`,`objectLines`,`array`,`object`, and `csv`. Default value is 
`object`. More documentation present 
[here](../api-reference/sql-api.md#request-body). 
   ```



##########
docs/api-reference/sql-api.md:
##########
@@ -390,12 +390,10 @@ Generally, the `sql` and `sql/statements` endpoints 
support the same response bo
 
 Keep the following in mind when submitting queries to the `sql/statements` 
endpoint:
 
-- There are additional context parameters  for `sql/statements`:
+- Apart from the context parameters mentioned 
[here](../multi-stage-query/reference.md#context-parameters) there are 
additional context parameters for `sql/statements` specifically:
 
    - `executionMode`  determines how query results are fetched. Druid 
currently only supports `ASYNC`. You must manually retrieve your results after 
the query completes.
-   - `selectDestination` determines where final results get written. By 
default, results are written to task reports. Set this parameter to 
`durableStorage` to instruct Druid to write the results from SELECT queries to 
durable storage, which allows you to fetch larger result sets. Note that this 
requires you to have [durable storage for MSQ 
enabled](../operations/durable-storage.md).
-
-- The only supported value for `resultFormat` is JSON LINES.
+   - `selectDestination` determines where final results get written. By 
default, results are written to task reports. Set this parameter to 
`durableStorage` to instruct Druid to write the results from SELECT queries to 
durable storage, which allows you to fetch larger result sets. For result sets 
with more than 3000 rows, it is highly recommended to use `durableStorage`. 
Note that this requires you to have [durable storage for MSQ 
enabled](../operations/durable-storage.md).

Review Comment:
   QQ - What's the significance of 3000 rows here?  Can we instead say it like: 
   `For result sets with many rows, xyz or so, ....` where xyz can be 3000, 
though a round number like 5000/10000 seems slightly better.
   A very minor nit, so we can leave it as is if not required.



##########
docs/api-reference/sql-api.md:
##########
@@ -390,12 +390,10 @@ Generally, the `sql` and `sql/statements` endpoints 
support the same response bo
 
 Keep the following in mind when submitting queries to the `sql/statements` 
endpoint:
 
-- There are additional context parameters  for `sql/statements`:
+- Apart from the context parameters mentioned 
[here](../multi-stage-query/reference.md#context-parameters) there are 
additional context parameters for `sql/statements` specifically:
 
    - `executionMode`  determines how query results are fetched. Druid 
currently only supports `ASYNC`. You must manually retrieve your results after 
the query completes.
-   - `selectDestination` determines where final results get written. By 
default, results are written to task reports. Set this parameter to 
`durableStorage` to instruct Druid to write the results from SELECT queries to 
durable storage, which allows you to fetch larger result sets. Note that this 
requires you to have [durable storage for MSQ 
enabled](../operations/durable-storage.md).
-
-- The only supported value for `resultFormat` is JSON LINES.
+   - `selectDestination` determines where final results get written. By 
default, results are written to task reports. Set this parameter to 
`durableStorage` to instruct Druid to write the results from SELECT queries to 
durable storage, which allows you to fetch larger result sets. For result sets 
with more than 3000 rows, it is highly recommended to use `durableStorage`. 
Note that this requires you to have [durable storage for MSQ 
enabled](../operations/durable-storage.md).

Review Comment:
   ```suggestion
      - `selectDestination` determines where final results get written. By 
default, results are written to task reports. Set this parameter to 
`durableStorage` to instruct Druid to write the results from SELECT queries to 
durable storage, which allows you to fetch larger result sets. For result sets 
with more than 3000 rows, it is highly recommended to use `durableStorage`. 
Note that this requires you to have [durable storage for 
MSQ](../operations/durable-storage.md) enabled.
   ```



##########
docs/api-reference/sql-api.md:
##########
@@ -827,6 +823,9 @@ When getting query results, keep the following in mind:
 * `page` (optional)
     * Type: Int
     * Refine paginated results

Review Comment:
   nit: what does refine mean? 



##########
docs/querying/query-from-deep-storage.md:
##########
@@ -182,12 +182,14 @@ Only the user who submitted a query can retrieve the 
results for the query.
 Use the following endpoint to retrieve results:
 
 ```
-GET 
https://ROUTER:8888/druid/v2/sql/statements/QUERYID/results?page=PAGENUMBER&size=RESULT_SIZE&timeout=TIMEOUT_MS
+GET 
https://ROUTER:8888/druid/v2/sql/statements/QUERYID/results?page=PAGENUMBER&resultFormat=FORMAT
 ```
 
 Results are returned in JSON format.
 
-You can use the optional `page`, `size`, and `timeout` parameters to refine 
your results. You can retrieve the `page` information for your results by 
fetching the status of the completed query.
+You can use the optional `page`, `resultFormat` parameters to refine your 
results. 

Review Comment:
   `resultFormat` doesn't refine the results I think
   ```suggestion
   You can use the optional `page` parameter to refine your results, and 
`resultFormat` parameter to define the format in which the results will be 
presented. 
   ```



##########
docs/api-reference/sql-api.md:
##########
@@ -827,6 +823,9 @@ When getting query results, keep the following in mind:
 * `page` (optional)
     * Type: Int
     * Refine paginated results
+* `resultFormat` (optional)
+    * Type: String
+    * Get results in different formats like 
`object`,`objectLines`,`array`,`arrayLines`,`csv`. Default is `object`.

Review Comment:
   Can we reword this like how it is done in `query-from-deep-storage.md`. That 
seems cleaner because it presents the options like an exhaustive list of the 
supported parameters
   ```suggestion
       * Defines the format in which the results are presented. The following 
options are supported `arrayLines`,`objectLines`,`array`,`object`, and `csv`. 
The default is `object`.
   ```



##########
docs/api-reference/sql-api.md:
##########
@@ -827,6 +823,9 @@ When getting query results, keep the following in mind:
 * `page` (optional)
     * Type: Int
     * Refine paginated results

Review Comment:
   Also, what's the default value of this parameter? 



-- 
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]

Reply via email to