gianm commented on a change in pull request #10198:
URL: https://github.com/apache/druid/pull/10198#discussion_r458377008



##########
File path: docs/querying/sql.md
##########
@@ -629,7 +629,9 @@ Druid SQL uses four different native query types.
 period)`, have no other grouping expressions, no HAVING or LIMIT clauses, no 
nesting, and either no ORDER BY, or an
 ORDER BY that orders by same expression as present in GROUP BY. It also uses 
Timeseries for "grand total" queries that
 have aggregation functions but no GROUP BY. This query type takes advantage of 
the fact that Druid segments are sorted
-by time.
+by time. The SQL planner can inject `timestampResultField` in the query 
context when there is a grouping key. This

Review comment:
       Hmm, I'm not sure that this additional content about 
`timestampResultField` is adding anything to the docs, really…
   
   I'm in favor of removing it for now, for the following reasons:
   
   1. This doesn't tell people much (it isn't clear what the parameter does, 
exactly).
   2. The parameter is an internal thing that we don't expect people to use 
directly.
   3. Mentioning it here distracts people from what they really need to know in 
this doc section, namely: learning what Druid SQL uses Timeseries queries for.
   
   It could make sense to include some content in the "Interpreting EXPLAIN 
PLAN output" section, though. But I'm not sure if it's worth it.




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

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