This is an automated email from the ASF dual-hosted git repository.
adarshsanjeev pushed a commit to branch 30.0.0
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/30.0.0 by this push:
new 9fb43e0cf73 Fix ExpressionPredicateIndexSupplier numeric
replace-with-default behavior. (#16448) (#16454)
9fb43e0cf73 is described below
commit 9fb43e0cf73e2ab5b39a13e8ad4d99d5a8de397a
Author: Adarsh Sanjeev <[email protected]>
AuthorDate: Thu May 16 12:52:13 2024 +0530
Fix ExpressionPredicateIndexSupplier numeric replace-with-default behavior.
(#16448) (#16454)
* Fix ExpressionPredicateIndexSupplier numeric replace-with-default
behavior.
In replace-with-default mode, null numeric values from the index should be
interpreted as zeroes by expressions. This makes the index supplier more
consistent with the behavior of the selectors created by the expression
virtual column.
* Fix test case.
Co-authored-by: Gian Merlino <[email protected]>
---
.../expr/ExpressionPredicateIndexSupplier.java | 29 +++-
.../druid/segment/filter/BaseFilterTest.java | 1 +
.../druid/segment/filter/BoundFilterTest.java | 16 ++
.../druid/segment/filter/NullFilterTests.java | 12 ++
.../druid/segment/filter/RangeFilterTests.java | 15 ++
.../sql/calcite/CalciteNestedDataQueryTest.java | 177 +++++++++++++++++++++
6 files changed, 248 insertions(+), 2 deletions(-)
diff --git
a/processing/src/main/java/org/apache/druid/math/expr/ExpressionPredicateIndexSupplier.java
b/processing/src/main/java/org/apache/druid/math/expr/ExpressionPredicateIndexSupplier.java
index 0afe36229e0..48d4deaa1b2 100644
---
a/processing/src/main/java/org/apache/druid/math/expr/ExpressionPredicateIndexSupplier.java
+++
b/processing/src/main/java/org/apache/druid/math/expr/ExpressionPredicateIndexSupplier.java
@@ -20,6 +20,7 @@
package org.apache.druid.math.expr;
import org.apache.druid.collections.bitmap.ImmutableBitmap;
+import org.apache.druid.common.config.NullHandling;
import org.apache.druid.query.filter.DruidDoublePredicate;
import org.apache.druid.query.filter.DruidFloatPredicate;
import org.apache.druid.query.filter.DruidLongPredicate;
@@ -75,8 +76,32 @@ public class ExpressionPredicateIndexSupplier implements
ColumnIndexSupplier
@Override
public BitmapColumnIndex forPredicate(DruidPredicateFactory matcherFactory)
{
- final java.util.function.Function<Object, ExprEval<?>> evalFunction =
- inputValue -> expr.eval(InputBindings.forInputSupplier(inputColumn,
inputType, () -> inputValue));
+ final java.util.function.Function<Object, ExprEval<?>> evalFunction;
+
+ if (NullHandling.sqlCompatible()) {
+ evalFunction =
+ inputValue ->
expr.eval(InputBindings.forInputSupplier(inputColumn, inputType, () ->
inputValue));
+ } else {
+ switch (inputType.getType()) {
+ case LONG:
+ evalFunction =
+ inputValue -> expr.eval(
+ InputBindings.forInputSupplier(inputColumn, inputType, ()
-> inputValue == null ? 0L : inputValue)
+ );
+ break;
+
+ case DOUBLE:
+ evalFunction =
+ inputValue -> expr.eval(
+ InputBindings.forInputSupplier(inputColumn, inputType, ()
-> inputValue == null ? 0.0 : inputValue)
+ );
+ break;
+
+ default:
+ evalFunction =
+ inputValue ->
expr.eval(InputBindings.forInputSupplier(inputColumn, inputType, () ->
inputValue));
+ }
+ }
return new
DictionaryScanningBitmapIndex(inputColumnIndexes.getCardinality())
{
diff --git
a/processing/src/test/java/org/apache/druid/segment/filter/BaseFilterTest.java
b/processing/src/test/java/org/apache/druid/segment/filter/BaseFilterTest.java
index a907bc7cd02..d9eb693ab76 100644
---
a/processing/src/test/java/org/apache/druid/segment/filter/BaseFilterTest.java
+++
b/processing/src/test/java/org/apache/druid/segment/filter/BaseFilterTest.java
@@ -143,6 +143,7 @@ public abstract class BaseFilterTest extends
InitializedNullHandlingTest
new ExpressionVirtualColumn("vd0", "d0", ColumnType.DOUBLE,
TestExprMacroTable.INSTANCE),
new ExpressionVirtualColumn("vf0", "f0", ColumnType.FLOAT,
TestExprMacroTable.INSTANCE),
new ExpressionVirtualColumn("vl0", "l0", ColumnType.LONG,
TestExprMacroTable.INSTANCE),
+ new ExpressionVirtualColumn("vd0-nvl-2", "nvl(vd0, 2.0)",
ColumnType.DOUBLE, TestExprMacroTable.INSTANCE),
new ExpressionVirtualColumn("vd0-add-sub", "d0 + (d0 - d0)",
ColumnType.DOUBLE, TestExprMacroTable.INSTANCE),
new ExpressionVirtualColumn("vf0-add-sub", "f0 + (f0 - f0)",
ColumnType.FLOAT, TestExprMacroTable.INSTANCE),
new ExpressionVirtualColumn("vl0-add-sub", "l0 + (l0 - l0)",
ColumnType.LONG, TestExprMacroTable.INSTANCE),
diff --git
a/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java
b/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java
index 6cba418a311..036abc13cd6 100644
---
a/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java
+++
b/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java
@@ -816,6 +816,22 @@ public class BoundFilterTest extends BaseFilterTest
? ImmutableList.of("0", "3", "7")
: ImmutableList.of("0")
);
+
+ assertFilterMatches(
+ new BoundDimFilter(
+ "vd0-nvl-2",
+ "0",
+ null,
+ true,
+ false,
+ false,
+ null,
+ StringComparators.NUMERIC
+ ),
+ NullHandling.replaceWithDefault()
+ ? ImmutableList.of("1", "3", "4", "5", "6")
+ : ImmutableList.of("1", "2", "3", "4", "5", "6", "7")
+ );
}
@Test
diff --git
a/processing/src/test/java/org/apache/druid/segment/filter/NullFilterTests.java
b/processing/src/test/java/org/apache/druid/segment/filter/NullFilterTests.java
index 5f9de867eca..767f450fc2e 100644
---
a/processing/src/test/java/org/apache/druid/segment/filter/NullFilterTests.java
+++
b/processing/src/test/java/org/apache/druid/segment/filter/NullFilterTests.java
@@ -229,6 +229,12 @@ public class NullFilterTests
ImmutableList.of("0", "1", "2", "3", "4", "5")
);
+ assertFilterMatches(NullFilter.forColumn("vd0-nvl-2"),
ImmutableList.of());
+ assertFilterMatches(
+ NotDimFilter.of(NullFilter.forColumn("vd0-nvl-2")),
+ ImmutableList.of("0", "1", "2", "3", "4", "5")
+ );
+
assertFilterMatches(NullFilter.forColumn("vf0-add-sub"),
ImmutableList.of());
assertFilterMatches(
NotDimFilter.of(NullFilter.forColumn("vf0-add-sub")),
@@ -274,6 +280,12 @@ public class NullFilterTests
assertFilterMatches(NullFilter.forColumn("vl0"),
ImmutableList.of("3"));
assertFilterMatches(NotDimFilter.of(NullFilter.forColumn("vl0")),
ImmutableList.of("0", "1", "2", "4", "5"));
+ assertFilterMatches(NullFilter.forColumn("vd0-nvl-2"),
ImmutableList.of());
+ assertFilterMatches(
+ NotDimFilter.of(NullFilter.forColumn("vd0-nvl-2")),
+ ImmutableList.of("0", "1", "2", "3", "4", "5")
+ );
+
if (NullHandling.sqlCompatible()) {
// these fail in default value mode that cannot be tested as numeric
default values becuase of type
// mismatch for subtract operation
diff --git
a/processing/src/test/java/org/apache/druid/segment/filter/RangeFilterTests.java
b/processing/src/test/java/org/apache/druid/segment/filter/RangeFilterTests.java
index 424fb28b086..e2c0be5909c 100644
---
a/processing/src/test/java/org/apache/druid/segment/filter/RangeFilterTests.java
+++
b/processing/src/test/java/org/apache/druid/segment/filter/RangeFilterTests.java
@@ -895,6 +895,21 @@ public class RangeFilterTests
: ImmutableList.of("0")
);
+ assertFilterMatches(
+ new RangeFilter(
+ "vd0-nvl-2",
+ ColumnType.DOUBLE,
+ 0.0,
+ null,
+ true,
+ false,
+ null
+ ),
+ NullHandling.replaceWithDefault()
+ ? ImmutableList.of("1", "3", "4", "5", "6")
+ : ImmutableList.of("1", "2", "3", "4", "5", "6", "7")
+ );
+
if (NullHandling.sqlCompatible() || canTestNumericNullsAsDefaultValues) {
// these fail in default value mode that cannot be tested as numeric
default values becuase of type
// mismatch for subtract operation
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteNestedDataQueryTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteNestedDataQueryTest.java
index 11266b92e6c..3b339c9b6ac 100644
---
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteNestedDataQueryTest.java
+++
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteNestedDataQueryTest.java
@@ -7134,4 +7134,181 @@ public class CalciteNestedDataQueryTest extends
BaseCalciteQueryTest
.build()
);
}
+
+ @Test
+ public void testNvlJsonValueDoubleMissingColumn()
+ {
+ testQuery(
+ "SELECT\n"
+ + "JSON_VALUE(nest, '$.nonexistent' RETURNING DOUBLE),\n"
+ + "NVL(JSON_VALUE(nest, '$.nonexistent' RETURNING DOUBLE), 1.0),\n"
+ + "NVL(JSON_VALUE(nest, '$.nonexistent' RETURNING DOUBLE), 1.0) > 0\n"
+ + "FROM druid.nested\n"
+ + "WHERE NVL(JSON_VALUE(nest, '$.nonexistent' RETURNING DOUBLE), 1.0)
> 0\n"
+ + "LIMIT 1",
+ ImmutableList.of(
+ newScanQueryBuilder()
+ .dataSource(DATA_SOURCE)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .virtualColumns(
+ expressionVirtualColumn("v0", "nvl(\"v1\",1.0)",
ColumnType.DOUBLE),
+ new NestedFieldVirtualColumn(
+ "nest",
+ "$.nonexistent",
+ "v1",
+ ColumnType.DOUBLE
+ ),
+ expressionVirtualColumn("v2", "notnull(nvl(\"v1\",1.0))",
ColumnType.LONG)
+ )
+ .filters(range("v0", ColumnType.LONG,
NullHandling.sqlCompatible() ? 0.0 : "0", null, true, false))
+ .limit(1)
+ .columns("v0", "v1", "v2")
+ .build()
+ ),
+ NullHandling.sqlCompatible()
+ ? ImmutableList.of(new Object[]{null, 1.0, true})
+ : ImmutableList.of(),
+ RowSignature.builder()
+ .add("EXPR$0", ColumnType.DOUBLE)
+ .add("EXPR$1", ColumnType.DOUBLE)
+ .add("EXPR$2", ColumnType.LONG)
+ .build()
+ );
+ }
+
+ @Test
+ public void testNvlJsonValueDoubleSometimesMissing()
+ {
+ testQuery(
+ "SELECT\n"
+ + "JSON_VALUE(nest, '$.y' RETURNING DOUBLE),\n"
+ + "NVL(JSON_VALUE(nest, '$.y' RETURNING DOUBLE), 1.0),\n"
+ + "NVL(JSON_VALUE(nest, '$.y' RETURNING DOUBLE), 1.0) > 0,\n"
+ + "NVL(JSON_VALUE(nest, '$.y' RETURNING DOUBLE), 1.0) = 1.0\n"
+ + "FROM druid.nested",
+ ImmutableList.of(
+ newScanQueryBuilder()
+ .dataSource(DATA_SOURCE)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .virtualColumns(
+ new NestedFieldVirtualColumn("nest", "$.y", "v0",
ColumnType.DOUBLE),
+ expressionVirtualColumn("v1", "nvl(\"v0\",1.0)",
ColumnType.DOUBLE),
+ expressionVirtualColumn("v2", "(nvl(\"v0\",1.0) > 0)",
ColumnType.LONG),
+ expressionVirtualColumn("v3", "(nvl(\"v0\",1.0) == 1.0)",
ColumnType.LONG)
+ )
+ .columns("v0", "v1", "v2", "v3")
+ .build()
+ ),
+ NullHandling.sqlCompatible()
+ ? ImmutableList.of(
+ new Object[]{2.02, 2.02, true, false},
+ new Object[]{null, 1.0, true, true},
+ new Object[]{3.03, 3.03, true, false},
+ new Object[]{null, 1.0, true, true},
+ new Object[]{null, 1.0, true, true},
+ new Object[]{2.02, 2.02, true, false},
+ new Object[]{null, 1.0, true, true}
+ )
+ : ImmutableList.of(
+ new Object[]{2.02, 2.02, true, false},
+ new Object[]{null, 0.0, false, false},
+ new Object[]{3.03, 3.03, true, false},
+ new Object[]{null, 0.0, false, false},
+ new Object[]{null, 0.0, false, false},
+ new Object[]{2.02, 2.02, true, false},
+ new Object[]{null, 0.0, false, false}
+ ),
+ RowSignature.builder()
+ .add("EXPR$0", ColumnType.DOUBLE)
+ .add("EXPR$1", ColumnType.DOUBLE)
+ .add("EXPR$2", ColumnType.LONG)
+ .add("EXPR$3", ColumnType.LONG)
+ .build()
+ );
+ }
+
+ @Test
+ public void testNvlJsonValueDoubleSometimesMissingRangeFilter()
+ {
+ testQuery(
+ "SELECT\n"
+ + "JSON_VALUE(nest, '$.y' RETURNING DOUBLE),\n"
+ + "NVL(JSON_VALUE(nest, '$.y' RETURNING DOUBLE), 1.0),\n"
+ + "NVL(JSON_VALUE(nest, '$.y' RETURNING DOUBLE), 1.0) > 0\n"
+ + "FROM druid.nested\n"
+ + "WHERE NVL(JSON_VALUE(nest, '$.y' RETURNING DOUBLE), 1.0) > 0",
+ ImmutableList.of(
+ newScanQueryBuilder()
+ .dataSource(DATA_SOURCE)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .virtualColumns(
+ expressionVirtualColumn("v0", "nvl(\"v1\",1.0)",
ColumnType.DOUBLE),
+ new NestedFieldVirtualColumn("nest", "$.y", "v1",
ColumnType.DOUBLE),
+ expressionVirtualColumn("v2", "notnull(nvl(\"v1\",1.0))",
ColumnType.LONG)
+ )
+ .filters(range("v0", ColumnType.LONG,
NullHandling.sqlCompatible() ? 0.0 : "0", null, true, false))
+ .columns("v0", "v1", "v2")
+ .build()
+ ),
+ NullHandling.sqlCompatible()
+ ? ImmutableList.of(
+ new Object[]{2.02, 2.02, true},
+ new Object[]{null, 1.0, true},
+ new Object[]{3.03, 3.03, true},
+ new Object[]{null, 1.0, true},
+ new Object[]{null, 1.0, true},
+ new Object[]{2.02, 2.02, true},
+ new Object[]{null, 1.0, true}
+ )
+ : ImmutableList.of(
+ new Object[]{2.02, 2.02, true},
+ new Object[]{3.03, 3.03, true},
+ new Object[]{2.02, 2.02, true}
+ ),
+ RowSignature.builder()
+ .add("EXPR$0", ColumnType.DOUBLE)
+ .add("EXPR$1", ColumnType.DOUBLE)
+ .add("EXPR$2", ColumnType.LONG)
+ .build()
+ );
+ }
+
+ @Test
+ public void testNvlJsonValueDoubleSometimesMissingEqualityFilter()
+ {
+ testQuery(
+ "SELECT\n"
+ + "JSON_VALUE(nest, '$.y' RETURNING DOUBLE),\n"
+ + "NVL(JSON_VALUE(nest, '$.y' RETURNING DOUBLE), 1.0),\n"
+ + "NVL(JSON_VALUE(nest, '$.y' RETURNING DOUBLE), 1.0) > 0\n"
+ + "FROM druid.nested\n"
+ + "WHERE NVL(JSON_VALUE(nest, '$.y' RETURNING DOUBLE), 1.0) = 1.0",
+ ImmutableList.of(
+ newScanQueryBuilder()
+ .dataSource(DATA_SOURCE)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .virtualColumns(
+ expressionVirtualColumn("v0", "nvl(\"v1\",1.0)",
ColumnType.DOUBLE),
+ new NestedFieldVirtualColumn("nest", "$.y", "v1",
ColumnType.DOUBLE),
+ expressionVirtualColumn("v2", "notnull(nvl(\"v1\",1.0))",
ColumnType.LONG)
+ )
+ .filters(equality("v0", 1.0, ColumnType.DOUBLE))
+ .columns("v0", "v1", "v2")
+ .build()
+ ),
+ NullHandling.sqlCompatible()
+ ? ImmutableList.of(
+ new Object[]{null, 1.0, true},
+ new Object[]{null, 1.0, true},
+ new Object[]{null, 1.0, true},
+ new Object[]{null, 1.0, true}
+ )
+ : ImmutableList.of(),
+ RowSignature.builder()
+ .add("EXPR$0", ColumnType.DOUBLE)
+ .add("EXPR$1", ColumnType.DOUBLE)
+ .add("EXPR$2", ColumnType.LONG)
+ .build()
+ );
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
