maytasm commented on code in PR #18949:
URL: https://github.com/apache/druid/pull/18949#discussion_r2744785717
##########
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/StringSqlAggregator.java:
##########
Review Comment:
Can you change this to NAME too
##########
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchBaseSqlAggregator.java:
##########
@@ -52,6 +53,7 @@
public abstract class HllSketchBaseSqlAggregator implements SqlAggregator
{
private static final boolean ROUND = true;
+ private static final String NAME = "APPROX_COUNT_DISTINCT_DS_HLL";
Review Comment:
This is an abstract class. The NAME is in the subclass (i.e.
HllSketchApproxCountDistinctUtf8SqlAggregator,
HllSketchApproxCountDistinctSqlAggregator, etc). The NAME is not always the
same depending on which subclass. This should be a abstract method and the
subclass can implement and return it's name
##########
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchApproxQuantileSqlAggregator.java:
##########
@@ -109,7 +110,7 @@ public Aggregation toDruidAggregation(
return null;
}
- k = ((Number) RexLiteral.value(resolutionArg)).intValue();
+ k =
DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(resolutionArg), NAME,
"resolution").intValue();
Review Comment:
nit: "k" to match doc
##########
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchBaseSqlAggregator.java:
##########
@@ -49,6 +50,8 @@
public abstract class ThetaSketchBaseSqlAggregator implements SqlAggregator
{
+ private static final String NAME = "APPROX_COUNT_DISTINCT_DS_THETA";
Review Comment:
same as above
##########
extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/sql/QuantileSqlAggregator.java:
##########
@@ -102,7 +103,7 @@ public Aggregation toDruidAggregation(
return null;
}
- resolution = ((Number) RexLiteral.value(resolutionArg)).intValue();
+ resolution =
DruidSqlParserUtils.getNumericLiteral(RexLiteral.value(resolutionArg), NAME,
"resolution").intValue();
Review Comment:
nit: "k" to match doc
--
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]
