This is an automated email from the ASF dual-hosted git repository.
FrankChen021 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new edcbfe742f1 fix: Validate the first operand of quantile aggregators
(#20782)
edcbfe742f1 is described below
commit edcbfe742f120b0b972da366899ed8479b021a79
Author: havefunatcode <[email protected]>
AuthorDate: Wed Nov 4 11:02:05 2026 +0900
fix: Validate the first operand of quantile aggregators (#20782)
* fix: Validate the first operand of quantile aggregators
APPROX_QUANTILE('m1', 0.5) with a single-quoted column name was planned
with the string literal as input and returned NaN or failed at runtime.
The quantile aggregators declared their first operand as ANY, and
DefaultOperandTypeChecker doesn't check ANY operands.
DefaultOperandTypeChecker gets two opt-in builder options:
operandComplexTypes lists the complex types an operand accepts, and
coercibleOperands lets a numeric operand take STRING input through an
implicit CAST. A string literal that can't be parsed as a number is
rejected with InvalidSqlInput before the cast is added. TIMESTAMP input
keeps working.
APPROX_QUANTILE, APPROX_QUANTILE_FIXED_BUCKETS, APPROX_QUANTILE_DS,
DS_QUANTILES_SKETCH, TDIGEST_QUANTILE and TDIGEST_GENERATE_SKETCH now
declare their first operand as NUMERIC and allow their own sketch type.
BOOLEAN, ARRAY, other complex types and NULL literals are rejected at
planning time.
toDruidExpressionForNumericAggregator also rejects non-numeric string
literals, for aggregators that still declare ANY.
Fixes #18010
* Use makeNullLiteral instead of deprecated constantNull in AggregationsTest
* Declare operand types with OperandType
Replace the operandComplexTypes() and coercibleOperands() builder
options with an OperandType value class, so each operand is declared
in one place, e.g.
OperandType.numeric().orComplex(type).coerceStrings()
DefaultOperandTypeChecker.Builder and
OperatorConversions.OperatorBuilder get an operandTypes(OperandType...)
overload, and the SqlTypeFamily overloads delegate to it.
coerceStrings() fails for non-NUMERIC operands.
* Reject orComplex() for ANY operands and add quantile tests
Add tests for numeric string literals in APPROX_QUANTILE_DS and for
string columns, numeric string literals and __time in
APPROX_QUANTILE_FIXED_BUCKETS.
* Keep accepting NULL for the first operand of quantile aggregators
Add OperandType.nullable() so that a required operand can accept a NULL
literal, and use it for the six quantile aggregators. Add tests for NULL
and CAST(NULL AS DOUBLE) inputs.
* Compute nullable operands in buildNullableOperands and fill test gaps
buildNullableOperands takes the operand types now and includes operands
declared with OperandType.nullable(). Add numeric string literal and __time
tests for APPROX_QUANTILE and TDIGEST_QUANTILE, and the same inputs plus
BOOLEAN, ARRAY and complex inputs for DS_QUANTILES_SKETCH and
TDIGEST_GENERATE_SKETCH.
* Update TDigest __time quantile for t-digest 3.3
master upgraded t-digest to 3.3 in #20765, which changes the median of
__time in testQuantileAndSketchOnTimeColumn. The sketch is the same.
---
.../sql/TDigestGenerateSketchSqlAggregator.java | 6 +-
.../sql/TDigestSketchQuantileSqlAggregator.java | 7 +-
.../sql/TDigestSketchSqlAggregatorTest.java | 268 ++++++++++++++++
.../DoublesSketchApproxQuantileSqlAggregator.java | 8 +-
.../sql/DoublesSketchObjectSqlAggregator.java | 6 +-
.../sql/DoublesSketchSqlAggregatorTest.java | 188 +++++++++++
...FixedBucketsHistogramQuantileSqlAggregator.java | 14 +-
.../histogram/sql/QuantileSqlAggregator.java | 10 +-
...dBucketsHistogramQuantileSqlAggregatorTest.java | 208 ++++++++++++
.../histogram/sql/QuantileSqlAggregatorTest.java | 169 ++++++++++
.../sql/calcite/aggregation/Aggregations.java | 41 ++-
.../expression/DefaultOperandTypeChecker.java | 105 +++++-
.../druid/sql/calcite/expression/OperandType.java | 147 +++++++++
.../calcite/expression/OperatorConversions.java | 35 +-
.../apache/druid/sql/calcite/planner/Calcites.java | 28 ++
.../sql/calcite/planner/DruidTypeCoercion.java | 19 ++
.../apache/druid/sql/calcite/CalciteQueryTest.java | 21 ++
.../sql/calcite/aggregation/AggregationsTest.java | 118 +++++++
.../sql/calcite/expression/OperandTypeTest.java | 114 +++++++
.../expression/OperatorConversionsTest.java | 357 ++++++++++++++++++++-
.../AssertionErrorOperatorConversion.java | 3 +-
.../numeric_agg_string_args.iq | 167 ++++++++++
22 files changed, 2001 insertions(+), 38 deletions(-)
diff --git
a/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java
b/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java
index cb879df275d..936078cc15c 100644
---
a/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java
+++
b/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestGenerateSketchSqlAggregator.java
@@ -38,6 +38,7 @@ import org.apache.druid.sql.calcite.aggregation.Aggregations;
import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
import org.apache.druid.sql.calcite.expression.DefaultOperandTypeChecker;
import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperandType;
import org.apache.druid.sql.calcite.planner.Calcites;
import org.apache.druid.sql.calcite.planner.PlannerContext;
import org.apache.druid.sql.calcite.rel.InputAccessor;
@@ -154,7 +155,10 @@ public class TDigestGenerateSketchSqlAggregator implements
SqlAggregator
DefaultOperandTypeChecker
.builder()
.operandNames("column", "compression")
- .operandTypes(SqlTypeFamily.ANY, SqlTypeFamily.NUMERIC)
+ .operandTypes(
+
OperandType.numeric().orComplex(TDigestSketchAggregatorFactory.TYPE).coerceStrings().nullable(),
+ OperandType.of(SqlTypeFamily.NUMERIC)
+ )
.requiredOperandCount(1)
.literalOperands(1)
.build(),
diff --git
a/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchQuantileSqlAggregator.java
b/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchQuantileSqlAggregator.java
index 613155e03c0..b780bf0f359 100644
---
a/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchQuantileSqlAggregator.java
+++
b/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchQuantileSqlAggregator.java
@@ -44,6 +44,7 @@ import org.apache.druid.sql.calcite.aggregation.Aggregations;
import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
import org.apache.druid.sql.calcite.expression.DefaultOperandTypeChecker;
import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperandType;
import org.apache.druid.sql.calcite.planner.PlannerContext;
import org.apache.druid.sql.calcite.rel.InputAccessor;
import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry;
@@ -181,7 +182,11 @@ public class TDigestSketchQuantileSqlAggregator implements
SqlAggregator
DefaultOperandTypeChecker
.builder()
.operandNames("column", "quantile", "compression")
- .operandTypes(SqlTypeFamily.ANY, SqlTypeFamily.NUMERIC,
SqlTypeFamily.NUMERIC)
+ .operandTypes(
+
OperandType.numeric().orComplex(TDigestSketchAggregatorFactory.TYPE).coerceStrings().nullable(),
+ OperandType.of(SqlTypeFamily.NUMERIC),
+ OperandType.of(SqlTypeFamily.NUMERIC)
+ )
.literalOperands(1, 2)
.requiredOperandCount(2)
.build(),
diff --git
a/extensions-contrib/tdigestsketch/src/test/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchSqlAggregatorTest.java
b/extensions-contrib/tdigestsketch/src/test/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchSqlAggregatorTest.java
index 48ae650c397..6e82d5caa37 100644
---
a/extensions-contrib/tdigestsketch/src/test/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchSqlAggregatorTest.java
+++
b/extensions-contrib/tdigestsketch/src/test/java/org/apache/druid/query/aggregation/tdigestsketch/sql/TDigestSketchSqlAggregatorTest.java
@@ -584,6 +584,274 @@ public class TDigestSketchSqlAggregatorTest extends
BaseCalciteQueryTest
);
}
+ @Test
+ public void testQuantileAndSketchOnStringColumn()
+ {
+ // string columns are implicitly cast to numbers, like
testQuantileOnCastedString
+ cannotVectorize();
+
+ testQuery(
+ "SELECT\n"
+ + " TDIGEST_QUANTILE(dim1, 0.5),\n"
+ + " TDIGEST_GENERATE_SKETCH(dim1, 200)\n"
+ + "FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "CAST(\"dim1\", 'DOUBLE')",
+ ColumnType.FLOAT,
+ ExprMacroTable.nil()
+ )
+ )
+ .aggregators(
+ new TDigestSketchAggregatorFactory("a0:agg", "v0", 100,
TDigestConfig.builder().build()),
+ new TDigestSketchAggregatorFactory("a1", "v0", 200,
TDigestConfig.builder().build())
+ )
+ .postAggregators(
+ new TDigestSketchToQuantilePostAggregator(
+ "a0",
+ new FieldAccessPostAggregator("a0:agg", "a0:agg"),
+ 0.5
+ )
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ResultMatchMode.EQUALS_EPS,
+ ImmutableList.of(
+ new Object[]{
+ 2.0,
+
"\"AAAAAT/wAAAAAAAAQCQzMzMzMzNAaQAAAAAAAAAAAAM/8AAAAAAAAD/wAAAAAAAAP/AAAAAAAABAAAAAAAAAAD/wAAAAAAAAQCQzMzMzMzM=\""
+ }
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileAndSketchOnNumericStringLiteral()
+ {
+ // string literals that can be parsed as numbers are accepted
+ cannotVectorize();
+
+ testQuery(
+ "SELECT\n"
+ + " TDIGEST_QUANTILE('1.5', 0.5),\n"
+ + " TDIGEST_GENERATE_SKETCH('1.5', 200)\n"
+ + "FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "1.5",
+ ColumnType.FLOAT,
+ ExprMacroTable.nil()
+ )
+ )
+ .aggregators(
+ new TDigestSketchAggregatorFactory("a0:agg", "v0", 100,
TDigestConfig.builder().build()),
+ new TDigestSketchAggregatorFactory("a1", "v0", 200,
TDigestConfig.builder().build())
+ )
+ .postAggregators(
+ new TDigestSketchToQuantilePostAggregator(
+ "a0",
+ new FieldAccessPostAggregator("a0:agg", "a0:agg"),
+ 0.5
+ )
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ResultMatchMode.EQUALS_EPS,
+ ImmutableList.of(
+ new Object[]{
+ 1.5,
+
"\"AAAAAT/4AAAAAAAAP/gAAAAAAABAaQAAAAAAAAAAAAY/8AAAAAAAAD/4AAAAAAAAP/AAAAAAAAA/+AAAAAAAAD/wAAAAAAAAP/gAAAAAAAA/8AAAAAAAAD/4AAAAAAAAP/AAAAAAAAA/+AAAAAAAAD/wAAAAAAAAP/gAAAAAAAA=\""
+ }
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileAndSketchOnTimeColumn()
+ {
+ // timestamps are used as numbers of milliseconds
+ cannotVectorize();
+
+ testQuery(
+ "SELECT\n"
+ + " TDIGEST_QUANTILE(__time, 0.5),\n"
+ + " TDIGEST_GENERATE_SKETCH(__time, 200)\n"
+ + "FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .granularity(Granularities.ALL)
+ .aggregators(
+ new TDigestSketchAggregatorFactory("a0:agg", "__time",
100, TDigestConfig.builder().build()),
+ new TDigestSketchAggregatorFactory("a1", "__time", 200,
TDigestConfig.builder().build())
+ )
+ .postAggregators(
+ new TDigestSketchToQuantilePostAggregator(
+ "a0",
+ new FieldAccessPostAggregator("a0:agg", "a0:agg"),
+ 0.5
+ )
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ResultMatchMode.EQUALS_EPS,
+ ImmutableList.of(
+ new Object[]{
+ 978307200000.0,
+
"\"AAAAAUJrjVn1gAAAQmx6Pn2AAABAaQAAAAAAAAAAAAY/8AAAAAAAAEJrjVn1gAAAP/AAAAAAAABCa43+wQAAAD/wAAAAAAAAQmuOo4yAAAA/8AAAAAAAAEJsePTmgAAAP/AAAAAAAABCbHmZsgAAAD/wAAAAAAAAQmx6Pn2AAAA=\""
+ }
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileAndSketchWithNonNumericStringLiteral()
+ {
+ // verify invalid queries return 400 (user error)
+ testQueryThrows(
+ "SELECT TDIGEST_QUANTILE('m1', 0.5) FROM foo",
+ invalidSqlContains(
+ "Function [TDIGEST_QUANTILE] argument [column] must be numeric,
got string literal ['m1']."
+ + " If you meant to refer to a column, use double quotes."
+ )
+ );
+ testQueryThrows(
+ "SELECT TDIGEST_GENERATE_SKETCH('m1') FROM foo",
+ invalidSqlContains(
+ "Function [TDIGEST_GENERATE_SKETCH] argument [column] must be
numeric, got string literal ['m1']."
+ + " If you meant to refer to a column, use double quotes."
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileOnNull()
+ {
+ // NULL inputs are accepted, the same as before the first operand was type
checked
+ cannotVectorize();
+ testQuery(
+ "SELECT TDIGEST_QUANTILE(NULL, 0.5), TDIGEST_QUANTILE(CAST(NULL AS
DOUBLE), 0.5) FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "null",
+ ColumnType.FLOAT,
+ ExprMacroTable.nil()
+ ),
+ new ExpressionVirtualColumn(
+ "v1",
+ "null",
+ ColumnType.FLOAT,
+ ExprMacroTable.nil()
+ )
+ )
+ .aggregators(ImmutableList.of(
+ new TDigestSketchAggregatorFactory("a0:agg", "v0", null,
TDigestConfig.builder().build()),
+ new TDigestSketchAggregatorFactory("a1:agg", "v1", null,
TDigestConfig.builder().build())
+ ))
+ .postAggregators(
+ new TDigestSketchToQuantilePostAggregator("a0",
makeFieldAccessPostAgg("a0:agg"), 0.5f),
+ new TDigestSketchToQuantilePostAggregator("a1",
makeFieldAccessPostAgg("a1:agg"), 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{Double.NaN, Double.NaN}
+ )
+ );
+ }
+
+ @Test
+ public void testGenerateSketchOnNull()
+ {
+ // NULL inputs are accepted, the same as before the first operand was type
checked
+ cannotVectorize();
+ testQuery(
+ "SELECT TDIGEST_GENERATE_SKETCH(NULL),
TDIGEST_GENERATE_SKETCH(CAST(NULL AS DOUBLE)) FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "null",
+ ColumnType.FLOAT,
+ ExprMacroTable.nil()
+ ),
+ new ExpressionVirtualColumn(
+ "v1",
+ "null",
+ ColumnType.FLOAT,
+ ExprMacroTable.nil()
+ )
+ )
+ .aggregators(ImmutableList.of(
+ new TDigestSketchAggregatorFactory("a0", "v0", null,
TDigestConfig.builder().build()),
+ new TDigestSketchAggregatorFactory("a1", "v1", null,
TDigestConfig.builder().build())
+ ))
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{"\"AAAAAX/wAAAAAAAA//AAAAAAAABAWQAAAAAAAAAAAAA=\"",
"\"AAAAAX/wAAAAAAAA//AAAAAAAABAWQAAAAAAAAAAAAA=\""}
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileAndSketchWithNonNumericInputTypes()
+ {
+ // verify invalid queries return 400 (user error)
+ testQueryThrows(
+ "SELECT TDIGEST_QUANTILE(m1 > 1, 0.5) FROM foo",
+ invalidSqlContains("Cannot apply 'TDIGEST_QUANTILE'")
+ );
+ testQueryThrows(
+ "SELECT TDIGEST_QUANTILE(ARRAY[1.0, 2.0], 0.5) FROM foo",
+ invalidSqlContains("Cannot apply 'TDIGEST_QUANTILE'")
+ );
+ testQueryThrows(
+ "SELECT TDIGEST_QUANTILE(PARSE_JSON('1'), 0.5) FROM foo",
+ invalidSqlContains("Cannot apply 'TDIGEST_QUANTILE'")
+ );
+ testQueryThrows(
+ "SELECT TDIGEST_GENERATE_SKETCH(m1 > 1) FROM foo",
+ invalidSqlContains("Cannot apply 'TDIGEST_GENERATE_SKETCH'")
+ );
+ testQueryThrows(
+ "SELECT TDIGEST_GENERATE_SKETCH(ARRAY[1.0, 2.0]) FROM foo",
+ invalidSqlContains("Cannot apply 'TDIGEST_GENERATE_SKETCH'")
+ );
+ testQueryThrows(
+ "SELECT TDIGEST_GENERATE_SKETCH(PARSE_JSON('1')) FROM foo",
+ invalidSqlContains("Cannot apply 'TDIGEST_GENERATE_SKETCH'")
+ );
+ }
+
private static PostAggregator makeFieldAccessPostAgg(String name)
{
return new FieldAccessPostAggregator(name, name);
diff --git
a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchApproxQuantileSqlAggregator.java
b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchApproxQuantileSqlAggregator.java
index 5038b722f7b..ccf3fe6aa93 100644
---
a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchApproxQuantileSqlAggregator.java
+++
b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchApproxQuantileSqlAggregator.java
@@ -32,6 +32,7 @@ import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.query.QueryContext;
import org.apache.druid.query.aggregation.AggregatorFactory;
import
org.apache.druid.query.aggregation.datasketches.quantiles.DoublesSketchAggregatorFactory;
+import
org.apache.druid.query.aggregation.datasketches.quantiles.DoublesSketchModule;
import
org.apache.druid.query.aggregation.datasketches.quantiles.DoublesSketchToQuantilePostAggregator;
import org.apache.druid.query.aggregation.post.FieldAccessPostAggregator;
import org.apache.druid.query.context.QueryContextParameters;
@@ -40,6 +41,7 @@ import org.apache.druid.sql.calcite.aggregation.Aggregation;
import org.apache.druid.sql.calcite.aggregation.Aggregations;
import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperandType;
import org.apache.druid.sql.calcite.expression.OperatorConversions;
import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
import org.apache.druid.sql.calcite.planner.PlannerContext;
@@ -61,7 +63,11 @@ public class DoublesSketchApproxQuantileSqlAggregator
implements SqlAggregator
private static final SqlAggFunction FUNCTION_INSTANCE =
OperatorConversions.aggregatorBuilder(NAME)
.operandNames("column", "probability", "k")
- .operandTypes(SqlTypeFamily.ANY,
SqlTypeFamily.NUMERIC, SqlTypeFamily.EXACT_NUMERIC)
+ .operandTypes(
+
OperandType.numeric().orComplex(DoublesSketchModule.TYPE).coerceStrings().nullable(),
+ OperandType.of(SqlTypeFamily.NUMERIC),
+ OperandType.of(SqlTypeFamily.EXACT_NUMERIC)
+ )
.returnTypeNonNull(SqlTypeName.DOUBLE)
.requiredOperandCount(2)
.literalOperands(1, 2)
diff --git
a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java
b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java
index 55973c093d6..25b0f8d2194 100644
---
a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java
+++
b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchObjectSqlAggregator.java
@@ -36,6 +36,7 @@ import org.apache.druid.sql.calcite.aggregation.Aggregation;
import org.apache.druid.sql.calcite.aggregation.Aggregations;
import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperandType;
import org.apache.druid.sql.calcite.expression.OperatorConversions;
import org.apache.druid.sql.calcite.planner.Calcites;
import org.apache.druid.sql.calcite.planner.PlannerContext;
@@ -51,7 +52,10 @@ public class DoublesSketchObjectSqlAggregator implements
SqlAggregator
private static final SqlAggFunction FUNCTION_INSTANCE =
OperatorConversions.aggregatorBuilder(NAME)
.operandNames("column", "k")
- .operandTypes(SqlTypeFamily.ANY,
SqlTypeFamily.EXACT_NUMERIC)
+ .operandTypes(
+
OperandType.numeric().orComplex(DoublesSketchModule.TYPE).coerceStrings().nullable(),
+ OperandType.of(SqlTypeFamily.EXACT_NUMERIC)
+ )
.returnTypeInference(
Calcites.complexReturnTypeWithNullability(DoublesSketchModule.TYPE, false)
)
diff --git
a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java
b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java
index a40875b3fb7..83c45afebf8 100644
---
a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java
+++
b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java
@@ -1141,6 +1141,194 @@ public class DoublesSketchSqlAggregatorTest extends
BaseCalciteQueryTest
}
}
+ @Test
+ public void testApproxQuantileWithNonNumericStringLiteral()
+ {
+ // verify invalid queries return 400 (user error)
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE_DS('m1', 0.99) FROM foo",
+ invalidSqlContains(
+ "Function [APPROX_QUANTILE_DS] argument [column] must be numeric,
got string literal ['m1']."
+ + " If you meant to refer to a column, use double quotes."
+ )
+ );
+ testQueryThrows(
+ "SELECT DS_QUANTILES_SKETCH('m1') FROM foo",
+ invalidSqlContains(
+ "Function [DS_QUANTILES_SKETCH] argument [column] must be numeric,
got string literal ['m1']."
+ + " If you meant to refer to a column, use double quotes."
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileOnNull()
+ {
+ // NULL inputs are accepted, the same as before the first operand was type
checked
+ testQuery(
+ "SELECT APPROX_QUANTILE_DS(NULL, 0.5), APPROX_QUANTILE_DS(CAST(NULL AS
DOUBLE), 0.5), DS_QUANTILES_SKETCH(NULL), DS_QUANTILES_SKETCH(CAST(NULL AS
DOUBLE)) FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "null",
+ ColumnType.FLOAT,
+ TestExprMacroTable.INSTANCE
+ )
+ )
+ .aggregators(ImmutableList.of(
+ new DoublesSketchAggregatorFactory("a0:agg", "v0", 128),
+ new DoublesSketchAggregatorFactory("a1", "v0", null,
null, false)
+ ))
+ .postAggregators(
+ new DoublesSketchToQuantilePostAggregator("a0",
makeFieldAccessPostAgg("a0:agg"), 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{Double.NaN, Double.NaN, "\"AQMIHoAAAAA=\"",
"\"AQMIHoAAAAA=\""}
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileAndSketchWithNonNumericInputTypes()
+ {
+ // verify invalid queries return 400 (user error)
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE_DS(m1 > 1, 0.99) FROM foo",
+ invalidSqlContains("Cannot apply 'APPROX_QUANTILE_DS'")
+ );
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE_DS(ARRAY[1.0, 2.0], 0.99) FROM foo",
+ invalidSqlContains("Cannot apply 'APPROX_QUANTILE_DS'")
+ );
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE_DS(PARSE_JSON('1'), 0.99) FROM foo",
+ invalidSqlContains("Cannot apply 'APPROX_QUANTILE_DS'")
+ );
+ testQueryThrows(
+ "SELECT DS_QUANTILES_SKETCH(m1 > 1) FROM foo",
+ invalidSqlContains("Cannot apply 'DS_QUANTILES_SKETCH'")
+ );
+ testQueryThrows(
+ "SELECT DS_QUANTILES_SKETCH(ARRAY[1.0, 2.0]) FROM foo",
+ invalidSqlContains("Cannot apply 'DS_QUANTILES_SKETCH'")
+ );
+ testQueryThrows(
+ "SELECT DS_QUANTILES_SKETCH(PARSE_JSON('1')) FROM foo",
+ invalidSqlContains("Cannot apply 'DS_QUANTILES_SKETCH'")
+ );
+ }
+
+ @Test
+ public void testQuantileOnStringColumn()
+ {
+ // string columns are implicitly cast to numbers, like
testQuantileOnCastedString
+ testQuery(
+ "SELECT APPROX_QUANTILE_DS(dim1, 0.5), DS_QUANTILES_SKETCH(dim1) FROM
foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "CAST(\"dim1\", 'DOUBLE')",
+ ColumnType.FLOAT,
+ TestExprMacroTable.INSTANCE
+ )
+ )
+ .aggregators(ImmutableList.of(
+ new DoublesSketchAggregatorFactory("a0:agg", "v0", 128),
+ new DoublesSketchAggregatorFactory("a1", "v0", null,
null, false)
+ ))
+ .postAggregators(
+ new DoublesSketchToQuantilePostAggregator("a0",
makeFieldAccessPostAgg("a0:agg"), 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{2.0,
"\"AgMIGoAAAAADAAAAAAAAAAAAAAAAAPA/MzMzMzMzJEAAAAAAAADwPwAAAAAAAABAMzMzMzMzJEA=\""}
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileOnTimeColumn()
+ {
+ // timestamps are used as numbers of milliseconds
+ testQuery(
+ "SELECT APPROX_QUANTILE_DS(__time, 0.5), DS_QUANTILES_SKETCH(__time)
FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .aggregators(ImmutableList.of(
+ new DoublesSketchAggregatorFactory("a0:agg", "__time",
128),
+ new DoublesSketchAggregatorFactory("a1", "__time", null,
null, false)
+ ))
+ .postAggregators(
+ new DoublesSketchToQuantilePostAggregator("a0",
makeFieldAccessPostAgg("a0:agg"), 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{
+ 946857600000.0,
+
"\"AgMIGoAAAAAGAAAAAAAAAAAAgPVZjWtCAACAfT56bEIAAID1WY1rQgAAAMH+jWtCAACAjKOOa0IAAIDm9HhsQgAAALKZeWxCAACAfT56bEI=\""
+ }
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileOnNumericStringLiteral()
+ {
+ // string literals that can be parsed as numbers are accepted
+ testQuery(
+ "SELECT APPROX_QUANTILE_DS('1.5', 0.5), DS_QUANTILES_SKETCH('1.5')
FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "1.5",
+ ColumnType.FLOAT,
+ TestExprMacroTable.INSTANCE
+ )
+ )
+ .aggregators(ImmutableList.of(
+ new DoublesSketchAggregatorFactory("a0:agg", "v0", 128),
+ new DoublesSketchAggregatorFactory("a1", "v0", null,
null, false)
+ ))
+ .postAggregators(
+ new DoublesSketchToQuantilePostAggregator("a0",
makeFieldAccessPostAgg("a0:agg"), 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{
+ 1.5,
+
"\"AgMIGoAAAAAGAAAAAAAAAAAAAAAAAPg/AAAAAAAA+D8AAAAAAAD4PwAAAAAAAPg/AAAAAAAA+D8AAAAAAAD4PwAAAAAAAPg/AAAAAAAA+D8=\""
+ }
+ )
+ );
+ }
+
private static PostAggregator makeFieldAccessPostAgg(String name)
{
return new FieldAccessPostAggregator(name, name);
diff --git
a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/sql/FixedBucketsHistogramQuantileSqlAggregator.java
b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/sql/FixedBucketsHistogramQuantileSqlAggregator.java
index eceaa7b8ad2..952db351d37 100644
---
a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/sql/FixedBucketsHistogramQuantileSqlAggregator.java
+++
b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/sql/FixedBucketsHistogramQuantileSqlAggregator.java
@@ -33,6 +33,7 @@ import org.apache.calcite.util.Optionality;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.query.aggregation.AggregatorFactory;
import org.apache.druid.query.aggregation.histogram.FixedBucketsHistogram;
+import
org.apache.druid.query.aggregation.histogram.FixedBucketsHistogramAggregator;
import
org.apache.druid.query.aggregation.histogram.FixedBucketsHistogramAggregatorFactory;
import org.apache.druid.query.aggregation.histogram.QuantilePostAggregator;
import org.apache.druid.segment.column.ColumnType;
@@ -41,6 +42,7 @@ import org.apache.druid.sql.calcite.aggregation.Aggregations;
import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
import org.apache.druid.sql.calcite.expression.DefaultOperandTypeChecker;
import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperandType;
import org.apache.druid.sql.calcite.planner.PlannerContext;
import org.apache.druid.sql.calcite.rel.InputAccessor;
import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry;
@@ -243,12 +245,12 @@ public class FixedBucketsHistogramQuantileSqlAggregator
implements SqlAggregator
"outlierHandlingMode"
)
.operandTypes(
- SqlTypeFamily.ANY,
- SqlTypeFamily.NUMERIC,
- SqlTypeFamily.NUMERIC,
- SqlTypeFamily.NUMERIC,
- SqlTypeFamily.NUMERIC,
- SqlTypeFamily.STRING
+
OperandType.numeric().orComplex(FixedBucketsHistogramAggregator.TYPE).coerceStrings().nullable(),
+ OperandType.of(SqlTypeFamily.NUMERIC),
+ OperandType.of(SqlTypeFamily.NUMERIC),
+ OperandType.of(SqlTypeFamily.NUMERIC),
+ OperandType.of(SqlTypeFamily.NUMERIC),
+ OperandType.of(SqlTypeFamily.STRING)
)
.literalOperands(1, 2, 3, 4, 5)
.requiredOperandCount(5)
diff --git
a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/sql/QuantileSqlAggregator.java
b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/sql/QuantileSqlAggregator.java
index 5dc39247753..7524ac59395 100644
---
a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/sql/QuantileSqlAggregator.java
+++
b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/sql/QuantileSqlAggregator.java
@@ -43,6 +43,7 @@ import org.apache.druid.sql.calcite.aggregation.Aggregations;
import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
import org.apache.druid.sql.calcite.expression.DefaultOperandTypeChecker;
import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperandType;
import org.apache.druid.sql.calcite.parser.DruidSqlParserUtils;
import org.apache.druid.sql.calcite.planner.PlannerContext;
import org.apache.druid.sql.calcite.rel.InputAccessor;
@@ -211,7 +212,14 @@ public class QuantileSqlAggregator implements SqlAggregator
DefaultOperandTypeChecker
.builder()
.operandNames("column", "probability", "resolution")
- .operandTypes(SqlTypeFamily.ANY, SqlTypeFamily.NUMERIC,
SqlTypeFamily.EXACT_NUMERIC)
+ .operandTypes(
+ OperandType.numeric()
+
.orComplex(ApproximateHistogramAggregatorFactory.TYPE)
+ .coerceStrings()
+ .nullable(),
+ OperandType.of(SqlTypeFamily.NUMERIC),
+ OperandType.of(SqlTypeFamily.EXACT_NUMERIC)
+ )
.requiredOperandCount(2)
.literalOperands(1, 2)
.build(),
diff --git
a/extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/sql/FixedBucketsHistogramQuantileSqlAggregatorTest.java
b/extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/sql/FixedBucketsHistogramQuantileSqlAggregatorTest.java
index 2268bb79c82..d4c100fc4a2 100644
---
a/extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/sql/FixedBucketsHistogramQuantileSqlAggregatorTest.java
+++
b/extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/sql/FixedBucketsHistogramQuantileSqlAggregatorTest.java
@@ -684,4 +684,212 @@ public class
FixedBucketsHistogramQuantileSqlAggregatorTest extends BaseCalciteQ
)
);
}
+
+ @Test
+ public void testQuantileOnStringColumn()
+ {
+ // string columns are implicitly cast to numbers, like
testQuantileOnCastedString
+ testQuery(
+ "SELECT APPROX_QUANTILE_FIXED_BUCKETS(dim1, 0.5, 20, 0.0, 10.0) FROM
foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "CAST(\"dim1\", 'DOUBLE')",
+ ColumnType.FLOAT,
+ TestExprMacroTable.INSTANCE
+ )
+ )
+ .aggregators(ImmutableList.of(
+ new FixedBucketsHistogramAggregatorFactory(
+ "a0:agg",
+ "v0",
+ 20,
+ 0.0d,
+ 10.0d,
+ FixedBucketsHistogram.OutlierHandlingMode.IGNORE,
+ false
+ )
+ ))
+ .postAggregators(
+ new QuantilePostAggregator("a0", "a0:agg", 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{1.5}
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileOnNumericStringLiteral()
+ {
+ // string literals that can be parsed as numbers are accepted
+ testQuery(
+ "SELECT APPROX_QUANTILE_FIXED_BUCKETS('1.5', 0.5, 20, 0.0, 10.0) FROM
foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "1.5",
+ ColumnType.FLOAT,
+ TestExprMacroTable.INSTANCE
+ )
+ )
+ .aggregators(ImmutableList.of(
+ new FixedBucketsHistogramAggregatorFactory(
+ "a0:agg",
+ "v0",
+ 20,
+ 0.0d,
+ 10.0d,
+ FixedBucketsHistogram.OutlierHandlingMode.IGNORE,
+ false
+ )
+ ))
+ .postAggregators(
+ new QuantilePostAggregator("a0", "a0:agg", 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{1.75}
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileOnTimeColumn()
+ {
+ // timestamps are used as numbers of milliseconds
+ testQuery(
+ "SELECT APPROX_QUANTILE_FIXED_BUCKETS(__time, 0.5, 20, 946684800000.0,
978393600000.0) FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .aggregators(ImmutableList.of(
+ new FixedBucketsHistogramAggregatorFactory(
+ "a0:agg",
+ "__time",
+ 20,
+ 946684800000.0d,
+ 978393600000.0d,
+ FixedBucketsHistogram.OutlierHandlingMode.IGNORE,
+ false
+ )
+ ))
+ .postAggregators(
+ new QuantilePostAggregator("a0", "a0:agg", 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{947741786112.0}
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileWithNonNumericStringLiteral()
+ {
+ // verify invalid queries return 400 (user error)
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE_FIXED_BUCKETS('m1', 0.99, 20, 0.0, 10.0) FROM
foo",
+ invalidSqlContains(
+ "Function [APPROX_QUANTILE_FIXED_BUCKETS] argument [column] must
be numeric, got string literal ['m1']."
+ + " If you meant to refer to a column, use double quotes."
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileOnNull()
+ {
+ // NULL inputs are accepted, the same as before the first operand was type
checked
+ testQuery(
+ "SELECT APPROX_QUANTILE_FIXED_BUCKETS(NULL, 0.5, 20, 0.0, 10.0),
APPROX_QUANTILE_FIXED_BUCKETS(CAST(NULL AS DOUBLE), 0.5, 20, 0.0, 10.0) FROM
foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "null",
+ ColumnType.FLOAT,
+ TestExprMacroTable.INSTANCE
+ ),
+ new ExpressionVirtualColumn(
+ "v1",
+ "null",
+ ColumnType.FLOAT,
+ TestExprMacroTable.INSTANCE
+ )
+ )
+ .aggregators(ImmutableList.of(
+ new FixedBucketsHistogramAggregatorFactory(
+ "a0:agg",
+ "v0",
+ 20,
+ 0.0d,
+ 10.0d,
+ FixedBucketsHistogram.OutlierHandlingMode.IGNORE,
+ false
+ ),
+ new FixedBucketsHistogramAggregatorFactory(
+ "a1:agg",
+ "v1",
+ 20,
+ 0.0d,
+ 10.0d,
+ FixedBucketsHistogram.OutlierHandlingMode.IGNORE,
+ false
+ )
+ ))
+ .postAggregators(
+ new QuantilePostAggregator("a0", "a0:agg", 0.5f),
+ new QuantilePostAggregator("a1", "a1:agg", 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{0.0, 0.0}
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileWithNonNumericInputTypes()
+ {
+ // verify invalid queries return 400 (user error)
+ final String expectedMessage = "Cannot apply
'APPROX_QUANTILE_FIXED_BUCKETS'";
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE_FIXED_BUCKETS(m1 > 1, 0.99, 20, 0.0, 10.0)
FROM foo",
+ invalidSqlContains(expectedMessage)
+ );
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE_FIXED_BUCKETS(ARRAY[1.0, 2.0], 0.99, 20, 0.0,
10.0) FROM foo",
+ invalidSqlContains(expectedMessage)
+ );
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE_FIXED_BUCKETS(PARSE_JSON('1'), 0.99, 20, 0.0,
10.0) FROM foo",
+ invalidSqlContains(expectedMessage)
+ );
+ }
}
diff --git
a/extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/sql/QuantileSqlAggregatorTest.java
b/extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/sql/QuantileSqlAggregatorTest.java
index 4e46ff70d18..451f392eb81 100644
---
a/extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/sql/QuantileSqlAggregatorTest.java
+++
b/extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/sql/QuantileSqlAggregatorTest.java
@@ -491,4 +491,173 @@ public class QuantileSqlAggregatorTest extends
BaseCalciteQueryTest
)
);
}
+
+ @Test
+ public void testQuantileWithNonNumericStringLiteral()
+ {
+ // verify invalid queries return 400 (user error)
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE('m1', 0.99) FROM foo",
+ invalidSqlContains(
+ "Function [APPROX_QUANTILE] argument [column] must be numeric, got
string literal ['m1']."
+ + " If you meant to refer to a column, use double quotes."
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileOnNumericStringLiteral()
+ {
+ // string literals that can be parsed as numbers are accepted
+ testQuery(
+ "SELECT APPROX_QUANTILE('1.5', 0.5) FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "1.5",
+ ColumnType.FLOAT,
+ TestExprMacroTable.INSTANCE
+ )
+ )
+ .aggregators(ImmutableList.of(
+ new ApproximateHistogramAggregatorFactory("a0:agg",
"v0", null, null, null, null, false)
+ ))
+ .postAggregators(
+ new QuantilePostAggregator("a0", "a0:agg", 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{1.5}
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileOnTimeColumn()
+ {
+ // timestamps are used as numbers of milliseconds
+ testQuery(
+ "SELECT APPROX_QUANTILE(__time, 0.5) FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .aggregators(ImmutableList.of(
+ new ApproximateHistogramAggregatorFactory("a0:agg",
"__time", null, null, null, null, false)
+ ))
+ .postAggregators(
+ new QuantilePostAggregator("a0", "a0:agg", 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{946857574400.0}
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileOnNull()
+ {
+ // NULL inputs are accepted, the same as before the first operand was type
checked
+ testQuery(
+ "SELECT APPROX_QUANTILE(NULL, 0.5), APPROX_QUANTILE(CAST(NULL AS
DOUBLE), 0.5) FROM foo",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn(
+ "v0",
+ "null",
+ ColumnType.FLOAT,
+ TestExprMacroTable.INSTANCE
+ ),
+ new ExpressionVirtualColumn(
+ "v1",
+ "null",
+ ColumnType.FLOAT,
+ TestExprMacroTable.INSTANCE
+ )
+ )
+ .aggregators(ImmutableList.of(
+ new ApproximateHistogramAggregatorFactory("a0:agg",
"v0", null, null, null, null, false),
+ new ApproximateHistogramAggregatorFactory("a1:agg",
"v1", null, null, null, null, false)
+ ))
+ .postAggregators(
+ new QuantilePostAggregator("a0", "a0:agg", 0.5f),
+ new QuantilePostAggregator("a1", "a1:agg", 0.5f)
+ )
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{Double.NaN, Double.NaN}
+ )
+ );
+ }
+
+ @Test
+ public void testQuantileWithNonNumericInputTypes()
+ {
+ // verify invalid queries return 400 (user error)
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE(m1 > 1, 0.99) FROM foo",
+ invalidSqlContains("Cannot apply 'APPROX_QUANTILE'")
+ );
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE(ARRAY[1.0, 2.0], 0.99) FROM foo",
+ invalidSqlContains("Cannot apply 'APPROX_QUANTILE'")
+ );
+ testQueryThrows(
+ "SELECT APPROX_QUANTILE(PARSE_JSON('1'), 0.99) FROM foo",
+ invalidSqlContains("Cannot apply 'APPROX_QUANTILE'")
+ );
+ }
+
+ @Test
+ public void testQuantileOnStringColumnWithConstantFilter()
+ {
+ // dim1 is known to be 'abc' at planning time, but it is a column, so it
must not be rejected like a string literal
+ testBuilder()
+ .sql("SELECT APPROX_QUANTILE(dim1, 0.5) FROM foo WHERE dim1 = 'abc'")
+ .expectedQueries(
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+ .intervals(new
MultipleIntervalSegmentSpec(ImmutableList.of(Filtration.eternity())))
+ .granularity(Granularities.ALL)
+ .virtualColumns(
+ new ExpressionVirtualColumn("v0", "null",
ColumnType.FLOAT, ExprMacroTable.nil())
+ )
+ .filters(equality("dim1", "abc", ColumnType.STRING))
+ .aggregators(
+ new ApproximateHistogramAggregatorFactory(
+ "a0:agg",
+ "v0",
+ 50,
+ 7,
+ Float.NEGATIVE_INFINITY,
+ Float.POSITIVE_INFINITY,
+ false
+ )
+ )
+ .postAggregators(new QuantilePostAggregator("a0",
"a0:agg", 0.5f))
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ )
+ )
+ .expectedResults(ImmutableList.of(new Object[]{Double.NaN}))
+ .run();
+ }
}
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/Aggregations.java
b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/Aggregations.java
index 5c06332a9bc..6f5ccecbed8 100644
---
a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/Aggregations.java
+++
b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/Aggregations.java
@@ -20,11 +20,15 @@
package org.apache.druid.sql.calcite.aggregation;
import org.apache.calcite.rel.core.AggregateCall;
+import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.druid.error.InvalidSqlInput;
import org.apache.druid.segment.column.RowSignature;
import org.apache.druid.segment.column.ValueType;
import org.apache.druid.sql.calcite.expression.DruidExpression;
import org.apache.druid.sql.calcite.expression.Expressions;
+import org.apache.druid.sql.calcite.planner.Calcites;
import org.apache.druid.sql.calcite.planner.PlannerContext;
import org.apache.druid.sql.calcite.rel.InputAccessor;
@@ -47,6 +51,10 @@ public class Aggregations
*
* 1) They can take direct field accesses or expressions as inputs.
* 2) They cannot implicitly cast strings to numbers when using a direct
field access.
+ *
+ * Unlike {@link #toDruidExpressionForNumericAggregator}, this does not
reject string literals, because it is also
+ * used by aggregators that accept strings, such as COUNT and SINGLE_VALUE.
+ *
* @param plannerContext SQL planner context
* @param call aggregate call object
* @param inputAccessor gives access to input fields and schema
@@ -64,7 +72,7 @@ public class Aggregations
.getArgList()
.stream()
.map(i -> inputAccessor.getField(i))
- .map(rexNode -> toDruidExpressionForNumericAggregator(plannerContext,
inputAccessor.getInputRowSignature(), rexNode))
+ .map(rexNode -> toDruidExpressionWithStringColumnCast(plannerContext,
inputAccessor.getInputRowSignature(), rexNode))
.collect(Collectors.toList());
if (args.stream().noneMatch(Objects::isNull)) {
@@ -82,6 +90,10 @@ public class Aggregations
* Consider using {@link Expressions#toDruidExpression(PlannerContext,
RowSignature, RexNode)} for projections
* or the aggregators that don't require numeric inputs.
*
+ * String literals are converted to numbers the same way as string columns,
but since their value is known at
+ * planning time, a string literal that cannot be parsed as a number is
rejected with an {@link InvalidSqlInput}
+ * error instead of silently producing nulls at runtime.
+ *
* @param plannerContext SQL planner context
* @param rowSignature signature of the rows to be extracted from
* @param rexNode expression meant to be applied on top of the rows
@@ -93,6 +105,17 @@ public class Aggregations
final RowSignature rowSignature,
final RexNode rexNode
)
+ {
+ validateStringLiteralIsNumeric(rexNode);
+ return toDruidExpressionWithStringColumnCast(plannerContext, rowSignature,
rexNode);
+ }
+
+ @Nullable
+ private static DruidExpression toDruidExpressionWithStringColumnCast(
+ final PlannerContext plannerContext,
+ final RowSignature rowSignature,
+ final RexNode rexNode
+ )
{
final DruidExpression druidExpression =
Expressions.toDruidExpression(plannerContext, rowSignature, rexNode);
if (druidExpression == null) {
@@ -109,4 +132,20 @@ public class Aggregations
return druidExpression;
}
}
+
+ /**
+ * Throws an {@link InvalidSqlInput} error if the given node is a non-null
string literal that cannot be parsed as a
+ * number. See {@link Calcites#isNumericString(String)}.
+ */
+ private static void validateStringLiteralIsNumeric(final RexNode rexNode)
+ {
+ if (!(rexNode instanceof RexLiteral) ||
!SqlTypeFamily.CHARACTER.contains(rexNode.getType())) {
+ return;
+ }
+
+ final String value = ((RexLiteral) rexNode).getValueAs(String.class);
+ if (value != null && !Calcites.isNumericString(value)) {
+ throw Calcites.nonNumericStringLiteralError("Aggregator input", value);
+ }
+ }
}
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/expression/DefaultOperandTypeChecker.java
b/sql/src/main/java/org/apache/druid/sql/calcite/expression/DefaultOperandTypeChecker.java
index a52ce5707c1..3206f080aed 100644
---
a/sql/src/main/java/org/apache/druid/sql/calcite/expression/DefaultOperandTypeChecker.java
+++
b/sql/src/main/java/org/apache/druid/sql/calcite/expression/DefaultOperandTypeChecker.java
@@ -25,6 +25,7 @@ import it.unimi.dsi.fastutil.ints.IntSet;
import it.unimi.dsi.fastutil.ints.IntSets;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlCharStringLiteral;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperandCountRange;
import org.apache.calcite.sql.SqlOperator;
@@ -33,8 +34,12 @@ import org.apache.calcite.sql.type.SqlOperandCountRanges;
import org.apache.calcite.sql.type.SqlOperandTypeChecker;
import org.apache.calcite.sql.type.SqlTypeFamily;
import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.validate.implicit.TypeCoercion;
import org.apache.calcite.util.Static;
import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.sql.calcite.planner.Calcites;
+import org.apache.druid.sql.calcite.planner.DruidTypeCoercion;
import javax.annotation.Nullable;
import java.util.Arrays;
@@ -53,14 +58,14 @@ public class DefaultOperandTypeChecker implements
SqlOperandTypeChecker
* {@link #operandTypes} are used instead.
*/
private final List<String> operandNames;
- private final List<SqlTypeFamily> operandTypes;
+ private final List<OperandType> operandTypes;
private final int requiredOperands;
private final IntSet nullableOperands;
private final IntSet literalOperands;
private DefaultOperandTypeChecker(
final List<String> operandNames,
- final List<SqlTypeFamily> operandTypes,
+ final List<OperandType> operandTypes,
final int requiredOperands,
final IntSet nullableOperands,
@Nullable final int[] literalOperands
@@ -110,13 +115,40 @@ public class DefaultOperandTypeChecker implements
SqlOperandTypeChecker
}
}
+ final OperandType expectedType = operandTypes.get(i);
+ if (expectedType.isCoerceStrings() && operand instanceof
SqlCharStringLiteral) {
+ // Verify that a string literal can be converted to a number. This
must happen before coercion, which would
+ // otherwise turn it into a null number. Explicit CASTs are not
checked, since they are the user's intent.
+ final String value = ((SqlCharStringLiteral)
operand).getValueAs(String.class);
+ if (!Calcites.isNumericString(value)) {
+ if (throwOnFailure) {
+ throw Calcites.nonNumericStringLiteralError(
+ StringUtils.format(
+ "Function [%s] argument [%s]",
+ callBinding.getOperator().getName(),
+ operandNames.isEmpty() ? i + 1 : operandNames.get(i)
+ ),
+ value
+ );
+ } else {
+ return false;
+ }
+ }
+ }
+
final RelDataType operandType =
callBinding.getValidator().deriveType(callBinding.getScope(), operand);
- final SqlTypeFamily expectedFamily = operandTypes.get(i);
+ final SqlTypeFamily expectedFamily = expectedType.getFamily();
if (expectedFamily == SqlTypeFamily.ANY) {
// ANY matches anything. This operand is all good; do nothing.
} else if
(expectedFamily.getTypeNames().contains(operandType.getSqlTypeName())) {
// Operand came in with one of the expected types.
+ } else if (expectedType.isAllowedComplexType(operandType)) {
+ // Operand came in with one of the complex types allowed for this
operand.
+ } else if (expectedType.isCoerceStrings() &&
SqlTypeFamily.DATETIME.contains(operandType)) {
+ // Operand came in with a datetime type, which Druid stores as a
number of milliseconds.
+ } else if (expectedType.isCoerceStrings() && coerceToNumber(callBinding,
i, operandType)) {
+ // Operand was a character string, and is now cast to a number.
} else if (operandType.getSqlTypeName() == SqlTypeName.NULL ||
SqlUtil.isNullLiteral(operand, true)) {
// Null came in, check if operand is a nullable type.
if (!nullableOperands.contains(i)) {
@@ -138,6 +170,37 @@ public class DefaultOperandTypeChecker implements
SqlOperandTypeChecker
return true;
}
+ /**
+ * Casts a character string operand to DOUBLE using implicit type coercion.
Returns false, without changing the
+ * operand, if the operand is not a character string or if type coercion is
not available.
+ */
+ private static boolean coerceToNumber(
+ final SqlCallBinding callBinding,
+ final int operand,
+ final RelDataType operandType
+ )
+ {
+ if (!SqlTypeFamily.CHARACTER.contains(operandType) ||
!callBinding.isTypeCoercionEnabled()) {
+ return false;
+ }
+
+ final TypeCoercion typeCoercion =
callBinding.getValidator().getTypeCoercion();
+ if (!(typeCoercion instanceof DruidTypeCoercion)) {
+ return false;
+ }
+
+ return ((DruidTypeCoercion) typeCoercion).coerceOperand(
+ callBinding.getScope(),
+ callBinding.getCall(),
+ operand,
+ Calcites.createSqlTypeWithNullability(
+ callBinding.getTypeFactory(),
+ SqlTypeName.DOUBLE,
+ operandType.isNullable()
+ )
+ );
+ }
+
@Override
public SqlOperandCountRange getOperandCountRange()
{
@@ -183,7 +246,7 @@ public class DefaultOperandTypeChecker implements
SqlOperandTypeChecker
public static class Builder
{
private List<String> operandNames = Collections.emptyList();
- private List<SqlTypeFamily> operandTypes;
+ private List<OperandType> operandTypes;
@Nullable
private Integer requiredOperandCount;
@@ -206,16 +269,24 @@ public class DefaultOperandTypeChecker implements
SqlOperandTypeChecker
return this;
}
- public Builder operandTypes(final SqlTypeFamily... operandTypes)
+ /**
+ * Sets the operand types, which may accept complex types or coerce
strings, such as
+ * {@code OperandType.numeric().orComplex(type).coerceStrings()}.
+ */
+ public Builder operandTypes(final OperandType... operandTypes)
{
this.operandTypes = Arrays.asList(operandTypes);
return this;
}
+ public Builder operandTypes(final SqlTypeFamily... operandTypes)
+ {
+ return
operandTypes(Arrays.stream(operandTypes).map(OperandType::of).toArray(OperandType[]::new));
+ }
+
public Builder operandTypes(final List<SqlTypeFamily> operandTypes)
{
- this.operandTypes = operandTypes;
- return this;
+ return
operandTypes(operandTypes.stream().map(OperandType::of).toArray(OperandType[]::new));
}
public Builder requiredOperandCount(Integer requiredOperandCount)
@@ -243,16 +314,30 @@ public class DefaultOperandTypeChecker implements
SqlOperandTypeChecker
operandNames,
operandTypes,
computedRequiredOperandCount,
-
DefaultOperandTypeChecker.buildNullableOperands(computedRequiredOperandCount,
operandTypes.size(), notNullOperands),
+
DefaultOperandTypeChecker.buildNullableOperands(computedRequiredOperandCount,
operandTypes, notNullOperands),
literalOperands
);
}
}
- public static IntSet buildNullableOperands(int requiredOperandCount, int
totalOperandCount, IntSet notNullOperands)
+ /**
+ * Returns the indexes of operands that accept NULL literals: optional
operands, and operands declared with
+ * {@link OperandType#nullable()}. Operands in notNullOperands are always
excluded, even if they are optional or
+ * nullable.
+ *
+ * @param requiredOperandCount number of required operands, at most the
number of operand types
+ * @param operandTypes types of all operands
+ * @param notNullOperands indexes of operands that never accept NULL
literals
+ */
+ public static IntSet buildNullableOperands(
+ int requiredOperandCount,
+ List<OperandType> operandTypes,
+ IntSet notNullOperands
+ )
{
final IntSet nullableOperands = new IntArraySet();
- IntStream.range(requiredOperandCount, totalOperandCount)
+ IntStream.range(0, operandTypes.size())
+ .filter(i -> i >= requiredOperandCount ||
operandTypes.get(i).isNullable())
.filter(i -> !notNullOperands.contains(i))
.forEach(nullableOperands::add);
return nullableOperands;
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperandType.java
b/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperandType.java
new file mode 100644
index 00000000000..40f430e3c4c
--- /dev/null
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperandType.java
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.sql.calcite.expression;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.druid.error.DruidException;
+import org.apache.druid.segment.column.ColumnType;
+import org.apache.druid.sql.calcite.table.RowSignatures;
+
+/**
+ * Type of one operand of a {@link DefaultOperandTypeChecker}: a {@link
SqlTypeFamily}, optionally with complex types
+ * that are accepted too, whether NULL literals are accepted, and for {@link
SqlTypeFamily#NUMERIC} operands, whether
+ * character strings are accepted by casting them to numbers. Instances are
immutable.
+ */
+public final class OperandType
+{
+ private final SqlTypeFamily family;
+ private final ImmutableSet<ColumnType> complexTypes;
+ private final boolean coerceStrings;
+ private final boolean nullable;
+
+ private OperandType(
+ final SqlTypeFamily family,
+ final ImmutableSet<ColumnType> complexTypes,
+ final boolean coerceStrings,
+ final boolean nullable
+ )
+ {
+ this.family = Preconditions.checkNotNull(family, "family");
+ this.complexTypes = complexTypes;
+ this.coerceStrings = coerceStrings;
+ this.nullable = nullable;
+ }
+
+ public static OperandType of(final SqlTypeFamily family)
+ {
+ return new OperandType(family, ImmutableSet.of(), false, false);
+ }
+
+ public static OperandType numeric()
+ {
+ return of(SqlTypeFamily.NUMERIC);
+ }
+
+ /**
+ * Returns an operand type that also accepts the given complex types.
Comparison uses
+ * {@link RowSignatures.ComplexSqlType#getColumnType()}, so it does not
depend on nullability. Not allowed for
+ * {@link SqlTypeFamily#ANY} operands, which accept complex types already.
+ */
+ public OperandType orComplex(final ColumnType... types)
+ {
+ if (family == SqlTypeFamily.ANY) {
+ throw DruidException.defensive("orComplex() does not apply to [%s]
operands", SqlTypeFamily.ANY);
+ }
+ return new OperandType(
+ family,
+
ImmutableSet.<ColumnType>builder().addAll(complexTypes).add(types).build(),
+ coerceStrings,
+ nullable
+ );
+ }
+
+ /**
+ * Returns a {@link SqlTypeFamily#NUMERIC} operand type that also accepts
character strings, such as string
+ * columns, by casting them to DOUBLE. String literals that cannot be parsed
as numbers are rejected, since they are
+ * usually column names written in single quotes instead of double quotes.
Datetime operands, such as
+ * {@code __time}, are also accepted without a cast, since Druid stores them
as numbers of milliseconds.
+ */
+ public OperandType coerceStrings()
+ {
+ if (family != SqlTypeFamily.NUMERIC) {
+ throw DruidException.defensive(
+ "coerceStrings() only applies to [%s] operands, got [%s]",
+ SqlTypeFamily.NUMERIC,
+ family
+ );
+ }
+ return new OperandType(family, complexTypes, true, nullable);
+ }
+
+ /**
+ * Returns an operand type that also accepts NULL literals, even if the
operand is required. Optional operands
+ * accept NULL literals already.
+ */
+ public OperandType nullable()
+ {
+ return new OperandType(family, complexTypes, coerceStrings, true);
+ }
+
+ public SqlTypeFamily getFamily()
+ {
+ return family;
+ }
+
+ public boolean isCoerceStrings()
+ {
+ return coerceStrings;
+ }
+
+ public boolean isNullable()
+ {
+ return nullable;
+ }
+
+ /**
+ * Whether the type is one of the complex types given to {@link
#orComplex(ColumnType...)}.
+ */
+ public boolean isAllowedComplexType(final RelDataType type)
+ {
+ return type instanceof RowSignatures.ComplexSqlType
+ && complexTypes.contains(((RowSignatures.ComplexSqlType)
type).getColumnType());
+ }
+
+ /**
+ * Used by {@link DefaultOperandTypeChecker#getAllowedSignatures} if the
operands have no names, e.g.
+ * {@code NUMERIC | COMPLEX<tdigest>}.
+ */
+ @Override
+ public String toString()
+ {
+ final StringBuilder sb = new StringBuilder(family.toString());
+ for (final ColumnType type : complexTypes) {
+ sb.append(" | ").append(type);
+ }
+ return sb.toString();
+ }
+}
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
b/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
index 450d6208240..3c0e72717b8 100644
---
a/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
+++
b/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
@@ -345,7 +345,7 @@ public class OperatorConversions
// For operand type checking
private SqlOperandTypeChecker operandTypeChecker;
private List<String> operandNames = Collections.emptyList();
- private List<SqlTypeFamily> operandTypes;
+ private List<OperandType> operandTypes;
private Integer requiredOperandCount;
private int[] literalOperands;
private SqlOperandTypeInference operandTypeInference;
@@ -471,7 +471,8 @@ public class OperatorConversions
/**
* Provide customized operand type checking logic.
*
- * Either {@link #operandTypes(SqlTypeFamily...)} or this method must be
used before calling {@link #build()}.
+ * Either {@link #operandTypes(SqlTypeFamily...)}, {@link
#operandTypes(OperandType...)} or this method must be used
+ * before calling {@link #build()}.
* These methods cannot be mixed; you must call exactly one.
*/
public OperatorBuilder<T> operandTypeChecker(final SqlOperandTypeChecker
operandTypeChecker)
@@ -501,6 +502,16 @@ public class OperatorConversions
* For deeper control, use {@link
#operandTypeChecker(SqlOperandTypeChecker)} instead.
*/
public OperatorBuilder<T> operandTypes(final SqlTypeFamily... operandTypes)
+ {
+ return
operandTypes(Arrays.stream(operandTypes).map(OperandType::of).toArray(OperandType[]::new));
+ }
+
+ /**
+ * Equivalent to calling {@link
DefaultOperandTypeChecker.Builder#operandTypes(OperandType...)}. Like
+ * {@link #operandTypes(SqlTypeFamily...)}, but the operand types may
accept complex types or coerce strings, such
+ * as {@code OperandType.numeric().orComplex(type).coerceStrings()}.
+ */
+ public OperatorBuilder<T> operandTypes(final OperandType... operandTypes)
{
this.operandTypes = Arrays.asList(operandTypes);
return this;
@@ -572,8 +583,8 @@ public class OperatorConversions
return DefaultOperandTypeChecker
.builder()
.operandNames(operandNames)
- .operandTypes(operandTypes)
- .requiredOperandCount(requiredOperandCount == null ?
operandTypes.size() : requiredOperandCount)
+ .operandTypes(operandTypes.toArray(new OperandType[0]))
+ .requiredOperandCount(getRequiredOperandCount())
.literalOperands(literalOperands)
.build();
} else if (operandNames.isEmpty()
@@ -589,11 +600,17 @@ public class OperatorConversions
}
}
+ private int getRequiredOperandCount()
+ {
+ return requiredOperandCount == null ? operandTypes.size() :
requiredOperandCount;
+ }
+
protected SqlOperandTypeInference buildOperandTypeInference()
{
- final IntSet nullableOperands = requiredOperandCount == null
+ // operandTypes is null if an operandTypeChecker was passed in instead,
e.g. GREATEST and LEAST.
+ final IntSet nullableOperands = operandTypes == null
? new IntArraySet()
- :
DefaultOperandTypeChecker.buildNullableOperands(requiredOperandCount,
operandTypes.size(), new IntArraySet());
+ :
DefaultOperandTypeChecker.buildNullableOperands(getRequiredOperandCount(),
operandTypes, new IntArraySet());
if (operandTypeInference == null) {
SqlOperandTypeInference defaultInference = new
DefaultOperandTypeInference(operandTypes, nullableOperands);
return (callBinding, returnType, types) -> {
@@ -685,10 +702,10 @@ public class OperatorConversions
*/
private static class DefaultOperandTypeInference implements
SqlOperandTypeInference
{
- private final List<SqlTypeFamily> operandTypes;
+ private final List<OperandType> operandTypes;
private final IntSet nullableOperands;
- DefaultOperandTypeInference(final List<SqlTypeFamily> operandTypes, final
IntSet nullableOperands)
+ DefaultOperandTypeInference(final List<OperandType> operandTypes, final
IntSet nullableOperands)
{
this.operandTypes = operandTypes;
this.nullableOperands = nullableOperands;
@@ -715,7 +732,7 @@ public class OperatorConversions
if (nullableOperands.contains(i)) {
inferredType = Calcites.createSqlTypeWithNullability(
callBinding.getTypeFactory(),
- defaultTypeForFamily(operandTypes.get(i)),
+ defaultTypeForFamily(operandTypes.get(i).getFamily()),
true
);
} else {
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java
index c0c6b3cb44c..88a8c03afe3 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java
@@ -41,8 +41,10 @@ import org.apache.calcite.util.DateString;
import org.apache.calcite.util.TimeString;
import org.apache.calcite.util.TimestampString;
import org.apache.druid.error.DruidException;
+import org.apache.druid.error.InvalidSqlInput;
import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.math.expr.ExprEval;
import org.apache.druid.math.expr.ExpressionProcessing;
import org.apache.druid.math.expr.ExpressionProcessingConfig;
import org.apache.druid.query.ordering.StringComparator;
@@ -527,6 +529,32 @@ public class Calcites
return StringUtils.format("%s:%s", prefix, suffix);
}
+ /**
+ * Returns whether a string literal can be used where a number is expected.
The parsing follows
+ * {@link ExprEval#computeNumber(String)}, which is how strings are
implicitly converted to numbers at runtime.
+ */
+ public static boolean isNumericString(final String value)
+ {
+ return ExprEval.computeNumber(value) != null;
+ }
+
+ /**
+ * Creates the error for a string literal that is used where a number is
expected, but cannot be parsed as one
+ * (see {@link #isNumericString(String)}). Such a literal is often a column
name written in single quotes instead of
+ * double quotes.
+ *
+ * @param target where the literal is used, such as "Function
[APPROX_QUANTILE] argument [column]"
+ * @param value value of the literal
+ */
+ public static DruidException nonNumericStringLiteralError(final String
target, final String value)
+ {
+ return InvalidSqlInput.exception(
+ "%s must be numeric, got string literal ['%s']. If you meant to refer
to a column, use double quotes.",
+ target,
+ value
+ );
+ }
+
public static Class<?> sqlTypeNameJdbcToJavaClass(SqlTypeName typeName)
{
// reference:
https://docs.oracle.com/javase/1.5.0/docs/guide/jdbc/getstart/mapping.html
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidTypeCoercion.java
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidTypeCoercion.java
index 699f32b09c4..787c9244346 100644
---
a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidTypeCoercion.java
+++
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidTypeCoercion.java
@@ -23,10 +23,12 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.type.SqlTypeCoercionRule;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.type.SqlTypeUtil;
import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
import org.apache.calcite.sql.validate.implicit.TypeCoercion;
import org.apache.calcite.sql.validate.implicit.TypeCoercionImpl;
import org.checkerframework.checker.nullness.qual.Nullable;
@@ -77,6 +79,23 @@ public class DruidTypeCoercion extends TypeCoercionImpl
return super.commonTypeForBinaryComparison(type1, type2);
}
+ /**
+ * Casts the operand at the given index of a call to the target type, if a
cast is needed. Exposes
+ * {@link #coerceOperandType(SqlValidatorScope, SqlCall, int, RelDataType)}
to operand type checkers, such as
+ * {@link org.apache.druid.sql.calcite.expression.DefaultOperandTypeChecker}.
+ *
+ * @return true if the operand was cast
+ */
+ public boolean coerceOperand(
+ @Nullable SqlValidatorScope scope,
+ SqlCall call,
+ int index,
+ RelDataType targetType
+ )
+ {
+ return coerceOperandType(scope, call, index, targetType);
+ }
+
/**
* Choose a common array element type for comparisons.
*/
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index 499df1dbddd..6bb807024ef 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -15403,6 +15403,27 @@ public class CalciteQueryTest extends
BaseCalciteQueryTest
}
}
+ @Test
+ public void testCountDistinctNonNumericStringLiteral()
+ {
+ // COUNT shares the argument translation of numeric aggregators, but must
still accept any string literal
+ testBuilder()
+ .sql("SELECT COUNT(DISTINCT 'abc') FROM foo")
+ .expectedResults(ImmutableList.of(new Object[]{1L}))
+ .run();
+ }
+
+ @Test
+ public void testScalarSubqueryNonNumericStringLiteral()
+ {
+ // SINGLE_VALUE shares the argument translation of numeric aggregators,
but must still accept any string literal
+ cannotVectorize();
+ testBuilder()
+ .sql("SELECT COUNT(*) FROM foo WHERE dim1 = (SELECT 'abc' FROM foo
WHERE dim1 = 'def')")
+ .expectedResults(ImmutableList.of(new Object[]{1L}))
+ .run();
+ }
+
/**
* see {@link TestDataBuilder#RAW_ROWS1_WITH_NUMERIC_DIMS}
* for the input data source of this test
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/aggregation/AggregationsTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/aggregation/AggregationsTest.java
new file mode 100644
index 00000000000..785dd7731c7
--- /dev/null
+++
b/sql/src/test/java/org/apache/druid/sql/calcite/aggregation/AggregationsTest.java
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.sql.calcite.aggregation;
+
+import org.apache.calcite.jdbc.JavaTypeFactoryImpl;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.error.DruidException;
+import org.apache.druid.segment.column.ColumnType;
+import org.apache.druid.segment.column.RowSignature;
+import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.ExpressionTestHelper;
+import org.apache.druid.sql.calcite.table.RowSignatures;
+import org.apache.druid.sql.calcite.util.CalciteTestBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class AggregationsTest extends CalciteTestBase
+{
+ private static final RowSignature ROW_SIGNATURE = RowSignature.builder()
+ .add("s",
ColumnType.STRING)
+ .add("d",
ColumnType.DOUBLE)
+ .build();
+
+ private final JavaTypeFactoryImpl typeFactory = new JavaTypeFactoryImpl();
+ private final RexBuilder rexBuilder = new RexBuilder(typeFactory);
+
+ @Test
+ public void testNumericAggregatorWithNonNumericStringLiteral()
+ {
+ for (String value : new String[]{"m1", "abc", "", " 1 "}) {
+ final DruidException e = Assertions.assertThrows(
+ DruidException.class,
+ () -> toDruidExpression(rexBuilder.makeLiteral(value))
+ );
+ Assertions.assertEquals(DruidException.Persona.USER,
e.getTargetPersona());
+ Assertions.assertEquals(DruidException.Category.INVALID_INPUT,
e.getCategory());
+ Assertions.assertEquals(
+ "Aggregator input must be numeric, got string literal ['" + value +
"']."
+ + " If you meant to refer to a column, use double quotes.",
+ e.getMessage()
+ );
+ }
+ }
+
+ @Test
+ public void testNumericAggregatorWithNumericStringLiteral()
+ {
+ // Strings that are implicitly converted to numbers at runtime, see
ExprEval.computeNumber
+ for (String value : new String[]{"1", "-1.5", "1e3", "true", "false"}) {
+ final DruidExpression expression =
toDruidExpression(rexBuilder.makeLiteral(value));
+ Assertions.assertNotNull(expression, value);
+ Assertions.assertFalse(expression.isDirectColumnAccess(), value);
+ }
+ }
+
+ @Test
+ public void testNumericAggregatorWithNullLiteral()
+ {
+
Assertions.assertNotNull(toDruidExpression(rexBuilder.makeNullLiteral(typeFactory.createSqlType(SqlTypeName.VARCHAR))));
+
Assertions.assertNotNull(toDruidExpression(rexBuilder.makeNullLiteral(typeFactory.createSqlType(SqlTypeName.DOUBLE))));
+
Assertions.assertNotNull(toDruidExpression(rexBuilder.makeNullLiteral(typeFactory.createSqlType(SqlTypeName.NULL))));
+ }
+
+ @Test
+ public void testNumericAggregatorWithStringColumn()
+ {
+ // STRING columns are implicitly converted, so they are used through an
expression instead of a direct access
+ final DruidExpression expression = toDruidExpression(makeInputRef("s"));
+ Assertions.assertNotNull(expression);
+ Assertions.assertFalse(expression.isDirectColumnAccess());
+ Assertions.assertEquals("\"s\"", expression.getExpression());
+ }
+
+ @Test
+ public void testNumericAggregatorWithNumericColumn()
+ {
+ final DruidExpression expression = toDruidExpression(makeInputRef("d"));
+ Assertions.assertNotNull(expression);
+ Assertions.assertTrue(expression.isDirectColumnAccess());
+ Assertions.assertEquals("d", expression.getDirectColumn());
+ }
+
+ private DruidExpression toDruidExpression(final RexNode rexNode)
+ {
+ return Aggregations.toDruidExpressionForNumericAggregator(
+ ExpressionTestHelper.PLANNER_CONTEXT,
+ ROW_SIGNATURE,
+ rexNode
+ );
+ }
+
+ private RexNode makeInputRef(final String columnName)
+ {
+ final RelDataType rowType = RowSignatures.toRelDataType(ROW_SIGNATURE,
typeFactory);
+ final int index = ROW_SIGNATURE.indexOf(columnName);
+ return
rexBuilder.makeInputRef(rowType.getFieldList().get(index).getType(), index);
+ }
+}
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/expression/OperandTypeTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/expression/OperandTypeTest.java
new file mode 100644
index 00000000000..3d2540a3742
--- /dev/null
+++
b/sql/src/test/java/org/apache/druid/sql/calcite/expression/OperandTypeTest.java
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.sql.calcite.expression;
+
+import org.apache.calcite.sql.type.SqlTypeFactoryImpl;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.error.DruidException;
+import org.apache.druid.segment.column.ColumnType;
+import org.apache.druid.sql.calcite.planner.DruidTypeSystem;
+import org.apache.druid.sql.calcite.table.RowSignatures;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class OperandTypeTest
+{
+ private static final SqlTypeFactoryImpl TYPE_FACTORY = new
SqlTypeFactoryImpl(DruidTypeSystem.INSTANCE);
+ private static final ColumnType COMPLEX_TYPE =
ColumnType.ofComplex("someComplex");
+
+ @Test
+ public void testOf()
+ {
+ final OperandType type = OperandType.of(SqlTypeFamily.STRING);
+ Assertions.assertEquals(SqlTypeFamily.STRING, type.getFamily());
+ Assertions.assertFalse(type.isCoerceStrings());
+ Assertions.assertFalse(type.isNullable());
+
Assertions.assertFalse(type.isAllowedComplexType(RowSignatures.makeComplexType(TYPE_FACTORY,
COMPLEX_TYPE, false)));
+ Assertions.assertEquals("STRING", type.toString());
+ }
+
+ @Test
+ public void testNullable()
+ {
+ final OperandType numeric =
OperandType.numeric().orComplex(COMPLEX_TYPE).coerceStrings();
+ final OperandType type = numeric.nullable();
+
+ Assertions.assertTrue(type.isNullable());
+ Assertions.assertFalse(numeric.isNullable());
+ Assertions.assertTrue(type.isCoerceStrings());
+
Assertions.assertTrue(type.isAllowedComplexType(RowSignatures.makeComplexType(TYPE_FACTORY,
COMPLEX_TYPE, false)));
+
+ // nullable() is kept by the other methods.
+
Assertions.assertTrue(OperandType.numeric().nullable().orComplex(COMPLEX_TYPE).coerceStrings().isNullable());
+ }
+
+ @Test
+ public void testOrComplex()
+ {
+ final OperandType numeric = OperandType.numeric();
+ final OperandType type = numeric.orComplex(COMPLEX_TYPE);
+
+ Assertions.assertEquals(SqlTypeFamily.NUMERIC, type.getFamily());
+
Assertions.assertTrue(type.isAllowedComplexType(RowSignatures.makeComplexType(TYPE_FACTORY,
COMPLEX_TYPE, false)));
+
Assertions.assertTrue(type.isAllowedComplexType(RowSignatures.makeComplexType(TYPE_FACTORY,
COMPLEX_TYPE, true)));
+ Assertions.assertFalse(
+ type.isAllowedComplexType(RowSignatures.makeComplexType(TYPE_FACTORY,
ColumnType.ofComplex("other"), false))
+ );
+
Assertions.assertFalse(type.isAllowedComplexType(TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE)));
+ Assertions.assertEquals("NUMERIC | COMPLEX<someComplex>", type.toString());
+
+ // The original operand type is not changed.
+ Assertions.assertFalse(
+
numeric.isAllowedComplexType(RowSignatures.makeComplexType(TYPE_FACTORY,
COMPLEX_TYPE, false))
+ );
+ }
+
+ @Test
+ public void testOrComplexOnAnyOperand()
+ {
+ final DruidException e = Assertions.assertThrows(
+ DruidException.class,
+ () -> OperandType.of(SqlTypeFamily.ANY).orComplex(COMPLEX_TYPE)
+ );
+ Assertions.assertEquals("orComplex() does not apply to [ANY] operands",
e.getMessage());
+ }
+
+ @Test
+ public void testCoerceStrings()
+ {
+ final OperandType numeric = OperandType.numeric().orComplex(COMPLEX_TYPE);
+ final OperandType type = numeric.coerceStrings();
+
+ Assertions.assertTrue(type.isCoerceStrings());
+ Assertions.assertFalse(numeric.isCoerceStrings());
+
Assertions.assertTrue(type.isAllowedComplexType(RowSignatures.makeComplexType(TYPE_FACTORY,
COMPLEX_TYPE, false)));
+ }
+
+ @Test
+ public void testCoerceStringsOnNonNumericOperand()
+ {
+ final DruidException e = Assertions.assertThrows(
+ DruidException.class,
+ () -> OperandType.of(SqlTypeFamily.STRING).coerceStrings()
+ );
+ Assertions.assertEquals("coerceStrings() only applies to [NUMERIC]
operands, got [STRING]", e.getMessage());
+ }
+}
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/expression/OperatorConversionsTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/expression/OperatorConversionsTest.java
index 486e48349b8..1f09272f3e9 100644
---
a/sql/src/test/java/org/apache/druid/sql/calcite/expression/OperatorConversionsTest.java
+++
b/sql/src/test/java/org/apache/druid/sql/calcite/expression/OperatorConversionsTest.java
@@ -20,6 +20,7 @@
package org.apache.druid.sql.calcite.expression;
import com.google.common.collect.ImmutableList;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.runtime.CalciteContextException;
import org.apache.calcite.runtime.Resources.ExInst;
@@ -36,8 +37,13 @@ import org.apache.calcite.sql.type.SqlTypeFamily;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.validate.SqlValidator;
import org.apache.calcite.sql.validate.SqlValidatorScope;
+import org.apache.calcite.sql.validate.implicit.TypeCoercion;
+import org.apache.druid.error.DruidException;
import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.segment.column.ColumnType;
+import org.apache.druid.sql.calcite.planner.DruidTypeCoercion;
import org.apache.druid.sql.calcite.planner.DruidTypeSystem;
+import org.apache.druid.sql.calcite.table.RowSignatures;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
@@ -45,11 +51,16 @@ import org.mockito.ArgumentMatchers;
import org.mockito.Mockito;
import org.mockito.stubbing.Answer;
+import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.List;
public class OperatorConversionsTest
{
+ private static final SqlTypeFactoryImpl TYPE_FACTORY = new
SqlTypeFactoryImpl(DruidTypeSystem.INSTANCE);
+ private static final ColumnType ALLOWED_COMPLEX_TYPE =
ColumnType.ofComplex("allowedComplex");
+ private static final ColumnType OTHER_COMPLEX_TYPE =
ColumnType.ofComplex("otherComplex");
+
@Nested
public class DefaultOperandTypeCheckerTest
{
@@ -384,6 +395,64 @@ public class OperatorConversionsTest
);
}
+ @Test
+ public void testBuildNullableOperands()
+ {
+ final List<OperandType> operandTypes = ImmutableList.of(
+ OperandType.of(SqlTypeFamily.CHARACTER),
+ OperandType.of(SqlTypeFamily.CHARACTER).nullable(),
+ OperandType.of(SqlTypeFamily.CHARACTER),
+ OperandType.of(SqlTypeFamily.CHARACTER)
+ );
+ Assertions.assertEquals(
+ new IntArraySet(new int[]{1, 3}),
+ DefaultOperandTypeChecker.buildNullableOperands(3, operandTypes, new
IntArraySet())
+ );
+ Assertions.assertEquals(
+ new IntArraySet(new int[]{1, 2, 3}),
+ DefaultOperandTypeChecker.buildNullableOperands(2, operandTypes, new
IntArraySet())
+ );
+ Assertions.assertTrue(
+ DefaultOperandTypeChecker.buildNullableOperands(3, operandTypes, new
IntArraySet(new int[]{1, 3})).isEmpty()
+ );
+ }
+
+ @Test
+ public void testNullLiteralForNullableRequiredOperand()
+ {
+ SqlFunction function = OperatorConversions
+ .operatorBuilder("testNullLiteralForNullableRequiredOperand")
+ .operandTypes(OperandType.of(SqlTypeFamily.CHARACTER).nullable(),
OperandType.of(SqlTypeFamily.CHARACTER))
+ .returnTypeNonNull(SqlTypeName.CHAR)
+ .build();
+ SqlOperandTypeChecker typeChecker = function.getOperandTypeChecker();
+ Assertions.assertTrue(
+ typeChecker.checkOperandTypes(
+ mockCallBinding(
+ function,
+ ImmutableList.of(
+ new OperandSpec(SqlTypeName.NULL, true),
+ new OperandSpec(SqlTypeName.CHAR, false)
+ )
+ ),
+ true
+ )
+ );
+ assertThrowsWithMessage(
+ () -> typeChecker.checkOperandTypes(
+ mockCallBinding(
+ function,
+ ImmutableList.of(
+ new OperandSpec(SqlTypeName.CHAR, false),
+ new OperandSpec(SqlTypeName.NULL, true)
+ )
+ ),
+ true
+ ),
+ "Exception in test for
operator[testNullLiteralForNullableRequiredOperand]: Illegal use of 'NULL'"
+ );
+ }
+
@Test
public void testNonCastableType()
{
@@ -409,6 +478,203 @@ public class OperatorConversionsTest
);
}
+ @Test
+ public void testAllowedComplexTypeOperand()
+ {
+ SqlFunction function =
numericOperandsFunction("testAllowedComplexTypeOperand");
+ SqlOperandTypeChecker typeChecker = function.getOperandTypeChecker();
+ Assertions.assertTrue(
+ typeChecker.checkOperandTypes(
+ mockCallBinding(
+ function,
+ ImmutableList.of(
+ new
OperandSpec(RowSignatures.makeComplexType(TYPE_FACTORY, ALLOWED_COMPLEX_TYPE,
false)),
+ new OperandSpec(SqlTypeName.DOUBLE, false)
+ )
+ ),
+ true
+ )
+ );
+ }
+
+ @Test
+ public void testOtherComplexTypeOperand()
+ {
+ SqlFunction function =
numericOperandsFunction("testOtherComplexTypeOperand");
+ SqlOperandTypeChecker typeChecker = function.getOperandTypeChecker();
+ assertThrowsWithMessage(
+ () -> typeChecker.checkOperandTypes(
+ mockCallBinding(
+ function,
+ ImmutableList.of(
+ new
OperandSpec(RowSignatures.makeComplexType(TYPE_FACTORY, OTHER_COMPLEX_TYPE,
true)),
+ new OperandSpec(SqlTypeName.DOUBLE, false)
+ )
+ ),
+ true
+ ),
+ "Cannot apply 'testOtherComplexTypeOperand' to arguments of type"
+ );
+ }
+
+ @Test
+ public void testOperandWithoutOptions()
+ {
+ // Only the first operand allows complex types and string coercion, so
the second one is checked as usual
+ SqlFunction function =
numericOperandsFunction("testOperandWithoutOptions");
+ SqlOperandTypeChecker typeChecker = function.getOperandTypeChecker();
+ final List<OperandSpec> invalidSecondOperands = ImmutableList.of(
+ new OperandSpec(RowSignatures.makeComplexType(TYPE_FACTORY,
ALLOWED_COMPLEX_TYPE, true)),
+ new OperandSpec(SqlTypeName.VARCHAR, false),
+ new OperandSpec(SqlLiteral.createCharString("abc",
SqlParserPos.ZERO))
+ );
+ for (OperandSpec secondOperand : invalidSecondOperands) {
+ final TypeCoercion typeCoercion =
Mockito.mock(DruidTypeCoercion.class);
+ assertThrowsWithMessage(
+ () -> typeChecker.checkOperandTypes(
+ mockCallBinding(
+ function,
+ ImmutableList.of(new OperandSpec(SqlTypeName.DOUBLE,
false), secondOperand),
+ typeCoercion
+ ),
+ true
+ ),
+ "Cannot apply 'testOperandWithoutOptions' to arguments of type"
+ );
+ Mockito.verifyNoInteractions(typeCoercion);
+ }
+ }
+
+ @Test
+ public void testNonNumericStringLiteralOnCoercibleOperand()
+ {
+ SqlFunction function =
numericOperandsFunction("testNonNumericStringLiteralOnCoercibleOperand");
+ SqlOperandTypeChecker typeChecker = function.getOperandTypeChecker();
+ final TypeCoercion typeCoercion = Mockito.mock(DruidTypeCoercion.class);
+ final List<OperandSpec> operands = ImmutableList.of(
+ new OperandSpec(SqlLiteral.createCharString("m1",
SqlParserPos.ZERO)),
+ new OperandSpec(SqlTypeName.DOUBLE, false)
+ );
+
+ final DruidException e = Assertions.assertThrows(
+ DruidException.class,
+ () -> typeChecker.checkOperandTypes(mockCallBinding(function,
operands, typeCoercion), true)
+ );
+ Assertions.assertEquals(DruidException.Persona.USER,
e.getTargetPersona());
+ Assertions.assertEquals(DruidException.Category.INVALID_INPUT,
e.getCategory());
+ Assertions.assertEquals(
+ "Function [testNonNumericStringLiteralOnCoercibleOperand] argument
[x] must be numeric, got string literal"
+ + " ['m1']. If you meant to refer to a column, use double quotes.",
+ e.getMessage()
+ );
+
+
Assertions.assertFalse(typeChecker.checkOperandTypes(mockCallBinding(function,
operands, typeCoercion), false));
+ Mockito.verifyNoInteractions(typeCoercion);
+ }
+
+ @Test
+ public void testNumericStringLiteralOnCoercibleOperand()
+ {
+ assertCoercedToDouble(
+ "testNumericStringLiteralOnCoercibleOperand",
+ new OperandSpec(SqlLiteral.createCharString("1.5",
SqlParserPos.ZERO))
+ );
+ }
+
+ @Test
+ public void testStringOperandOnCoercibleOperand()
+ {
+ assertCoercedToDouble("testStringOperandOnCoercibleOperand", new
OperandSpec(SqlTypeName.VARCHAR, false, true));
+ }
+
+ @Test
+ public void testBooleanOperandOnCoercibleOperand()
+ {
+ SqlFunction function =
numericOperandsFunction("testBooleanOperandOnCoercibleOperand");
+ SqlOperandTypeChecker typeChecker = function.getOperandTypeChecker();
+ final TypeCoercion typeCoercion = Mockito.mock(DruidTypeCoercion.class);
+ assertThrowsWithMessage(
+ () -> typeChecker.checkOperandTypes(
+ mockCallBinding(
+ function,
+ ImmutableList.of(new OperandSpec(SqlTypeName.BOOLEAN,
false), new OperandSpec(SqlTypeName.DOUBLE, false)),
+ typeCoercion
+ ),
+ true
+ ),
+ "Cannot apply 'testBooleanOperandOnCoercibleOperand' to arguments of
type"
+ );
+ Mockito.verifyNoInteractions(typeCoercion);
+ }
+
+ @Test
+ public void testTimestampOperandOnCoercibleOperand()
+ {
+ SqlFunction function =
numericOperandsFunction("testTimestampOperandOnCoercibleOperand");
+ SqlOperandTypeChecker typeChecker = function.getOperandTypeChecker();
+ final TypeCoercion typeCoercion = Mockito.mock(DruidTypeCoercion.class);
+ Assertions.assertTrue(
+ typeChecker.checkOperandTypes(
+ mockCallBinding(
+ function,
+ ImmutableList.of(
+ new
OperandSpec(TYPE_FACTORY.createSqlType(SqlTypeName.TIMESTAMP)),
+ new OperandSpec(SqlTypeName.DOUBLE, false)
+ ),
+ typeCoercion
+ ),
+ true
+ )
+ );
+ Mockito.verifyNoInteractions(typeCoercion);
+ }
+
+ private void assertCoercedToDouble(String functionName, OperandSpec
firstOperand)
+ {
+ SqlFunction function = numericOperandsFunction(functionName);
+ SqlOperandTypeChecker typeChecker = function.getOperandTypeChecker();
+ final DruidTypeCoercion typeCoercion =
Mockito.mock(DruidTypeCoercion.class);
+ Mockito.when(
+ typeCoercion.coerceOperand(
+ ArgumentMatchers.any(),
+ ArgumentMatchers.any(),
+ ArgumentMatchers.anyInt(),
+ ArgumentMatchers.any()
+ )
+ ).thenReturn(true);
+ final SqlCallBinding callBinding = mockCallBinding(
+ function,
+ ImmutableList.of(firstOperand, new OperandSpec(SqlTypeName.DOUBLE,
false)),
+ typeCoercion
+ );
+
+ Assertions.assertTrue(typeChecker.checkOperandTypes(callBinding, true));
+ Mockito.verify(typeCoercion).coerceOperand(
+ ArgumentMatchers.eq(callBinding.getScope()),
+ ArgumentMatchers.same(callBinding.getCall()),
+ ArgumentMatchers.eq(0),
+ ArgumentMatchers.argThat(type -> type.getSqlTypeName() ==
SqlTypeName.DOUBLE)
+ );
+ Mockito.verifyNoMoreInteractions(typeCoercion);
+ }
+
+ /**
+ * Function with two numeric operands, where only the first one allows
{@link #ALLOWED_COMPLEX_TYPE} and coercion
+ * of strings.
+ */
+ private SqlFunction numericOperandsFunction(String name)
+ {
+ return OperatorConversions
+ .operatorBuilder(name)
+ .operandNames("x", "y")
+ .operandTypes(
+
OperandType.numeric().orComplex(ALLOWED_COMPLEX_TYPE).coerceStrings(),
+ OperandType.of(SqlTypeFamily.NUMERIC)
+ )
+ .returnTypeNonNull(SqlTypeName.DOUBLE)
+ .build();
+ }
+
private static void assertThrowsWithMessage(Runnable runnable, String
expectedMessage)
{
final CalciteContextException e =
Assertions.assertThrows(CalciteContextException.class, runnable::run);
@@ -450,25 +716,62 @@ public class OperatorConversionsTest
);
}
+ @Test
+ public void testSignatureWithoutNamesWithComplexTypes()
+ {
+ SqlFunction function = OperatorConversions
+ .operatorBuilder("testSignatureWithoutNamesWithComplexTypes")
+ .operandTypes(
+
OperandType.numeric().orComplex(ALLOWED_COMPLEX_TYPE).coerceStrings(),
+ OperandType.of(SqlTypeFamily.NUMERIC)
+ )
+ .returnTypeNonNull(SqlTypeName.DOUBLE)
+ .build();
+ SqlOperandTypeChecker typeChecker = function.getOperandTypeChecker();
+
+ Assertions.assertEquals(
+ "'testSignatureWithoutNamesWithComplexTypes(<NUMERIC |
COMPLEX<allowedComplex>>, <NUMERIC>)'",
+ typeChecker.getAllowedSignatures(function, function.getName())
+ );
+ }
+
private static SqlCallBinding mockCallBinding(
SqlFunction function,
List<OperandSpec> actualOperands
)
+ {
+ return mockCallBinding(function, actualOperands, null);
+ }
+
+ private static SqlCallBinding mockCallBinding(
+ SqlFunction function,
+ List<OperandSpec> actualOperands,
+ @Nullable TypeCoercion typeCoercion
+ )
{
SqlValidator validator = Mockito.mock(SqlValidator.class);
- Mockito.when(validator.getTypeFactory()).thenReturn(new
SqlTypeFactoryImpl(DruidTypeSystem.INSTANCE));
+ Mockito.when(validator.getTypeFactory()).thenReturn(TYPE_FACTORY);
+ Mockito.when(validator.config()).thenReturn(SqlValidator.Config.DEFAULT);
+ Mockito.when(validator.getTypeCoercion()).thenReturn(typeCoercion);
List<SqlNode> operands = new ArrayList<>(actualOperands.size());
for (OperandSpec operand : actualOperands) {
+ if (operand.node != null || operand.relDataType != null) {
+ final SqlNode node = operand.node != null ? operand.node :
nonLiteralNode();
+ final RelDataType relDataType = operand.relDataType != null
+ ? operand.relDataType
+ : ((SqlLiteral)
node).createSqlType(TYPE_FACTORY);
+ Mockito.when(validator.deriveType(ArgumentMatchers.any(),
ArgumentMatchers.same(node)))
+ .thenReturn(relDataType);
+ operands.add(node);
+ continue;
+ }
+
final SqlNode node;
if (operand.isLiteral) {
node = Mockito.mock(SqlLiteral.class);
Mockito.when(node.getKind()).thenReturn(SqlKind.LITERAL);
} else {
- node = Mockito.mock(SqlNode.class);
- // Setting this as SqlUtil.isLiteral makes a call on
- // node.getKind() and without this change would
- // return a NPE
- Mockito.when(node.getKind()).thenReturn(SqlKind.OTHER_FUNCTION);
+ node = nonLiteralNode();
}
RelDataType relDataType = Mockito.mock(RelDataType.class);
@@ -502,12 +805,34 @@ public class OperatorConversionsTest
return callBinding;
}
+ private static SqlNode nonLiteralNode()
+ {
+ final SqlNode node = Mockito.mock(SqlNode.class);
+ // Setting this as SqlUtil.isLiteral makes a call on
+ // node.getKind() and without this change would
+ // return a NPE
+ Mockito.when(node.getKind()).thenReturn(SqlKind.OTHER_FUNCTION);
+ return node;
+ }
+
private static class OperandSpec
{
private final SqlTypeName type;
private final boolean isLiteral;
private final boolean isNullable;
+ /**
+ * Actual node of the operand, used instead of a mock if not null.
+ */
+ @Nullable
+ private final SqlNode node;
+
+ /**
+ * Actual derived type of the operand, used instead of a mock if not
null.
+ */
+ @Nullable
+ private final RelDataType relDataType;
+
private OperandSpec(SqlTypeName type, boolean isLiteral)
{
this(type, isLiteral, type == SqlTypeName.NULL);
@@ -518,6 +843,26 @@ public class OperatorConversionsTest
this.type = type;
this.isLiteral = isLiteral;
this.isNullable = isNullable;
+ this.node = null;
+ this.relDataType = null;
+ }
+
+ private OperandSpec(SqlLiteral node)
+ {
+ this.type = null;
+ this.isLiteral = true;
+ this.isNullable = false;
+ this.node = node;
+ this.relDataType = null;
+ }
+
+ private OperandSpec(RelDataType relDataType)
+ {
+ this.type = relDataType.getSqlTypeName();
+ this.isLiteral = false;
+ this.isNullable = relDataType.isNullable();
+ this.node = null;
+ this.relDataType = relDataType;
}
}
}
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/util/testoperator/AssertionErrorOperatorConversion.java
b/sql/src/test/java/org/apache/druid/sql/calcite/util/testoperator/AssertionErrorOperatorConversion.java
index 6d4aa44e5e7..116e965e79e 100644
---
a/sql/src/test/java/org/apache/druid/sql/calcite/util/testoperator/AssertionErrorOperatorConversion.java
+++
b/sql/src/test/java/org/apache/druid/sql/calcite/util/testoperator/AssertionErrorOperatorConversion.java
@@ -22,6 +22,7 @@ package org.apache.druid.sql.calcite.util.testoperator;
import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.SqlTypeFamily;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.druid.segment.column.RowSignature;
import org.apache.druid.sql.calcite.expression.DruidExpression;
@@ -45,7 +46,7 @@ public class AssertionErrorOperatorConversion implements
SqlOperatorConversion
{
private static final SqlOperator OPERATOR =
OperatorConversions.operatorBuilder("assertion_error")
- .operandTypes()
+ .operandTypes(new SqlTypeFamily[0])
.returnTypeNonNull(SqlTypeName.BIGINT)
.build();
diff --git
a/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/numeric_agg_string_args.iq
b/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/numeric_agg_string_args.iq
new file mode 100644
index 00000000000..c6246a74565
--- /dev/null
+++
b/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/numeric_agg_string_args.iq
@@ -0,0 +1,167 @@
+!use druidtest:///
+!set outputformat mysql
+
+# Numeric aggregators on STRING columns and string literals.
+# A STRING column is implicitly cast to a number for SUM, AVG and COUNT; MIN
and MAX reject it.
+
+# Case 1: SUM(dim1), STRING column.
+SELECT SUM(dim1) FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| 13.1 |
++--------+
+(1 row)
+
+!ok
+
+# Case 2: MIN(dim1), STRING column.
+SELECT MIN(dim1) FROM foo;
+Aggregation [MIN] does not support type [STRING], column [v0]
+!error
+
+# Case 3: MAX(dim1), STRING column.
+SELECT MAX(dim1) FROM foo;
+Aggregation [MAX] does not support type [STRING], column [v0]
+!error
+
+# Case 4: AVG(dim1), STRING column.
+SELECT AVG(dim1) FROM foo;
++-------------------+
+| EXPR$0 |
++-------------------+
+| 4.366666666666666 |
++-------------------+
+(1 row)
+
+!ok
+
+# Case 5: COUNT(dim1), STRING column.
+SELECT COUNT(dim1) FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| 6 |
++--------+
+(1 row)
+
+!ok
+
+# Case 6: COUNT(DISTINCT dim1), STRING column.
+SELECT COUNT(DISTINCT dim1) FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| 6 |
++--------+
+(1 row)
+
+!ok
+
+# Case 7: SUM('1.5'), numeric string literal.
+SELECT SUM('1.5') FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| 9.0 |
++--------+
+(1 row)
+
+!ok
+
+# Case 8: MIN('1.5'), numeric string literal.
+SELECT MIN('1.5') FROM foo;
+Aggregation [MIN] does not support type [STRING], column [v0]
+!error
+
+# Case 9: MAX('1.5'), numeric string literal.
+SELECT MAX('1.5') FROM foo;
+Aggregation [MAX] does not support type [STRING], column [v0]
+!error
+
+# Case 10: AVG('1.5'), numeric string literal.
+SELECT AVG('1.5') FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| 1.5 |
++--------+
+(1 row)
+
+!ok
+
+# Case 11: COUNT('1.5'), numeric string literal.
+SELECT COUNT('1.5') FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| 6 |
++--------+
+(1 row)
+
+!ok
+
+# Case 12: COUNT(DISTINCT '1.5'), numeric string literal.
+SELECT COUNT(DISTINCT '1.5') FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| 1 |
++--------+
+(1 row)
+
+!ok
+
+# Case 13: SUM('abc'), non-numeric string literal.
+SELECT SUM('abc') FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| |
++--------+
+(1 row)
+
+!ok
+
+# Case 14: MIN('abc'), non-numeric string literal.
+SELECT MIN('abc') FROM foo;
+Aggregation [MIN] does not support type [STRING], column [v0]
+!error
+
+# Case 15: MAX('abc'), non-numeric string literal.
+SELECT MAX('abc') FROM foo;
+Aggregation [MAX] does not support type [STRING], column [v0]
+!error
+
+# Case 16: AVG('abc'), non-numeric string literal.
+SELECT AVG('abc') FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| |
++--------+
+(1 row)
+
+!ok
+
+# Case 17: COUNT('abc'), non-numeric string literal.
+SELECT COUNT('abc') FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| 6 |
++--------+
+(1 row)
+
+!ok
+
+# Case 18: COUNT(DISTINCT 'abc'), non-numeric string literal.
+SELECT COUNT(DISTINCT 'abc') FROM foo;
++--------+
+| EXPR$0 |
++--------+
+| 1 |
++--------+
+(1 row)
+
+!ok
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
