frankgrimes97 commented on PR #13887: URL: https://github.com/apache/druid/pull/13887#issuecomment-1477790326
> The changes generally look good to me. I'd consider changing the names to start with `TUPLE_DOUBLES_` rather than `ARRAY_OF_DOUBLES_SKETCH_`. (Like `TUPLE_DOUBLES_SKETCH`, `TUPLE_DOUBLES_NOT`, etc.) It's a bit less typing and IMO clearer. > > Curious what people think. One issue with changing the naming only for the SQL functions is that it would be inconsistent with the naming in the native Druid functions and Apache Data Sketches codebase/documentation and so might lead to confusion. That issue aside, I do think the naming can be more terse without losing too much meaning. It might be worth noting that the naming of the Data Sketch SQL functions doesn't seem entirely consistent across the board. e.g. I see other sketch SQL functions use a `DS` prefix or suffix to designate them as Data Sketches: - `APPROX_COUNT_DISTINCT_DS_HLL` - `APPROX_COUNT_DISTINCT_DS_THETA` - `APPROX_QUANTILE_DS` - `DS_CDF` - `DS_GET_QUANTILE` - `DS_GET_QUANTILES` - `DS_HISTOGRAM` - `DS_HLL` - `DS_QUANTILE_SUMMARY` - `DS_QUANTILES_SKETCH` - `DS_RANK` - `DS_THETA` We could perhaps consider the following: - `DS_TUPLE_DOUBLES` - `DS_TUPLE_DOUBLES_INTERSECT` - `DS_TUPLE_DOUBLES_METRICS_SUM_ESTIMATE` - `DS_TUPLE_DOUBLES_NOT ` - `DS_TUPLE_DOUBLES_UNION` -- 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]
