Skip to content

Commit

Permalink
Fix ExpressionPredicateIndexSupplier numeric replace-with-default beh…
Browse files Browse the repository at this point in the history 
…avior. (#16448)

* 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.
  • Loading branch information
@gianm
gianm committed May 15, 2024
1 parent c419ae5 commit 0fb0944
Show file tree
Hide file tree
Showing 6 changed files with 248 additions and 2 deletions.
29 changes: 27 additions & 2 deletions processing/src/main/java/org/apache/druid/math/expr/ExpressionPredicateIndexSupplier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -75,8 +76,32 @@ private final class ExpressionPredicateIndexes implements DruidPredicateIndexes
@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())
{
Expand Down
1 change: 1 addition & 0 deletions processing/src/test/java/org/apache/druid/segment/filter/BaseFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
16 changes: 16 additions & 0 deletions processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,22 @@ public void testVirtualNumericNullsAndZeros()
? 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
Expand Down
12 changes: 12 additions & 0 deletions processing/src/test/java/org/apache/druid/segment/filter/NullFilterTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,12 @@ public void testVirtualNumericColumnNullsAndDefaults()
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")),
Expand Down Expand Up @@ -274,6 +280,12 @@ public void testVirtualNumericColumnNullsAndDefaults()
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
Expand Down
15 changes: 15 additions & 0 deletions processing/src/test/java/org/apache/druid/segment/filter/RangeFilterTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,21 @@ public void testVirtualNumericNullsAndZeros()
: 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
Expand Down
177 changes: 177 additions & 0 deletions sql/src/test/java/org/apache/druid/sql/calcite/CalciteNestedDataQueryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7145,4 +7145,181 @@ public void testJsonValueNestedEmptyArray()
.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()
);
}
}

0 comments on commit 0fb0944

Please sign in to comment.