From 6648e67d8c9568d5936f82b495cc24dea5510762 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Fri, 4 Sep 2020 01:29:59 +0530 Subject: [PATCH 01/12] Support SearchQueryDimFilter in sql via new methods --- .../builtin/ContainsOperatorConversion.java | 126 ++++++++++++++++++ .../calcite/planner/DruidOperatorTable.java | 3 + .../expression/ExpressionTestHelper.java | 15 ++- .../calcite/expression/ExpressionsTest.java | 105 +++++++++++++++ 4 files changed, 244 insertions(+), 5 deletions(-) create mode 100644 sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java new file mode 100644 index 000000000000..74d85f069de0 --- /dev/null +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java @@ -0,0 +1,126 @@ +/* + * 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.builtin; + +import org.apache.calcite.rex.RexCall; +import org.apache.calcite.rex.RexLiteral; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.sql.SqlFunction; +import org.apache.calcite.sql.SqlFunctionCategory; +import org.apache.calcite.sql.type.SqlTypeFamily; +import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.query.filter.DimFilter; +import org.apache.druid.query.filter.SearchQueryDimFilter; +import org.apache.druid.query.search.ContainsSearchQuerySpec; +import org.apache.druid.query.search.SearchQuerySpec; +import org.apache.druid.segment.VirtualColumn; +import org.apache.druid.segment.column.RowSignature; +import org.apache.druid.sql.calcite.expression.DirectOperatorConversion; +import org.apache.druid.sql.calcite.expression.DruidExpression; +import org.apache.druid.sql.calcite.expression.Expressions; +import org.apache.druid.sql.calcite.expression.OperatorConversions; +import org.apache.druid.sql.calcite.expression.SqlOperatorConversion; +import org.apache.druid.sql.calcite.planner.PlannerContext; +import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry; + +import javax.annotation.Nullable; +import java.util.List; + +public class ContainsOperatorConversion extends DirectOperatorConversion +{ + private static final String CASE_SENSITIVE_FN_NAME = "contains"; + private static final String CASE_INSENSITIVE_FN_NAME = "icontains"; + private final boolean caseSensitive; + + public ContainsOperatorConversion( + final SqlFunction sqlFunction, + final String functionName, + final boolean caseSensitive + ) + { + super(sqlFunction, functionName); + this.caseSensitive = caseSensitive; + } + + public static SqlOperatorConversion createOperatorConversion(boolean caseSensitive) + { + final String functionName = caseSensitive ? CASE_SENSITIVE_FN_NAME : CASE_INSENSITIVE_FN_NAME; + final SqlFunction sqlFunction = createSqlFunction(functionName); + return new ContainsOperatorConversion(sqlFunction, functionName, caseSensitive); + } + + private static SqlFunction createSqlFunction(final String functionName) + { + return OperatorConversions + .operatorBuilder(StringUtils.toUpperCase(functionName)) + .operandTypes(SqlTypeFamily.CHARACTER, SqlTypeFamily.CHARACTER) + .requiredOperands(2) + .literalOperands(1) + .returnTypeNonNull(SqlTypeName.BOOLEAN) + .functionCategory(SqlFunctionCategory.STRING) + .build(); + } + + @Nullable + @Override + public DimFilter toDruidFilter( + PlannerContext plannerContext, + RowSignature rowSignature, + @Nullable VirtualColumnRegistry virtualColumnRegistry, + RexNode rexNode + ) + { + final List operands = ((RexCall) rexNode).getOperands(); + final DruidExpression druidExpression = Expressions.toDruidExpression( + plannerContext, + rowSignature, + operands.get(0) + ); + + if (druidExpression == null) { + return null; + } + + final String search = RexLiteral.stringValue(operands.get(1)); + final SearchQuerySpec spec = new ContainsSearchQuerySpec(search, caseSensitive); + + if (druidExpression.isSimpleExtraction()) { + return new SearchQueryDimFilter( + druidExpression.getSimpleExtraction().getColumn(), + spec, + druidExpression.getSimpleExtraction().getExtractionFn(), + null + ); + } else if (virtualColumnRegistry != null) { + VirtualColumn v = virtualColumnRegistry.getOrCreateVirtualColumnForExpression( + plannerContext, + druidExpression, + operands.get(0).getType() + ); + + return new SearchQueryDimFilter( + v.getOutputName(), spec, null, null); + } else { + return null; + } + } + +} diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java index 3f7699fa9f18..0188bda22d85 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java @@ -61,6 +61,7 @@ import org.apache.druid.sql.calcite.expression.builtin.CastOperatorConversion; import org.apache.druid.sql.calcite.expression.builtin.CeilOperatorConversion; import org.apache.druid.sql.calcite.expression.builtin.ConcatOperatorConversion; +import org.apache.druid.sql.calcite.expression.builtin.ContainsOperatorConversion; import org.apache.druid.sql.calcite.expression.builtin.DateTruncOperatorConversion; import org.apache.druid.sql.calcite.expression.builtin.ExtractOperatorConversion; import org.apache.druid.sql.calcite.expression.builtin.FloorOperatorConversion; @@ -181,6 +182,8 @@ public class DruidOperatorTable implements SqlOperatorTable .add(new AliasedOperatorConversion(new TruncateOperatorConversion(), "TRUNC")) .add(new LPadOperatorConversion()) .add(new RPadOperatorConversion()) + .add(ContainsOperatorConversion.createOperatorConversion(true)) + .add(ContainsOperatorConversion.createOperatorConversion(false)) .build(); private static final List VALUE_COERCION_OPERATOR_CONVERSIONS = diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionTestHelper.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionTestHelper.java index 1d842175ef46..6e05ce8b49cf 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionTestHelper.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionTestHelper.java @@ -38,7 +38,9 @@ import org.apache.druid.segment.RowAdapters; import org.apache.druid.segment.RowBasedColumnSelectorFactory; import org.apache.druid.segment.VirtualColumn; +import org.apache.druid.segment.VirtualColumns; import org.apache.druid.segment.column.RowSignature; +import org.apache.druid.segment.virtual.VirtualizedColumnSelectorFactory; import org.apache.druid.sql.calcite.planner.Calcites; import org.apache.druid.sql.calcite.planner.PlannerConfig; import org.apache.druid.sql.calcite.planner.PlannerContext; @@ -283,11 +285,14 @@ void testFilter( ); final ValueMatcher matcher = expectedFilter.toFilter().makeMatcher( - RowBasedColumnSelectorFactory.create( - RowAdapters.standardRow(), - () -> new MapBasedRow(0L, bindings), - rowSignature, - false + new VirtualizedColumnSelectorFactory( + RowBasedColumnSelectorFactory.create( + RowAdapters.standardRow(), + () -> new MapBasedRow(0L, bindings), + rowSignature, + false + ), + VirtualColumns.create(virtualColumns) ) ); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java index 9975cffe64cf..6f0a5c7d4776 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java @@ -35,9 +35,12 @@ import org.apache.druid.query.expression.TestExprMacroTable; import org.apache.druid.query.extraction.RegexDimExtractionFn; import org.apache.druid.query.filter.RegexDimFilter; +import org.apache.druid.query.filter.SearchQueryDimFilter; +import org.apache.druid.query.search.ContainsSearchQuerySpec; import org.apache.druid.segment.column.RowSignature; import org.apache.druid.segment.column.ValueType; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; +import org.apache.druid.sql.calcite.expression.builtin.ContainsOperatorConversion; import org.apache.druid.sql.calcite.expression.builtin.DateTruncOperatorConversion; import org.apache.druid.sql.calcite.expression.builtin.LPadOperatorConversion; import org.apache.druid.sql.calcite.expression.builtin.LeftOperatorConversion; @@ -1072,6 +1075,108 @@ public void testPad() ); } + @Test + public void testContains() + { + testHelper.testFilter( + ContainsOperatorConversion.createOperatorConversion(true).calciteOperator(), + ImmutableList.of( + testHelper.makeInputRef("spacey"), + testHelper.makeLiteral("there") + ), + Collections.emptyList(), + new SearchQueryDimFilter("spacey", new ContainsSearchQuerySpec("there", true), null), + true + ); + + testHelper.testFilter( + ContainsOperatorConversion.createOperatorConversion(true).calciteOperator(), + ImmutableList.of( + testHelper.makeInputRef("spacey"), + testHelper.makeLiteral("There") + ), + Collections.emptyList(), + new SearchQueryDimFilter("spacey", new ContainsSearchQuerySpec("There", true), null), + false + ); + + testHelper.testFilter( + ContainsOperatorConversion.createOperatorConversion(false).calciteOperator(), + ImmutableList.of( + testHelper.makeInputRef("spacey"), + testHelper.makeLiteral("There") + ), + Collections.emptyList(), + new SearchQueryDimFilter("spacey", new ContainsSearchQuerySpec("There", false), null), + true + ); + + testHelper.testFilter( + ContainsOperatorConversion.createOperatorConversion(true).calciteOperator(), + ImmutableList.of( + testHelper.makeCall( + SqlStdOperatorTable.CONCAT, + testHelper.makeLiteral("what is"), + testHelper.makeInputRef("spacey") + ), + testHelper.makeLiteral("what") + ), + ImmutableList.of( + new ExpressionVirtualColumn( + "v0", + "concat('what is',\"spacey\")", + ValueType.STRING, + TestExprMacroTable.INSTANCE + ) + ), + new SearchQueryDimFilter("v0", new ContainsSearchQuerySpec("what", true), null), + true + ); + + testHelper.testFilter( + ContainsOperatorConversion.createOperatorConversion(true).calciteOperator(), + ImmutableList.of( + testHelper.makeCall( + SqlStdOperatorTable.CONCAT, + testHelper.makeLiteral("what is"), + testHelper.makeInputRef("spacey") + ), + testHelper.makeLiteral("there") + ), + ImmutableList.of( + new ExpressionVirtualColumn( + "v0", + "concat('what is',\"spacey\")", + ValueType.STRING, + TestExprMacroTable.INSTANCE + ) + ), + new SearchQueryDimFilter("v0", new ContainsSearchQuerySpec("there", true), null), + true + ); + + testHelper.testFilter( + ContainsOperatorConversion.createOperatorConversion(false).calciteOperator(), + ImmutableList.of( + testHelper.makeCall( + SqlStdOperatorTable.CONCAT, + testHelper.makeLiteral("what is"), + testHelper.makeInputRef("spacey") + ), + testHelper.makeLiteral("What") + ), + ImmutableList.of( + new ExpressionVirtualColumn( + "v0", + "concat('what is',\"spacey\")", + ValueType.STRING, + TestExprMacroTable.INSTANCE + ) + ), + new SearchQueryDimFilter("v0", new ContainsSearchQuerySpec("What", false), null), + true + ); + } @Test public void testTimeFloor() From 6e86280ad4b9e006fb58bc54e818cd89f4fe1bae Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Fri, 4 Sep 2020 18:19:45 +0530 Subject: [PATCH 02/12] Contains is a reserved word --- docs/querying/sql.md | 2 ++ integration-tests/docker/docker-compose.base.yml | 3 ++- .../expression/builtin/ContainsOperatorConversion.java | 4 ++-- website/.spelling | 2 ++ 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/querying/sql.md b/docs/querying/sql.md index 1595c274db7f..fea5f245e1a3 100644 --- a/docs/querying/sql.md +++ b/docs/querying/sql.md @@ -561,6 +561,8 @@ The [DataSketches extension](../development/extensions-core/datasketches-extensi |`COALESCE(value1, value2, ...)`|Returns the first value that is neither NULL nor empty string.| |`NVL(expr,expr-for-null)`|Returns 'expr-for-null' if 'expr' is null (or empty string for string type).| |`BLOOM_FILTER_TEST(, )`|Returns true if the value is contained in a Base64-serialized bloom filter. See the [Bloom filter extension](../development/extensions-core/bloom-filter.html) documentation for additional details.| +|`CONTAINS_STR(, str)`|Returns true if the `str` is a substring of `expr`.| +|`ICONTAINS_STR(, str)`|Returns true if the `str` is a substring of `expr`. The match is case-insensitive.| ## Multi-value string functions diff --git a/integration-tests/docker/docker-compose.base.yml b/integration-tests/docker/docker-compose.base.yml index 1f7931a623af..07927e64934a 100644 --- a/integration-tests/docker/docker-compose.base.yml +++ b/integration-tests/docker/docker-compose.base.yml @@ -187,6 +187,7 @@ services: volumes: - ${HOME}/shared:/shared - ./service-supervisords/druid.conf:/usr/lib/druid/conf/druid.conf + - ../../sql/target/druid-sql-0.20.0-SNAPSHOT.jar:/usr/local/druid/lib/druid-sql-0.20.0-SNAPSHOT.jar env_file: - ./environment-configs/common - ./environment-configs/broker @@ -268,4 +269,4 @@ networks: name: druid-it-net ipam: config: - - subnet: 172.172.172.0/24 \ No newline at end of file + - subnet: 172.172.172.0/24 diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java index 74d85f069de0..aede473093d9 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java @@ -46,8 +46,8 @@ public class ContainsOperatorConversion extends DirectOperatorConversion { - private static final String CASE_SENSITIVE_FN_NAME = "contains"; - private static final String CASE_INSENSITIVE_FN_NAME = "icontains"; + private static final String CASE_SENSITIVE_FN_NAME = "contains_str"; + private static final String CASE_INSENSITIVE_FN_NAME = "icontains_str"; private final boolean caseSensitive; public ContainsOperatorConversion( diff --git a/website/.spelling b/website/.spelling index 249db61de5f5..1df6aedafab1 100644 --- a/website/.spelling +++ b/website/.spelling @@ -1513,6 +1513,8 @@ total_size useApproximateCountDistinct useApproximateTopN wikipedia +CONTAINS_STR +ICONTAINS_STR - ../docs/querying/timeseriesquery.md fieldName1 fieldName2 From 176d71272f6629fed5b0924b54c4f78ecef94e3f Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Fri, 4 Sep 2020 18:21:16 +0530 Subject: [PATCH 03/12] revert unnecessary change --- integration-tests/docker/docker-compose.base.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/integration-tests/docker/docker-compose.base.yml b/integration-tests/docker/docker-compose.base.yml index 07927e64934a..6b55fd110794 100644 --- a/integration-tests/docker/docker-compose.base.yml +++ b/integration-tests/docker/docker-compose.base.yml @@ -187,7 +187,6 @@ services: volumes: - ${HOME}/shared:/shared - ./service-supervisords/druid.conf:/usr/lib/druid/conf/druid.conf - - ../../sql/target/druid-sql-0.20.0-SNAPSHOT.jar:/usr/local/druid/lib/druid-sql-0.20.0-SNAPSHOT.jar env_file: - ./environment-configs/common - ./environment-configs/broker From d5a841a4a04a301daf8a839b15a03926e570d5da Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Thu, 10 Sep 2020 15:00:16 +0530 Subject: [PATCH 04/12] Fix toDruidExpression method --- docs/querying/sql.md | 4 +- .../builtin/ContainsOperatorConversion.java | 37 +++++++++++++++---- .../calcite/planner/DruidOperatorTable.java | 4 +- .../calcite/expression/ExpressionsTest.java | 12 +++--- 4 files changed, 40 insertions(+), 17 deletions(-) diff --git a/docs/querying/sql.md b/docs/querying/sql.md index fea5f245e1a3..141950d2e1dd 100644 --- a/docs/querying/sql.md +++ b/docs/querying/sql.md @@ -397,6 +397,8 @@ String functions accept strings, and return a type appropriate to the function. |`POSITION(needle IN haystack [FROM fromIndex])`|Returns the index of needle within haystack, with indexes starting from 1. The search will begin at fromIndex, or 1 if fromIndex is not specified. If the needle is not found, returns 0.| |`REGEXP_EXTRACT(expr, pattern, [index])`|Apply regular expression `pattern` to `expr` and extract a capture group, or `NULL` if there is no match. If index is unspecified or zero, returns the first substring that matched the pattern. The pattern may match anywhere inside `expr`; if you want to match the entire string instead, use the `^` and `$` markers at the start and end of your pattern. Note: when `druid.generic.useDefaultValueForNull = true`, it is not possible to differentiate an empty-string match from a non-match (both will return `NULL`).| |`REGEXP_LIKE(expr, pattern)`|Returns whether `expr` matches regular expression `pattern`. The pattern may match anywhere inside `expr`; if you want to match the entire string instead, use the `^` and `$` markers at the start and end of your pattern. Similar to [`LIKE`](#comparison-operators), but uses regexps instead of LIKE patterns. Especially useful in WHERE clauses.| +|`CONTAINS_STR(, str)`|Returns true if the `str` is a substring of `expr`.| +|`ICONTAINS_STR(, str)`|Returns true if the `str` is a substring of `expr`. The match is case-insensitive.| |`REPLACE(expr, pattern, replacement)`|Replaces pattern with replacement in expr, and returns the result.| |`STRPOS(haystack, needle)`|Returns the index of needle within haystack, with indexes starting from 1. If the needle is not found, returns 0.| |`SUBSTRING(expr, index, [length])`|Returns a substring of expr starting at index, with a max length, both measured in UTF-16 code units.| @@ -561,8 +563,6 @@ The [DataSketches extension](../development/extensions-core/datasketches-extensi |`COALESCE(value1, value2, ...)`|Returns the first value that is neither NULL nor empty string.| |`NVL(expr,expr-for-null)`|Returns 'expr-for-null' if 'expr' is null (or empty string for string type).| |`BLOOM_FILTER_TEST(, )`|Returns true if the value is contained in a Base64-serialized bloom filter. See the [Bloom filter extension](../development/extensions-core/bloom-filter.html) documentation for additional details.| -|`CONTAINS_STR(, str)`|Returns true if the `str` is a substring of `expr`.| -|`ICONTAINS_STR(, str)`|Returns true if the `str` is a substring of `expr`. The match is case-insensitive.| ## Multi-value string functions diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java index aede473093d9..68a98a3efd85 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java @@ -24,6 +24,7 @@ import org.apache.calcite.rex.RexNode; import org.apache.calcite.sql.SqlFunction; import org.apache.calcite.sql.SqlFunctionCategory; +import org.apache.calcite.sql.SqlOperator; import org.apache.calcite.sql.type.SqlTypeFamily; import org.apache.calcite.sql.type.SqlTypeName; import org.apache.druid.java.util.common.StringUtils; @@ -33,7 +34,6 @@ import org.apache.druid.query.search.SearchQuerySpec; import org.apache.druid.segment.VirtualColumn; import org.apache.druid.segment.column.RowSignature; -import org.apache.druid.sql.calcite.expression.DirectOperatorConversion; import org.apache.druid.sql.calcite.expression.DruidExpression; import org.apache.druid.sql.calcite.expression.Expressions; import org.apache.druid.sql.calcite.expression.OperatorConversions; @@ -44,10 +44,11 @@ import javax.annotation.Nullable; import java.util.List; -public class ContainsOperatorConversion extends DirectOperatorConversion +public class ContainsOperatorConversion implements SqlOperatorConversion { private static final String CASE_SENSITIVE_FN_NAME = "contains_str"; private static final String CASE_INSENSITIVE_FN_NAME = "icontains_str"; + private final SqlOperator operator; private final boolean caseSensitive; public ContainsOperatorConversion( @@ -56,15 +57,20 @@ public ContainsOperatorConversion( final boolean caseSensitive ) { - super(sqlFunction, functionName); + this.operator = sqlFunction; this.caseSensitive = caseSensitive; } - public static SqlOperatorConversion createOperatorConversion(boolean caseSensitive) + public static SqlOperatorConversion caseSensitive() { - final String functionName = caseSensitive ? CASE_SENSITIVE_FN_NAME : CASE_INSENSITIVE_FN_NAME; - final SqlFunction sqlFunction = createSqlFunction(functionName); - return new ContainsOperatorConversion(sqlFunction, functionName, caseSensitive); + final SqlFunction sqlFunction = createSqlFunction(CASE_SENSITIVE_FN_NAME); + return new ContainsOperatorConversion(sqlFunction, CASE_SENSITIVE_FN_NAME, true); + } + + public static SqlOperatorConversion caseInsensitive() + { + final SqlFunction sqlFunction = createSqlFunction(CASE_INSENSITIVE_FN_NAME); + return new ContainsOperatorConversion(sqlFunction, CASE_INSENSITIVE_FN_NAME, false); } private static SqlFunction createSqlFunction(final String functionName) @@ -79,6 +85,23 @@ private static SqlFunction createSqlFunction(final String functionName) .build(); } + @Override + public SqlOperator calciteOperator() + { + return operator; + } + + @Nullable + @Override + public DruidExpression toDruidExpression( + PlannerContext plannerContext, + RowSignature rowSignature, + RexNode rexNode + ) + { + return null; + } + @Nullable @Override public DimFilter toDruidFilter( diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java index 0188bda22d85..e1fb0b48fe54 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidOperatorTable.java @@ -182,8 +182,8 @@ public class DruidOperatorTable implements SqlOperatorTable .add(new AliasedOperatorConversion(new TruncateOperatorConversion(), "TRUNC")) .add(new LPadOperatorConversion()) .add(new RPadOperatorConversion()) - .add(ContainsOperatorConversion.createOperatorConversion(true)) - .add(ContainsOperatorConversion.createOperatorConversion(false)) + .add(ContainsOperatorConversion.caseSensitive()) + .add(ContainsOperatorConversion.caseInsensitive()) .build(); private static final List VALUE_COERCION_OPERATOR_CONVERSIONS = diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java index 6f0a5c7d4776..f43280d12736 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java @@ -1079,7 +1079,7 @@ public void testPad() public void testContains() { testHelper.testFilter( - ContainsOperatorConversion.createOperatorConversion(true).calciteOperator(), + ContainsOperatorConversion.caseSensitive().calciteOperator(), ImmutableList.of( testHelper.makeInputRef("spacey"), testHelper.makeLiteral("there") @@ -1090,7 +1090,7 @@ public void testContains() ); testHelper.testFilter( - ContainsOperatorConversion.createOperatorConversion(true).calciteOperator(), + ContainsOperatorConversion.caseSensitive().calciteOperator(), ImmutableList.of( testHelper.makeInputRef("spacey"), testHelper.makeLiteral("There") @@ -1101,7 +1101,7 @@ public void testContains() ); testHelper.testFilter( - ContainsOperatorConversion.createOperatorConversion(false).calciteOperator(), + ContainsOperatorConversion.caseInsensitive().calciteOperator(), ImmutableList.of( testHelper.makeInputRef("spacey"), testHelper.makeLiteral("There") @@ -1112,7 +1112,7 @@ public void testContains() ); testHelper.testFilter( - ContainsOperatorConversion.createOperatorConversion(true).calciteOperator(), + ContainsOperatorConversion.caseSensitive().calciteOperator(), ImmutableList.of( testHelper.makeCall( SqlStdOperatorTable.CONCAT, @@ -1134,7 +1134,7 @@ public void testContains() ); testHelper.testFilter( - ContainsOperatorConversion.createOperatorConversion(true).calciteOperator(), + ContainsOperatorConversion.caseSensitive().calciteOperator(), ImmutableList.of( testHelper.makeCall( SqlStdOperatorTable.CONCAT, @@ -1156,7 +1156,7 @@ public void testContains() ); testHelper.testFilter( - ContainsOperatorConversion.createOperatorConversion(false).calciteOperator(), + ContainsOperatorConversion.caseInsensitive().calciteOperator(), ImmutableList.of( testHelper.makeCall( SqlStdOperatorTable.CONCAT, From b7d3cef69b128a5dde2b83cbfe502c671ddda06f Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Thu, 10 Sep 2020 16:34:32 +0530 Subject: [PATCH 05/12] rename methods --- docs/querying/sql.md | 4 ++-- .../expression/builtin/ContainsOperatorConversion.java | 4 ++-- website/.spelling | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/querying/sql.md b/docs/querying/sql.md index 141950d2e1dd..94cfcca15d52 100644 --- a/docs/querying/sql.md +++ b/docs/querying/sql.md @@ -397,8 +397,8 @@ String functions accept strings, and return a type appropriate to the function. |`POSITION(needle IN haystack [FROM fromIndex])`|Returns the index of needle within haystack, with indexes starting from 1. The search will begin at fromIndex, or 1 if fromIndex is not specified. If the needle is not found, returns 0.| |`REGEXP_EXTRACT(expr, pattern, [index])`|Apply regular expression `pattern` to `expr` and extract a capture group, or `NULL` if there is no match. If index is unspecified or zero, returns the first substring that matched the pattern. The pattern may match anywhere inside `expr`; if you want to match the entire string instead, use the `^` and `$` markers at the start and end of your pattern. Note: when `druid.generic.useDefaultValueForNull = true`, it is not possible to differentiate an empty-string match from a non-match (both will return `NULL`).| |`REGEXP_LIKE(expr, pattern)`|Returns whether `expr` matches regular expression `pattern`. The pattern may match anywhere inside `expr`; if you want to match the entire string instead, use the `^` and `$` markers at the start and end of your pattern. Similar to [`LIKE`](#comparison-operators), but uses regexps instead of LIKE patterns. Especially useful in WHERE clauses.| -|`CONTAINS_STR(, str)`|Returns true if the `str` is a substring of `expr`.| -|`ICONTAINS_STR(, str)`|Returns true if the `str` is a substring of `expr`. The match is case-insensitive.| +|`CONTAINS_STRING(, str)`|Returns true if the `str` is a substring of `expr`.| +|`ICONTAINS_STRING(, str)`|Returns true if the `str` is a substring of `expr`. The match is case-insensitive.| |`REPLACE(expr, pattern, replacement)`|Replaces pattern with replacement in expr, and returns the result.| |`STRPOS(haystack, needle)`|Returns the index of needle within haystack, with indexes starting from 1. If the needle is not found, returns 0.| |`SUBSTRING(expr, index, [length])`|Returns a substring of expr starting at index, with a max length, both measured in UTF-16 code units.| diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java index 68a98a3efd85..bbbb745eb4f1 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java @@ -46,8 +46,8 @@ public class ContainsOperatorConversion implements SqlOperatorConversion { - private static final String CASE_SENSITIVE_FN_NAME = "contains_str"; - private static final String CASE_INSENSITIVE_FN_NAME = "icontains_str"; + private static final String CASE_SENSITIVE_FN_NAME = "contains_string"; + private static final String CASE_INSENSITIVE_FN_NAME = "icontains_string"; private final SqlOperator operator; private final boolean caseSensitive; diff --git a/website/.spelling b/website/.spelling index 1df6aedafab1..ca0ce1d7acb7 100644 --- a/website/.spelling +++ b/website/.spelling @@ -1513,8 +1513,8 @@ total_size useApproximateCountDistinct useApproximateTopN wikipedia -CONTAINS_STR -ICONTAINS_STR +CONTAINS_STRING +ICONTAINS_STRING - ../docs/querying/timeseriesquery.md fieldName1 fieldName2 From bbfe4cbfb39747c0c60960fae452c4cd6be354a6 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Thu, 10 Sep 2020 16:50:33 +0530 Subject: [PATCH 06/12] java docs --- .../expression/builtin/ContainsOperatorConversion.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java index bbbb745eb4f1..63a181d0058c 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java @@ -44,6 +44,11 @@ import javax.annotation.Nullable; import java.util.List; +/** + * Register {@code contains_string} and {@code icontains_string} functions with calcite that internally + * translate these functions into {@link SearchQueryDimFilter} with {@link ContainsSearchQuerySpec} as + * search query spec. + */ public class ContainsOperatorConversion implements SqlOperatorConversion { private static final String CASE_SENSITIVE_FN_NAME = "contains_string"; @@ -53,7 +58,6 @@ public class ContainsOperatorConversion implements SqlOperatorConversion public ContainsOperatorConversion( final SqlFunction sqlFunction, - final String functionName, final boolean caseSensitive ) { @@ -64,13 +68,13 @@ public ContainsOperatorConversion( public static SqlOperatorConversion caseSensitive() { final SqlFunction sqlFunction = createSqlFunction(CASE_SENSITIVE_FN_NAME); - return new ContainsOperatorConversion(sqlFunction, CASE_SENSITIVE_FN_NAME, true); + return new ContainsOperatorConversion(sqlFunction, true); } public static SqlOperatorConversion caseInsensitive() { final SqlFunction sqlFunction = createSqlFunction(CASE_INSENSITIVE_FN_NAME); - return new ContainsOperatorConversion(sqlFunction, CASE_INSENSITIVE_FN_NAME, false); + return new ContainsOperatorConversion(sqlFunction, false); } private static SqlFunction createSqlFunction(final String functionName) From 7e7125d36acde6c70ef1e7b4a86b805e5199d7e3 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Thu, 10 Sep 2020 22:38:38 +0530 Subject: [PATCH 07/12] Add native functions --- docs/misc/math-expr.md | 2 + .../CaseInsensitiveContainsExprMacro.java | 67 +++++++++++ .../druid/query/expression/ContainsExpr.java | 91 ++++++++++++++ .../query/expression/ContainsExprMacro.java | 66 ++++++++++ .../apache/druid/guice/ExpressionModule.java | 4 + .../builtin/ContainsOperatorConversion.java | 18 ++- .../calcite/expression/ExpressionsTest.java | 113 ++++++++++++++++++ 7 files changed, 356 insertions(+), 5 deletions(-) create mode 100644 processing/src/main/java/org/apache/druid/query/expression/CaseInsensitiveContainsExprMacro.java create mode 100644 processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java create mode 100644 processing/src/main/java/org/apache/druid/query/expression/ContainsExprMacro.java diff --git a/docs/misc/math-expr.md b/docs/misc/math-expr.md index dc356479ad58..3867fcb5718a 100644 --- a/docs/misc/math-expr.md +++ b/docs/misc/math-expr.md @@ -78,6 +78,8 @@ The following built-in functions are available. |parse_long|parse_long(string[, radix]) parses a string as a long with the given radix, or 10 (decimal) if a radix is not provided.| |regexp_extract|regexp_extract(expr, pattern[, index]) applies a regular expression pattern and extracts a capture group index, or null if there is no match. If index is unspecified or zero, returns the substring that matched the pattern. The pattern may match anywhere inside `expr`; if you want to match the entire string instead, use the `^` and `$` markers at the start and end of your pattern.| |regexp_like|regexp_like(expr, pattern) returns whether `expr` matches regular expression `pattern`. The pattern may match anywhere inside `expr`; if you want to match the entire string instead, use the `^` and `$` markers at the start and end of your pattern. | +|contains_string|contains_string(expr, string) returns whether `expr` contains `string` as a substring. This method is case-sensitive.| +|icontains_string|contains_string(expr, string) returns whether `expr` contains `string` as a substring. This method is case-insensitive.| |replace|replace(expr, pattern, replacement) replaces pattern with replacement| |substring|substring(expr, index, length) behaves like java.lang.String's substring| |right|right(expr, length) returns the rightmost length characters from a string| diff --git a/processing/src/main/java/org/apache/druid/query/expression/CaseInsensitiveContainsExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/CaseInsensitiveContainsExprMacro.java new file mode 100644 index 000000000000..1d0956ece8c3 --- /dev/null +++ b/processing/src/main/java/org/apache/druid/query/expression/CaseInsensitiveContainsExprMacro.java @@ -0,0 +1,67 @@ +/* + * 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.query.expression; + +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.math.expr.Expr; +import org.apache.druid.math.expr.ExprMacroTable; + +import java.util.List; + +/** + * This class implements a function that checks if one string contains another string. It is required that second + * string be a literal. This expression is case-insensitive. + * signature: + * long contains_string(string, string) + *

+ * Examples: + * - {@code contains_string("foobar", "bar") - 1 } + * - {@code contains_string("foobar", "car") - 0 } + * - {@code contains_string("foobar", "Bar") - 1 } + *

+ * See {@link ContainsExprMacro} for the case-sensitive version. + */ + +public class CaseInsensitiveContainsExprMacro implements ExprMacroTable.ExprMacro +{ + public static final String FN_NAME = "icontains_string"; + + @Override + public String name() + { + return FN_NAME; + } + + @Override + public Expr apply(final List args) + { + if (args.size() != 2) { + throw new IAE("Function[%s] must have 2 arguments", name()); + } + + final Expr arg = args.get(0); + final Expr searchStr = args.get(1); + + if (!ExprUtils.isStringLiteral(searchStr)) { + throw new IAE("Function[%s] substring must be a string literal", name()); + } + return new ContainsExpr(FN_NAME, arg, searchStr, false); + } +} diff --git a/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java b/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java new file mode 100644 index 000000000000..81d6e8396874 --- /dev/null +++ b/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java @@ -0,0 +1,91 @@ +/* + * 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.query.expression; + +import org.apache.druid.common.config.NullHandling; +import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.math.expr.Expr; +import org.apache.druid.math.expr.ExprEval; +import org.apache.druid.math.expr.ExprMacroTable; +import org.apache.druid.math.expr.ExprType; + +import javax.annotation.Nonnull; +import java.util.function.Function; + +/** + * {@link Expr} class returned by {@link ContainsExprMacro} and {@link CaseInsensitiveContainsExprMacro} for + * evaluating the expression. + */ +class ContainsExpr extends ExprMacroTable.BaseScalarUnivariateMacroFunctionExpr +{ + private final Function searchFunction; + private final Expr searchStrExpr; + + ContainsExpr(String functioName, Expr arg, Expr searchStrExpr, boolean caseSensitive) + { + super(functioName, arg); + this.searchStrExpr = searchStrExpr; + // Creates the function eagerly to avoid branching in eval. + this.searchFunction = createFunction(searchStrExpr, caseSensitive); + } + + private ContainsExpr(String functioName, Expr arg, Expr searchStrExpr, Function searchFunction) + { + super(functioName, arg); + this.searchFunction = searchFunction; + this.searchStrExpr = searchStrExpr; + } + + @Nonnull + @Override + public ExprEval eval(final Expr.ObjectBinding bindings) + { + final String s = NullHandling.nullToEmptyIfNeeded(arg.eval(bindings).asString()); + + if (s == null) { + // same behavior as regexp_like. + return ExprEval.of(false, ExprType.LONG); + } else { + final boolean doesContain = searchFunction.apply(s); + return ExprEval.of(doesContain, ExprType.LONG); + } + } + + @Override + public Expr visit(Expr.Shuttle shuttle) + { + Expr newArg = arg.visit(shuttle); + return shuttle.visit(new ContainsExpr(name, newArg, searchStrExpr, searchFunction)); + } + + @Override + public String stringify() + { + return StringUtils.format("%s(%s, %s)", name, arg.stringify(), searchStrExpr.stringify()); + } + + private Function createFunction(Expr searchStrExpr, boolean caseSensitive) + { + String searchStr = (String) searchStrExpr.getLiteralValue(); + return caseSensitive ? + (s -> s.contains(searchStr)) : + (s -> org.apache.commons.lang3.StringUtils.containsIgnoreCase(s, searchStr)); + } +} diff --git a/processing/src/main/java/org/apache/druid/query/expression/ContainsExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/ContainsExprMacro.java new file mode 100644 index 000000000000..56c047ee5182 --- /dev/null +++ b/processing/src/main/java/org/apache/druid/query/expression/ContainsExprMacro.java @@ -0,0 +1,66 @@ +/* + * 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.query.expression; + +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.math.expr.Expr; +import org.apache.druid.math.expr.ExprMacroTable; + +import java.util.List; + +/** + * This class implements a function that checks if one string contains another string. It is required that second + * string be a literal. This expression is case-sensitive. + * signature: + * long contains_string(string, string) + *

+ * Examples: + * - {@code contains_string("foobar", "bar") - 1 } + * - {@code contains_string("foobar", "car") - 0 } + * - {@code contains_string("foobar", "Bar") - 0 } + *

+ * See {@link CaseInsensitiveContainsExprMacro} for the case-insensitive version. + */ +public class ContainsExprMacro implements ExprMacroTable.ExprMacro +{ + public static final String FN_NAME = "contains_string"; + + @Override + public String name() + { + return FN_NAME; + } + + @Override + public Expr apply(final List args) + { + if (args.size() != 2) { + throw new IAE("Function[%s] must have 2 arguments", name()); + } + + final Expr arg = args.get(0); + final Expr searchStr = args.get(1); + + if (!ExprUtils.isStringLiteral(searchStr)) { + throw new IAE("Function[%s] substring must be a string literal", name()); + } + return new ContainsExpr(FN_NAME, arg, searchStr, true); + } +} diff --git a/server/src/main/java/org/apache/druid/guice/ExpressionModule.java b/server/src/main/java/org/apache/druid/guice/ExpressionModule.java index 9e451e8f12f9..7a25f92e37b6 100644 --- a/server/src/main/java/org/apache/druid/guice/ExpressionModule.java +++ b/server/src/main/java/org/apache/druid/guice/ExpressionModule.java @@ -25,6 +25,8 @@ import com.google.inject.multibindings.Multibinder; import org.apache.druid.initialization.DruidModule; import org.apache.druid.math.expr.ExprMacroTable; +import org.apache.druid.query.expression.CaseInsensitiveContainsExprMacro; +import org.apache.druid.query.expression.ContainsExprMacro; import org.apache.druid.query.expression.GuiceExprMacroTable; import org.apache.druid.query.expression.IPv4AddressMatchExprMacro; import org.apache.druid.query.expression.IPv4AddressParseExprMacro; @@ -52,6 +54,8 @@ public class ExpressionModule implements DruidModule .add(LikeExprMacro.class) .add(RegexpExtractExprMacro.class) .add(RegexpLikeExprMacro.class) + .add(ContainsExprMacro.class) + .add(CaseInsensitiveContainsExprMacro.class) .add(TimestampCeilExprMacro.class) .add(TimestampExtractExprMacro.class) .add(TimestampFloorExprMacro.class) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java index 63a181d0058c..fd9073aea933 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java @@ -28,6 +28,8 @@ import org.apache.calcite.sql.type.SqlTypeFamily; import org.apache.calcite.sql.type.SqlTypeName; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.query.expression.CaseInsensitiveContainsExprMacro; +import org.apache.druid.query.expression.ContainsExprMacro; import org.apache.druid.query.filter.DimFilter; import org.apache.druid.query.filter.SearchQueryDimFilter; import org.apache.druid.query.search.ContainsSearchQuerySpec; @@ -51,8 +53,6 @@ */ public class ContainsOperatorConversion implements SqlOperatorConversion { - private static final String CASE_SENSITIVE_FN_NAME = "contains_string"; - private static final String CASE_INSENSITIVE_FN_NAME = "icontains_string"; private final SqlOperator operator; private final boolean caseSensitive; @@ -67,13 +67,13 @@ public ContainsOperatorConversion( public static SqlOperatorConversion caseSensitive() { - final SqlFunction sqlFunction = createSqlFunction(CASE_SENSITIVE_FN_NAME); + final SqlFunction sqlFunction = createSqlFunction(ContainsExprMacro.FN_NAME); return new ContainsOperatorConversion(sqlFunction, true); } public static SqlOperatorConversion caseInsensitive() { - final SqlFunction sqlFunction = createSqlFunction(CASE_INSENSITIVE_FN_NAME); + final SqlFunction sqlFunction = createSqlFunction(CaseInsensitiveContainsExprMacro.FN_NAME); return new ContainsOperatorConversion(sqlFunction, false); } @@ -103,7 +103,15 @@ public DruidExpression toDruidExpression( RexNode rexNode ) { - return null; + return OperatorConversions.convertCall( + plannerContext, + rowSignature, + rexNode, + operands -> DruidExpression.fromExpression(DruidExpression.functionCall( + StringUtils.toLowerCase(operator.getName()), + operands + )) + ); } @Nullable diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java index f43280d12736..ec5dca05b9f7 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java @@ -1077,6 +1077,119 @@ public void testPad() @Test public void testContains() + { + testHelper.testExpression( + ContainsOperatorConversion.caseSensitive().calciteOperator(), + ImmutableList.of( + testHelper.makeInputRef("spacey"), + testHelper.makeLiteral("there") + ), + DruidExpression.fromExpression("contains_string(\"spacey\",'there')"), + 1L + ); + + testHelper.testExpression( + ContainsOperatorConversion.caseSensitive().calciteOperator(), + ImmutableList.of( + testHelper.makeInputRef("spacey"), + testHelper.makeLiteral("There") + ), + DruidExpression.fromExpression("contains_string(\"spacey\",'There')"), + 0L + ); + + testHelper.testExpression( + ContainsOperatorConversion.caseInsensitive().calciteOperator(), + ImmutableList.of( + testHelper.makeInputRef("spacey"), + testHelper.makeLiteral("There") + ), + DruidExpression.fromExpression("icontains_string(\"spacey\",'There')"), + 1L + ); + + testHelper.testExpression( + ContainsOperatorConversion.caseSensitive().calciteOperator(), + ImmutableList.of( + testHelper.makeCall( + SqlStdOperatorTable.CONCAT, + testHelper.makeLiteral("what is"), + testHelper.makeInputRef("spacey") + ), + testHelper.makeLiteral("what") + ), + DruidExpression.fromExpression("contains_string(concat('what is',\"spacey\"),'what')"), + 1L + ); + + testHelper.testExpression( + ContainsOperatorConversion.caseSensitive().calciteOperator(), + ImmutableList.of( + testHelper.makeCall( + SqlStdOperatorTable.CONCAT, + testHelper.makeLiteral("what is"), + testHelper.makeInputRef("spacey") + ), + testHelper.makeLiteral("there") + ), + DruidExpression.fromExpression("contains_string(concat('what is',\"spacey\"),'there')"), + 1L + ); + + testHelper.testExpression( + ContainsOperatorConversion.caseInsensitive().calciteOperator(), + ImmutableList.of( + testHelper.makeCall( + SqlStdOperatorTable.CONCAT, + testHelper.makeLiteral("what is"), + testHelper.makeInputRef("spacey") + ), + testHelper.makeLiteral("There") + ), + DruidExpression.fromExpression("icontains_string(concat('what is',\"spacey\"),'There')"), + 1L + ); + + testHelper.testExpression( + SqlStdOperatorTable.AND, + ImmutableList.of( + testHelper.makeCall( + ContainsOperatorConversion.caseSensitive().calciteOperator(), + testHelper.makeInputRef("spacey"), + testHelper.makeLiteral("there") + ), + testHelper.makeCall( + SqlStdOperatorTable.EQUALS, + testHelper.makeLiteral("yes"), + testHelper.makeLiteral("yes") + ) + ), + DruidExpression.fromExpression("(contains_string(\"spacey\",'there') && ('yes' == 'yes'))"), + 1L + ); + + testHelper.testExpression( + SqlStdOperatorTable.AND, + ImmutableList.of( + testHelper.makeCall( + ContainsOperatorConversion.caseInsensitive().calciteOperator(), + testHelper.makeInputRef("spacey"), + testHelper.makeLiteral("There") + ), + testHelper.makeCall( + SqlStdOperatorTable.EQUALS, + testHelper.makeLiteral("yes"), + testHelper.makeLiteral("yes") + ) + ), + DruidExpression.fromExpression("(icontains_string(\"spacey\",'There') && ('yes' == 'yes'))"), + 1L + ); + + } + + @Test + public void testContainsAsFilter() { testHelper.testFilter( ContainsOperatorConversion.caseSensitive().calciteOperator(), From 38306ecc759b2c35cb1aa515f787ac14ce738613 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Thu, 10 Sep 2020 22:46:36 +0530 Subject: [PATCH 08/12] revert change in dockerfile --- integration-tests/docker/docker-compose.base.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/integration-tests/docker/docker-compose.base.yml b/integration-tests/docker/docker-compose.base.yml index 6b55fd110794..e93d9dd3d27d 100644 --- a/integration-tests/docker/docker-compose.base.yml +++ b/integration-tests/docker/docker-compose.base.yml @@ -269,3 +269,4 @@ networks: ipam: config: - subnet: 172.172.172.0/24 + From 549971029d0f2659b9a4734711ea8147ede726c5 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Thu, 10 Sep 2020 22:47:48 +0530 Subject: [PATCH 09/12] remove changes from dockerfile --- integration-tests/docker/docker-compose.base.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/integration-tests/docker/docker-compose.base.yml b/integration-tests/docker/docker-compose.base.yml index e93d9dd3d27d..1f7931a623af 100644 --- a/integration-tests/docker/docker-compose.base.yml +++ b/integration-tests/docker/docker-compose.base.yml @@ -268,5 +268,4 @@ networks: name: druid-it-net ipam: config: - - subnet: 172.172.172.0/24 - + - subnet: 172.172.172.0/24 \ No newline at end of file From 1f408a72d73e5dd8fd60e03269eecae0e68f2c71 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Fri, 11 Sep 2020 17:04:58 +0530 Subject: [PATCH 10/12] More tests --- .../CaseInsensitiveContainsExprMacro.java | 4 -- .../druid/query/expression/ContainsExpr.java | 24 ++++++++-- .../query/expression/ContainsExprMacro.java | 4 -- .../builtin/ContainsOperatorConversion.java | 2 +- .../calcite/expression/ExpressionsTest.java | 47 +++++++++++++++++++ 5 files changed, 67 insertions(+), 14 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/expression/CaseInsensitiveContainsExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/CaseInsensitiveContainsExprMacro.java index 1d0956ece8c3..5f69c382b7fd 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/CaseInsensitiveContainsExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/CaseInsensitiveContainsExprMacro.java @@ -58,10 +58,6 @@ public Expr apply(final List args) final Expr arg = args.get(0); final Expr searchStr = args.get(1); - - if (!ExprUtils.isStringLiteral(searchStr)) { - throw new IAE("Function[%s] substring must be a string literal", name()); - } return new ContainsExpr(FN_NAME, arg, searchStr, false); } } diff --git a/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java b/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java index 81d6e8396874..54c7d9cbf67d 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java +++ b/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java @@ -20,6 +20,7 @@ package org.apache.druid.query.expression; import org.apache.druid.common.config.NullHandling; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprEval; @@ -41,7 +42,7 @@ class ContainsExpr extends ExprMacroTable.BaseScalarUnivariateMacroFunctionExpr ContainsExpr(String functioName, Expr arg, Expr searchStrExpr, boolean caseSensitive) { super(functioName, arg); - this.searchStrExpr = searchStrExpr; + this.searchStrExpr = validateSearchExpr(searchStrExpr, functioName); // Creates the function eagerly to avoid branching in eval. this.searchFunction = createFunction(searchStrExpr, caseSensitive); } @@ -50,7 +51,7 @@ private ContainsExpr(String functioName, Expr arg, Expr searchStrExpr, Function< { super(functioName, arg); this.searchFunction = searchFunction; - this.searchStrExpr = searchStrExpr; + this.searchStrExpr = validateSearchExpr(searchStrExpr, functioName); } @Nonnull @@ -84,8 +85,21 @@ public String stringify() private Function createFunction(Expr searchStrExpr, boolean caseSensitive) { String searchStr = (String) searchStrExpr.getLiteralValue(); - return caseSensitive ? - (s -> s.contains(searchStr)) : - (s -> org.apache.commons.lang3.StringUtils.containsIgnoreCase(s, searchStr)); + if (null == searchStr) { + return s -> false; + } + + if (caseSensitive) { + return s -> s.contains(searchStr); + } + return s -> org.apache.commons.lang3.StringUtils.containsIgnoreCase(s, searchStr); + } + + private Expr validateSearchExpr(Expr searchExpr, String functioName) + { + if (!ExprUtils.isStringLiteral(searchExpr)) { + throw new IAE("Function[%s] substring must be a string literal", functioName); + } + return searchExpr; } } diff --git a/processing/src/main/java/org/apache/druid/query/expression/ContainsExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/ContainsExprMacro.java index 56c047ee5182..a1744f63b717 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/ContainsExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/ContainsExprMacro.java @@ -57,10 +57,6 @@ public Expr apply(final List args) final Expr arg = args.get(0); final Expr searchStr = args.get(1); - - if (!ExprUtils.isStringLiteral(searchStr)) { - throw new IAE("Function[%s] substring must be a string literal", name()); - } return new ContainsExpr(FN_NAME, arg, searchStr, true); } } diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java index fd9073aea933..aca2260de949 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ContainsOperatorConversion.java @@ -56,7 +56,7 @@ public class ContainsOperatorConversion implements SqlOperatorConversion private final SqlOperator operator; private final boolean caseSensitive; - public ContainsOperatorConversion( + private ContainsOperatorConversion( final SqlFunction sqlFunction, final boolean caseSensitive ) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java index ec5dca05b9f7..891e523de514 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java @@ -1186,6 +1186,15 @@ public void testContains() 1L ); + testHelper.testExpression( + ContainsOperatorConversion.caseSensitive().calciteOperator(), + ImmutableList.of( + testHelper.makeInputRef("spacey"), + testHelper.makeLiteral("") + ), + DruidExpression.fromExpression("contains_string(\"spacey\",'')"), + NullHandling.replaceWithDefault() ? 0L : 1L + ); } @Test @@ -1289,6 +1298,44 @@ public void testContainsAsFilter() new SearchQueryDimFilter("v0", new ContainsSearchQuerySpec("What", false), null), true ); + + testHelper.testFilter( + ContainsOperatorConversion.caseSensitive().calciteOperator(), + ImmutableList.of( + testHelper.makeInputRef("spacey"), + testHelper.makeLiteral("") + ), + Collections.emptyList(), + new SearchQueryDimFilter("spacey", new ContainsSearchQuerySpec("", true), null), + true + ); + } + + @Test(expected = IAE.class) + public void testContainsIncorrectArgs() + { + testHelper.testExpression( + ContainsOperatorConversion.caseSensitive().calciteOperator(), + ImmutableList.of( + testHelper.makeInputRef("spacey") + ), + DruidExpression.fromExpression("contains_string(\"spacey\")"), + 0L + ); + } + + @Test(expected = IAE.class) + public void testContainsSecondArgNotLiteral() + { + testHelper.testExpression( + ContainsOperatorConversion.caseSensitive().calciteOperator(), + ImmutableList.of( + testHelper.makeInputRef("spacey"), + testHelper.makeInputRef("spacey") + ), + DruidExpression.fromExpression("contains_string(\"spacey\",\"spacey\")"), + 1L + ); } @Test From cfd26bdaaf9e9768e734d1e460b38e57bd44921d Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Fri, 11 Sep 2020 19:20:35 +0530 Subject: [PATCH 11/12] travis fix --- .../java/org/apache/druid/query/expression/ContainsExpr.java | 2 +- website/.spelling | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java b/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java index 54c7d9cbf67d..4cffdadb34b7 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java +++ b/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java @@ -92,7 +92,7 @@ private Function createFunction(Expr searchStrExpr, boolean cas if (caseSensitive) { return s -> s.contains(searchStr); } - return s -> org.apache.commons.lang3.StringUtils.containsIgnoreCase(s, searchStr); + return s -> org.apache.commons.lang.StringUtils.containsIgnoreCase(s, searchStr); } private Expr validateSearchExpr(Expr searchExpr, String functioName) diff --git a/website/.spelling b/website/.spelling index ca0ce1d7acb7..55f92fdfce26 100644 --- a/website/.spelling +++ b/website/.spelling @@ -1094,6 +1094,8 @@ nvl parse_long regexp_extract regexp_like +contains_string +icontains_string result1 result2 rint @@ -1513,8 +1515,6 @@ total_size useApproximateCountDistinct useApproximateTopN wikipedia -CONTAINS_STRING -ICONTAINS_STRING - ../docs/querying/timeseriesquery.md fieldName1 fieldName2 From a8ae16920d494aaafc1d6f454a644fbd096fc044 Mon Sep 17 00:00:00 2001 From: Abhishek Agarwal <1477457+abhishekagarwal87@users.noreply.github.com> Date: Fri, 11 Sep 2020 21:01:23 +0530 Subject: [PATCH 12/12] Handle null values better --- .../druid/query/expression/ContainsExpr.java | 6 +- .../CaseInsensitiveExprMacroTest.java | 156 ++++++++++++++++++ .../expression/ContainsExprMacroTest.java | 142 ++++++++++++++++ .../calcite/expression/ExpressionsTest.java | 37 ----- 4 files changed, 299 insertions(+), 42 deletions(-) create mode 100644 processing/src/test/java/org/apache/druid/query/expression/CaseInsensitiveExprMacroTest.java create mode 100644 processing/src/test/java/org/apache/druid/query/expression/ContainsExprMacroTest.java diff --git a/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java b/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java index 4cffdadb34b7..f9550f32429b 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java +++ b/processing/src/main/java/org/apache/druid/query/expression/ContainsExpr.java @@ -84,11 +84,7 @@ public String stringify() private Function createFunction(Expr searchStrExpr, boolean caseSensitive) { - String searchStr = (String) searchStrExpr.getLiteralValue(); - if (null == searchStr) { - return s -> false; - } - + String searchStr = StringUtils.nullToEmptyNonDruidDataString((String) searchStrExpr.getLiteralValue()); if (caseSensitive) { return s -> s.contains(searchStr); } diff --git a/processing/src/test/java/org/apache/druid/query/expression/CaseInsensitiveExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/CaseInsensitiveExprMacroTest.java new file mode 100644 index 000000000000..1722ad32fe08 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/expression/CaseInsensitiveExprMacroTest.java @@ -0,0 +1,156 @@ +/* + * 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.query.expression; + +import com.google.common.collect.ImmutableMap; +import org.apache.druid.common.config.NullHandling; +import org.apache.druid.math.expr.ExprEval; +import org.apache.druid.math.expr.ExprType; +import org.apache.druid.math.expr.Parser; +import org.junit.Assert; +import org.junit.Test; + +public class CaseInsensitiveExprMacroTest extends MacroTestBase +{ + public CaseInsensitiveExprMacroTest() + { + super(new CaseInsensitiveContainsExprMacro()); + } + + @Test + public void testErrorZeroArguments() + { + expectException(IllegalArgumentException.class, "Function[icontains_string] must have 2 arguments"); + eval("icontains_string()", Parser.withMap(ImmutableMap.of())); + } + + @Test + public void testErrorThreeArguments() + { + expectException(IllegalArgumentException.class, "Function[icontains_string] must have 2 arguments"); + eval("icontains_string('a', 'b', 'c')", Parser.withMap(ImmutableMap.of())); + } + + @Test + public void testMatchSearchLowerCase() + { + final ExprEval result = eval("icontains_string(a, 'OBA')", Parser.withMap(ImmutableMap.of("a", "foobar"))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testMatchSearchUpperCase() + { + final ExprEval result = eval("icontains_string(a, 'oba')", Parser.withMap(ImmutableMap.of("a", "FOOBAR"))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testNoMatch() + { + final ExprEval result = eval("icontains_string(a, 'bar')", Parser.withMap(ImmutableMap.of("a", "foo"))); + Assert.assertEquals( + ExprEval.of(false, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testNullSearch() + { + if (NullHandling.sqlCompatible()) { + expectException(IllegalArgumentException.class, "Function[icontains_string] substring must be a string literal"); + } + + final ExprEval result = eval("icontains_string(a, null)", Parser.withMap(ImmutableMap.of("a", "foo"))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testEmptyStringSearch() + { + final ExprEval result = eval("icontains_string(a, '')", Parser.withMap(ImmutableMap.of("a", "foo"))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testNullSearchOnEmptyString() + { + if (NullHandling.sqlCompatible()) { + expectException(IllegalArgumentException.class, "Function[icontains_string] substring must be a string literal"); + } + + final ExprEval result = eval("icontains_string(a, null)", Parser.withMap(ImmutableMap.of("a", ""))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testEmptyStringSearchOnEmptyString() + { + final ExprEval result = eval("icontains_string(a, '')", Parser.withMap(ImmutableMap.of("a", ""))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testNullSearchOnNull() + { + if (NullHandling.sqlCompatible()) { + expectException(IllegalArgumentException.class, "Function[icontains_string] substring must be a string literal"); + } + + final ExprEval result = eval( + "icontains_string(a, null)", + Parser.withSuppliers(ImmutableMap.of("a", () -> null)) + ); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testEmptyStringSearchOnNull() + { + final ExprEval result = eval("icontains_string(a, '')", Parser.withSuppliers(ImmutableMap.of("a", () -> null))); + Assert.assertEquals( + ExprEval.of(!NullHandling.sqlCompatible(), ExprType.LONG).value(), + result.value() + ); + } + +} diff --git a/processing/src/test/java/org/apache/druid/query/expression/ContainsExprMacroTest.java b/processing/src/test/java/org/apache/druid/query/expression/ContainsExprMacroTest.java new file mode 100644 index 000000000000..bfbff7d0ab56 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/expression/ContainsExprMacroTest.java @@ -0,0 +1,142 @@ +/* + * 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.query.expression; + +import com.google.common.collect.ImmutableMap; +import org.apache.druid.common.config.NullHandling; +import org.apache.druid.math.expr.ExprEval; +import org.apache.druid.math.expr.ExprType; +import org.apache.druid.math.expr.Parser; +import org.junit.Assert; +import org.junit.Test; + +public class ContainsExprMacroTest extends MacroTestBase +{ + public ContainsExprMacroTest() + { + super(new ContainsExprMacro()); + } + + @Test + public void testErrorZeroArguments() + { + expectException(IllegalArgumentException.class, "Function[contains_string] must have 2 arguments"); + eval("contains_string()", Parser.withMap(ImmutableMap.of())); + } + + @Test + public void testErrorThreeArguments() + { + expectException(IllegalArgumentException.class, "Function[contains_string] must have 2 arguments"); + eval("contains_string('a', 'b', 'c')", Parser.withMap(ImmutableMap.of())); + } + + @Test + public void testMatch() + { + final ExprEval result = eval("contains_string(a, 'oba')", Parser.withMap(ImmutableMap.of("a", "foobar"))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testNoMatch() + { + final ExprEval result = eval("contains_string(a, 'bar')", Parser.withMap(ImmutableMap.of("a", "foo"))); + Assert.assertEquals( + ExprEval.of(false, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testNullSearch() + { + if (NullHandling.sqlCompatible()) { + expectException(IllegalArgumentException.class, "Function[contains_string] substring must be a string literal"); + } + + final ExprEval result = eval("contains_string(a, null)", Parser.withMap(ImmutableMap.of("a", "foo"))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testEmptyStringSearch() + { + final ExprEval result = eval("contains_string(a, '')", Parser.withMap(ImmutableMap.of("a", "foo"))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testNullSearchOnEmptyString() + { + if (NullHandling.sqlCompatible()) { + expectException(IllegalArgumentException.class, "Function[contains_string] substring must be a string literal"); + } + + final ExprEval result = eval("contains_string(a, null)", Parser.withMap(ImmutableMap.of("a", ""))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testEmptyStringSearchOnEmptyString() + { + final ExprEval result = eval("contains_string(a, '')", Parser.withMap(ImmutableMap.of("a", ""))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testNullSearchOnNull() + { + if (NullHandling.sqlCompatible()) { + expectException(IllegalArgumentException.class, "Function[contains_string] substring must be a string literal"); + } + + final ExprEval result = eval("contains_string(a, null)", Parser.withSuppliers(ImmutableMap.of("a", () -> null))); + Assert.assertEquals( + ExprEval.of(true, ExprType.LONG).value(), + result.value() + ); + } + + @Test + public void testEmptyStringSearchOnNull() + { + final ExprEval result = eval("contains_string(a, '')", Parser.withSuppliers(ImmutableMap.of("a", () -> null))); + Assert.assertEquals( + ExprEval.of(!NullHandling.sqlCompatible(), ExprType.LONG).value(), + result.value() + ); + } +} diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java index 891e523de514..bf4bd4a0cb5a 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java @@ -1185,16 +1185,6 @@ public void testContains() DruidExpression.fromExpression("(icontains_string(\"spacey\",'There') && ('yes' == 'yes'))"), 1L ); - - testHelper.testExpression( - ContainsOperatorConversion.caseSensitive().calciteOperator(), - ImmutableList.of( - testHelper.makeInputRef("spacey"), - testHelper.makeLiteral("") - ), - DruidExpression.fromExpression("contains_string(\"spacey\",'')"), - NullHandling.replaceWithDefault() ? 0L : 1L - ); } @Test @@ -1311,33 +1301,6 @@ public void testContainsAsFilter() ); } - @Test(expected = IAE.class) - public void testContainsIncorrectArgs() - { - testHelper.testExpression( - ContainsOperatorConversion.caseSensitive().calciteOperator(), - ImmutableList.of( - testHelper.makeInputRef("spacey") - ), - DruidExpression.fromExpression("contains_string(\"spacey\")"), - 0L - ); - } - - @Test(expected = IAE.class) - public void testContainsSecondArgNotLiteral() - { - testHelper.testExpression( - ContainsOperatorConversion.caseSensitive().calciteOperator(), - ImmutableList.of( - testHelper.makeInputRef("spacey"), - testHelper.makeInputRef("spacey") - ), - DruidExpression.fromExpression("contains_string(\"spacey\",\"spacey\")"), - 1L - ); - } - @Test public void testTimeFloor() {