Skip to content

Commit

Permalink
fix issues with array_contains and array_overlap with null left side …
Browse files Browse the repository at this point in the history
…arguments (#15974)

changes:
* fix issues with array_contains and array_overlap with null left side arguments
* modify singleThreaded stuff to allow optimizing Function similar to how we do for ExprMacro - removed SingleThreadSpecializable in favor of default impl of asSingleThreaded on Expr with clear javadocs that most callers shouldn't be calling it directly and should be using Expr.singleThreaded static method which uses a shuttle and delegates to asSingleThreaded instead
* add optimized 'singleThreaded' versions of array_contains and array_overlap
* add mv_harmonize_nulls native expression to use with MV_CONTAINS and MV_OVERLAP to allow them to behave consistently with filter rewrites, coercing null and [] into [null]
* fix bug with casting rhs argument for native array_contains and array_overlap expressions
  • Loading branch information
clintropolis authored Mar 14, 2024
1 parent e9d2cac commit dd9bc37
Show file tree
Hide file tree
Showing 16 changed files with 655 additions and 106 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,12 @@ public String getFormatString()
// 40: regex filtering
"SELECT string4, COUNT(*) FROM foo WHERE REGEXP_EXTRACT(string1, '^1') IS NOT NULL OR REGEXP_EXTRACT('Z' || string2, '^Z2') IS NOT NULL GROUP BY 1",
// 41: complicated filtering
"SELECT string2, SUM(long1) FROM foo WHERE string1 = '1000' AND string5 LIKE '%1%' AND (string3 in ('1', '10', '20', '22', '32') AND long2 IN (1, 19, 21, 23, 25, 26, 46) AND double3 < 1010.0 AND double3 > 1000.0 AND (string4 = '1' OR REGEXP_EXTRACT(string1, '^1') IS NOT NULL OR REGEXP_EXTRACT('Z' || string2, '^Z2') IS NOT NULL)) GROUP BY 1 ORDER BY 2"
"SELECT string2, SUM(long1) FROM foo WHERE string1 = '1000' AND string5 LIKE '%1%' AND (string3 in ('1', '10', '20', '22', '32') AND long2 IN (1, 19, 21, 23, 25, 26, 46) AND double3 < 1010.0 AND double3 > 1000.0 AND (string4 = '1' OR REGEXP_EXTRACT(string1, '^1') IS NOT NULL OR REGEXP_EXTRACT('Z' || string2, '^Z2') IS NOT NULL)) GROUP BY 1 ORDER BY 2",
// 42: array_contains expr
"SELECT ARRAY_CONTAINS(\"multi-string3\", 100) FROM foo",
"SELECT ARRAY_CONTAINS(\"multi-string3\", ARRAY[1, 2, 10, 11, 20, 22, 30, 33, 40, 44, 50, 55, 100]) FROM foo",
"SELECT ARRAY_OVERLAP(\"multi-string3\", ARRAY[1, 100]) FROM foo",
"SELECT ARRAY_OVERLAP(\"multi-string3\", ARRAY[1, 2, 10, 11, 20, 22, 30, 33, 40, 44, 50, 55, 100]) FROM foo"
);

@Param({"5000000"})
Expand Down Expand Up @@ -275,7 +280,11 @@ public String getFormatString()
"38",
"39",
"40",
"41"
"41",
"42",
"43",
"44",
"45"
})
private String query;

Expand Down Expand Up @@ -369,8 +378,8 @@ public void setup()
.writeValueAsString(jsonMapper.readValue((String) planResult[0], List.class))
);
}
catch (JsonProcessingException e) {
throw new RuntimeException(e);
catch (JsonProcessingException ignored) {

}

try (final DruidPlanner planner = plannerFactory.createPlannerForTesting(engine, sql, ImmutableMap.of())) {
Expand All @@ -384,6 +393,9 @@ public void setup()
}
log.info("Total result row count:" + rowCounter);
}
catch (Throwable ignored) {

}
}

@TearDown(Level.Trial)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
* {@link Expr}.
*/
@Immutable
abstract class ConstantExpr<T> implements Expr, Expr.SingleThreadSpecializable
abstract class ConstantExpr<T> implements Expr
{
final ExpressionType outputType;

Expand Down Expand Up @@ -122,7 +122,7 @@ public String toString()
}

@Override
public Expr toSingleThreaded()
public Expr asSingleThreaded(InputBindingInspector inspector)
{
return new ExprEvalBasedConstantExpr<T>(realEval());
}
Expand Down
35 changes: 12 additions & 23 deletions processing/src/main/java/org/apache/druid/math/expr/Expr.java
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,16 @@ default boolean canVectorize(InputBindingInspector inspector)
return false;
}

/**
* Possibly convert the {@link Expr} into an optimized, possibly not thread-safe {@link Expr}. Does not convert
* child {@link Expr}. Most callers should use {@link Expr#singleThreaded(Expr, InputBindingInspector)} to convert
* an entire tree, which delegates to this method to translate individual nodes.
*/
default Expr asSingleThreaded(InputBindingInspector inspector)
{
return this;
}

/**
* Builds a 'vectorized' expression processor, that can operate on batches of input values for use in vectorized
* query engines.
Expand Down Expand Up @@ -769,30 +779,9 @@ private static Set<String> map(
* Returns the single-threaded version of the given expression tree.
*
* Nested expressions in the subtree are also optimized.
* Individual {@link Expr}-s which have a singleThreaded implementation via {@link SingleThreadSpecializable} are substituted.
*/
static Expr singleThreaded(Expr expr)
static Expr singleThreaded(Expr expr, InputBindingInspector inspector)
{
return expr.visit(
node -> {
if (node instanceof SingleThreadSpecializable) {
SingleThreadSpecializable canBeSingleThreaded = (SingleThreadSpecializable) node;
return canBeSingleThreaded.toSingleThreaded();
} else {
return node;
}
}
);
}

/**
* Implementing this interface allows to provide a non-threadsafe {@link Expr} implementation.
*/
interface SingleThreadSpecializable
{
/**
* Non-threadsafe version of this expression.
*/
Expr toSingleThreaded();
return expr.visit(node -> node.asSingleThreaded(inspector));
}
}
Loading

0 comments on commit dd9bc37

Please sign in to comment.