gianm commented on code in PR #13887:
URL: https://github.com/apache/druid/pull/13887#discussion_r1142887011


##########
docs/querying/sql-aggregations.md:
##########
@@ -139,6 +139,16 @@ Load the [DataSketches 
extension](../development/extensions-core/datasketches-ex
 |`DS_QUANTILES_SKETCH(expr, [k])`|Creates a [Quantiles 
sketch](../development/extensions-core/datasketches-quantiles.md) on the values 
of `expr`, which can be a regular column or a column containing quantiles 
sketches. The `k` parameter is described in the Quantiles sketch 
documentation.<br/><br/>See the [known 
issue](sql-translation.md#approximations) with this function.|`'0'` (STRING)|
 
 
+### Tuple sketch functions
+
+Load the [DataSketches 
extension](../development/extensions-core/datasketches-extension.md) to use the 
following functions.

Review Comment:
   IMO we don't need this description added here, since documentation already 
exists for tuple sketches generally: 
https://druid.apache.org/docs/latest/development/extensions-core/datasketches-tuple.html.
 Admittedly it could use improvement— they are quite terse when it comes to why 
you might want to use the sketches in the first place— but I'd consider that 
out of scope for a patch that is about to add SQL bindings.



##########
docs/querying/sql-aggregations.md:
##########
@@ -139,6 +139,16 @@ Load the [DataSketches 
extension](../development/extensions-core/datasketches-ex
 |`DS_QUANTILES_SKETCH(expr, [k])`|Creates a [Quantiles 
sketch](../development/extensions-core/datasketches-quantiles.md) on the values 
of `expr`, which can be a regular column or a column containing quantiles 
sketches. The `k` parameter is described in the Quantiles sketch 
documentation.<br/><br/>See the [known 
issue](sql-translation.md#approximations) with this function.|`'0'` (STRING)|
 
 
+### Tuple sketch functions
+
+Load the [DataSketches 
extension](../development/extensions-core/datasketches-extension.md) to use the 
following functions.

Review Comment:
   IMO we don't need this description added here, since documentation already 
exists for tuple sketches generally: 
https://druid.apache.org/docs/latest/development/extensions-core/datasketches-tuple.html.
 Admittedly it could use improvement— they are quite terse when it comes to why 
you might want to use the sketches in the first place— but I'd consider that 
out of scope for a patch that is about adding SQL bindings.



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