diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/CastFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/CastFunctionIT.java index 2d87fe536fc..a46cc89d0bb 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/CastFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/CastFunctionIT.java @@ -5,6 +5,7 @@ package org.opensearch.sql.ppl; +import static org.junit.Assert.assertEquals; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATATYPE_NONNUMERIC; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATATYPE_NUMERIC; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_FORMATS; @@ -19,6 +20,7 @@ import static org.opensearch.sql.util.MatcherUtils.verifySchema; import java.io.IOException; +import java.util.List; import java.util.Locale; import org.json.JSONObject; import org.junit.Test; @@ -471,4 +473,34 @@ public void testCastDoubleAsString() throws IOException { verifySchema(actual, schema("s", "string")); verifyDataRows(actual, rows("0.0")); } + + /** + * A redundant date/time cast on the filtered field changes only how the predicate is executed + * (native range query instead of a per-document script), never which rows match. Compare the + * wrapped forms against the bare-field form to keep that guarantee enforced. + */ + @Test + public void testRedundantDateCastOnFilteredFieldDoesNotChangeRows() throws IOException { + String template = + "source=%s | where %s | sort strict_date_optional_time | fields strict_date_optional_time"; + JSONObject bare = + executeQuery( + String.format( + Locale.ROOT, + template, + TEST_INDEX_DATE_FORMATS, + "strict_date_optional_time >= '1984-04-12 09:07:42'")); + + for (String wrapped : + List.of( + "timestamp(strict_date_optional_time) >= timestamp('1984-04-12 09:07:42')", + "cast(strict_date_optional_time as timestamp) >= timestamp('1984-04-12 09:07:42')")) { + JSONObject actual = + executeQuery(String.format(Locale.ROOT, template, TEST_INDEX_DATE_FORMATS, wrapped)); + assertEquals( + "wrapping the filtered field in a redundant date cast must not change the result", + bare.getJSONArray("datarows").toString(), + actual.getJSONArray("datarows").toString()); + } + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java index 62eadd7ef5e..f43f4b0594b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java @@ -76,6 +76,36 @@ public void testFilterByCompareStringTimestampPushDownExplain() throws IOExcepti + "| where birthdate < '2018-11-09 00:00:00.000000000' ")); } + /** + * Wrapping an already timestamp-typed field in timestamp() is a no-op, so the comparison must + * still push down to a native range query instead of falling back to a per-document script. + */ + @Test + public void testFilterTimestampWrappedFieldPushDownExplain() throws IOException { + String expected = loadExpectedPlan("explain_filter_push_timestamp_wrapped_field.yaml"); + assertYamlEqualsIgnoreId( + expected, + explainQueryYaml( + "source=opensearch-sql_test_index_bank" + + "| where timestamp(birthdate) > cast('2016-12-08 00:00:00' as timestamp) " + + "| where timestamp(birthdate) < cast('2018-11-09 00:00:00' as timestamp) ")); + } + + /** + * last_day() takes a single date argument and returns a date, but it changes the value, so it is + * not a redundant conversion and must not be folded to the bare field -- it stays on the script + * path rather than becoming a range query. + */ + @Test + public void testFilterLastDayOverDateFieldNoPushDownExplain() throws IOException { + String expected = loadExpectedPlan("explain_filter_last_day_no_push.yaml"); + assertYamlEqualsIgnoreId( + expected, + explainQueryYaml( + "source=opensearch-sql_test_index_date_formats | fields yyyy-MM-dd" + + "| where last_day(yyyy-MM-dd) = date('2018-11-30') ")); + } + @Test public void testFilterByCompareStringDatePushDownExplain() throws IOException { String expected = loadExpectedPlan("explain_filter_push_compare_date_string.yaml"); diff --git a/integ-test/src/test/resources/expectedOutput/calcite/explain_filter_last_day_no_push.yaml b/integ-test/src/test/resources/expectedOutput/calcite/explain_filter_last_day_no_push.yaml new file mode 100644 index 00000000000..a0154610943 --- /dev/null +++ b/integ-test/src/test/resources/expectedOutput/calcite/explain_filter_last_day_no_push.yaml @@ -0,0 +1,8 @@ +calcite: + logical: | + LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) + LogicalFilter(condition=[=(LAST_DAY($0), DATE('2018-11-30':VARCHAR))]) + LogicalProject(yyyy-MM-dd=[$83]) + CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_date_formats]]) + physical: | + CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_date_formats]], PushDownContext=[[PROJECT->[yyyy-MM-dd], SCRIPT->=(LAST_DAY($0), '2018-11-30'), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":10000,"timeout":"1m","query":{"script":{"script":{"source":"{\"langType\":\"calcite\",\"script\":\"rO0ABXQDfHsKICAib3AiOiB7CiAgICAibmFtZSI6ICI9IiwKICAgICJraW5kIjogIkVRVUFMUyIsCiAgICAic3ludGF4IjogIkJJTkFSWSIKICB9LAogICJvcGVyYW5kcyI6IFsKICAgIHsKICAgICAgIm9wIjogewogICAgICAgICJuYW1lIjogIkxBU1RfREFZIiwKICAgICAgICAia2luZCI6ICJPVEhFUl9GVU5DVElPTiIsCiAgICAgICAgInN5bnRheCI6ICJGVU5DVElPTiIKICAgICAgfSwKICAgICAgIm9wZXJhbmRzIjogWwogICAgICAgIHsKICAgICAgICAgICJkeW5hbWljUGFyYW0iOiAwLAogICAgICAgICAgInR5cGUiOiB7CiAgICAgICAgICAgICJ1ZHQiOiAiRVhQUl9EQVRFIiwKICAgICAgICAgICAgInR5cGUiOiAiVkFSQ0hBUiIsCiAgICAgICAgICAgICJudWxsYWJsZSI6IHRydWUsCiAgICAgICAgICAgICJwcmVjaXNpb24iOiAtMQogICAgICAgICAgfQogICAgICAgIH0KICAgICAgXSwKICAgICAgImNsYXNzIjogIm9yZy5vcGVuc2VhcmNoLnNxbC5leHByZXNzaW9uLmZ1bmN0aW9uLlVzZXJEZWZpbmVkRnVuY3Rpb25CdWlsZGVyJDEiLAogICAgICAidHlwZSI6IHsKICAgICAgICAidWR0IjogIkVYUFJfREFURSIsCiAgICAgICAgInR5cGUiOiAiVkFSQ0hBUiIsCiAgICAgICAgIm51bGxhYmxlIjogdHJ1ZSwKICAgICAgICAicHJlY2lzaW9uIjogLTEKICAgICAgfSwKICAgICAgImRldGVybWluaXN0aWMiOiB0cnVlLAogICAgICAiZHluYW1pYyI6IGZhbHNlCiAgICB9LAogICAgewogICAgICAiZHluYW1pY1BhcmFtIjogMSwKICAgICAgInR5cGUiOiB7CiAgICAgICAgInVkdCI6ICJFWFBSX0RBVEUiLAogICAgICAgICJ0eXBlIjogIlZBUkNIQVIiLAogICAgICAgICJudWxsYWJsZSI6IHRydWUsCiAgICAgICAgInByZWNpc2lvbiI6IC0xCiAgICAgIH0KICAgIH0KICBdCn0=\"}","lang":"opensearch_compounded_script","params":{"utcTimestamp": 0,"SOURCES":[0,2],"DIGESTS":["yyyy-MM-dd","2018-11-30"]}},"boost":1.0}},"_source":{"includes":["yyyy-MM-dd"]}}, requestedTotalSize=10000, pageSize=null, startFrom=0)]) diff --git a/integ-test/src/test/resources/expectedOutput/calcite/explain_filter_push_timestamp_wrapped_field.yaml b/integ-test/src/test/resources/expectedOutput/calcite/explain_filter_push_timestamp_wrapped_field.yaml new file mode 100644 index 00000000000..c1c443ac91b --- /dev/null +++ b/integ-test/src/test/resources/expectedOutput/calcite/explain_filter_push_timestamp_wrapped_field.yaml @@ -0,0 +1,9 @@ +calcite: + logical: | + LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) + LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12]) + LogicalFilter(condition=[<(TIMESTAMP($3), TIMESTAMP('2018-11-09 00:00:00':VARCHAR))]) + LogicalFilter(condition=[>(TIMESTAMP($3), TIMESTAMP('2016-12-08 00:00:00':VARCHAR))]) + CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]]) + physical: | + CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]], PushDownContext=[[PROJECT->[account_number, firstname, address, birthdate, gender, city, lastname, balance, employer, state, age, email, male], FILTER->SEARCH(TIMESTAMP($3), Sarg[('2016-12-08 00:00:00':VARCHAR..'2018-11-09 00:00:00':VARCHAR)]:VARCHAR), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":10000,"timeout":"1m","query":{"range":{"birthdate":{"from":"2016-12-08T00:00:00.000Z","to":"2018-11-09T00:00:00.000Z","include_lower":false,"include_upper":false,"format":"date_time","boost":1.0}}},"_source":{"includes":["account_number","firstname","address","birthdate","gender","city","lastname","balance","employer","state","age","email","male"]}}, requestedTotalSize=10000, pageSize=null, startFrom=0)]) diff --git a/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_filter_last_day_no_push.yaml b/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_filter_last_day_no_push.yaml new file mode 100644 index 00000000000..46c54883f66 --- /dev/null +++ b/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_filter_last_day_no_push.yaml @@ -0,0 +1,10 @@ +calcite: + logical: | + LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) + LogicalFilter(condition=[=(LAST_DAY($0), DATE('2018-11-30':VARCHAR))]) + LogicalProject(yyyy-MM-dd=[$83]) + CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_date_formats]]) + physical: | + EnumerableLimit(fetch=[10000]) + EnumerableCalc(expr#0..94=[{inputs}], expr#95=[LAST_DAY($t83)], expr#96=['2018-11-30':EXPR_DATE VARCHAR], expr#97=[=($t95, $t96)], yyyy-MM-dd=[$t83], $condition=[$t97]) + CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_date_formats]]) diff --git a/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_filter_push_timestamp_wrapped_field.yaml b/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_filter_push_timestamp_wrapped_field.yaml new file mode 100644 index 00000000000..ecaf2c06282 --- /dev/null +++ b/integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_filter_push_timestamp_wrapped_field.yaml @@ -0,0 +1,11 @@ +calcite: + logical: | + LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) + LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12]) + LogicalFilter(condition=[<(TIMESTAMP($3), TIMESTAMP('2018-11-09 00:00:00':VARCHAR))]) + LogicalFilter(condition=[>(TIMESTAMP($3), TIMESTAMP('2016-12-08 00:00:00':VARCHAR))]) + CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]]) + physical: | + EnumerableLimit(fetch=[10000]) + EnumerableCalc(expr#0..18=[{inputs}], expr#19=[TIMESTAMP($t3)], expr#20=[Sarg[('2016-12-08 00:00:00':VARCHAR..'2018-11-09 00:00:00':VARCHAR)]:VARCHAR], expr#21=[SEARCH($t19, $t20)], proj#0..12=[{exprs}], $condition=[$t21]) + CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]]) diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_last_day_no_push.yaml b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_last_day_no_push.yaml new file mode 100644 index 00000000000..a04d52a3199 --- /dev/null +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_last_day_no_push.yaml @@ -0,0 +1,20 @@ +root: + name: ProjectOperator + description: + fields: "[yyyy-MM-dd]" + children: + - name: FilterOperator + description: + conditions: "=(last_day(yyyy-MM-dd), date(\"2018-11-30\"))" + children: + - name: ProjectOperator + description: + fields: "[yyyy-MM-dd]" + children: + - name: OpenSearchIndexScan + description: + request: "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_date_formats,\ + \ sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"\ + _source\":{\"includes\":[\"yyyy-MM-dd\"]}}, pitId=*,\ + \ cursorKeepAlive=1m, searchAfter=null, searchResponse=null)" + children: [] diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push_timestamp_wrapped_field.yaml b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push_timestamp_wrapped_field.yaml new file mode 100644 index 00000000000..5f1ae1a53a5 --- /dev/null +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push_timestamp_wrapped_field.yaml @@ -0,0 +1,19 @@ +root: + name: ProjectOperator + description: + fields: "[account_number, firstname, address, birthdate, gender, city, lastname,\ + \ balance, employer, state, age, email, male]" + children: + - name: OpenSearchIndexScan + description: + request: "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_bank,\ + \ sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\"\ + :{\"bool\":{\"filter\":[{\"range\":{\"birthdate\":{\"from\":null,\"to\"\ + :1541721600000,\"include_lower\":true,\"include_upper\":false,\"boost\"\ + :1.0}}},{\"range\":{\"birthdate\":{\"from\":1481155200000,\"to\":null,\"\ + include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\"\ + :true,\"boost\":1.0}},\"_source\":{\"includes\":[\"account_number\",\"firstname\"\ + ,\"address\",\"birthdate\",\"gender\",\"city\",\"lastname\",\"balance\"\ + ,\"employer\",\"state\",\"age\",\"email\",\"male\"]}}, pitId=*,\ + \ cursorKeepAlive=1m, searchAfter=null, searchResponse=null)" + children: [] diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java index 476e0018fcd..5bb6693d90a 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java @@ -249,6 +249,13 @@ public static QueryExpression analyzeExpression( } } + /** + * Date/time conversion operators, i.e. the ones that only reinterpret the value. A conversion to + * the type the field already has is a no-op and can be folded away; see {@link + * Visitor#isRedundantDateCastOverField}. + */ + private static final Set DATE_CONVERSION_OPERATORS = Set.of("TIMESTAMP", "DATE", "TIME"); + /** Traverses {@link RexNode} tree and builds OpenSearch query. */ static class Visitor extends RexVisitorImpl { @@ -365,6 +372,14 @@ static RexUnknownAs getNullAsForSearch(RexCall search) { @Override public Expression visitCall(RexCall call) { + // Fold a redundant date/time cast over a field of the same date/time type -- e.g. + // timestamp() or CAST( AS TIMESTAMP) -- to the bare field + // reference. Such a wrap is a no-op, so the enclosing comparison can push down to a native + // range query instead of falling back to a per-document script. + if (isRedundantDateCastOverField(call)) { + return visitInputRef((RexInputRef) call.getOperands().get(0)); + } + SqlSyntax syntax = call.getOperator().getSyntax(); if (!supportedRexCall(call)) { String message = format(Locale.ROOT, "Unsupported call: [%s]", call); @@ -1000,6 +1015,47 @@ private CastExpression toCastExpression(RexCall call) { return new CastExpression(call.getType(), argument); } + /** + * True if {@code call} is a date/time cast — a {@code CAST(... AS TIMESTAMP/DATE/TIME)} or the + * {@code timestamp()}/{@code date()}/{@code time()} builtins, both of which yield a date/time + * UDT — applied to a single field reference whose own type is the same date/time type. + * Such a wrap is a no-op, so it is redundant for a comparison and can be unwrapped to the bare + * field, letting the predicate push down instead of falling back to a per-document script. + * + *

The target type must match the field type exactly. A cast that changes the date/time type + * is a real conversion and must not be folded: {@code date()} truncates the + * time component (so {@code date(ts) <= '2024-01-15'} is not {@code ts <= '2024-01-15'}), and + * {@code time()} extracts the time of day, which is not even monotonic with + * respect to the timestamp. + */ + private boolean isRedundantDateCastOverField(RexCall call) { + if (call.getOperands().size() != 1) { + return false; + } + if (!(call.getOperands().get(0) instanceof RexInputRef inputRef)) { + return false; + } + // Only a cast or a date/time conversion operator can be a no-op. The result type alone is not + // sufficient: other single-argument functions also return a date/time type while changing the + // value (LAST_DAY being the clearest example), and folding those away would be wrong. + if (call.getKind() != SqlKind.CAST + && !DATE_CONVERSION_OPERATORS.contains( + call.getOperator().getName().toUpperCase(Locale.ROOT))) { + return false; + } + // Date/time values are modelled as UDTs (EXPR_DATE/EXPR_TIME/EXPR_TIMESTAMP) whose backing + // SqlTypeName is VARCHAR, so the UDT identifies the cast target -- not getSqlTypeName(). + if (!(call.getType() instanceof ExprSqlType exprSqlType)) { + return false; + } + ExprUDT udt = exprSqlType.getUdt(); + if (udt != ExprUDT.EXPR_TIMESTAMP && udt != ExprUDT.EXPR_DATE && udt != ExprUDT.EXPR_TIME) { + return false; + } + ExprType fieldType = new NamedFieldExpression(inputRef, schema, fieldTypes).getCoreExprType(); + return udt.getExprCoreType().equals(fieldType); + } + private static NamedFieldExpression toNamedField(RexLiteral literal) { return new NamedFieldExpression(literal); } @@ -1871,6 +1927,17 @@ boolean isTimeStampType() { : type.getOriginalExprType()); } + /** The field's underlying core type, unwrapping {@link OpenSearchDataType} if present. */ + @Nullable + ExprType getCoreExprType() { + if (type == null) { + return null; + } + return type.getOriginalExprType() instanceof OpenSearchDataType osType + ? osType.getExprCoreType() + : type.getOriginalExprType(); + } + boolean isTextType() { return type != null && type.getOriginalExprType() instanceof OpenSearchTextType; } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/LuceneQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/LuceneQuery.java index 426af9a4b11..200ecbb19bd 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/LuceneQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/LuceneQuery.java @@ -52,10 +52,11 @@ public abstract class LuceneQuery { * @return return true if supported, otherwise false. */ public boolean canSupport(FunctionExpression func) { - return (func.getArguments().size() == 2) - && (func.getArguments().get(0) instanceof ReferenceExpression) + return ((func.getArguments().size() == 2) + && (func.getArguments().get(0) instanceof ReferenceExpression + || referenceWrappedByRedundantDateCast(func.getArguments().get(0))) && (func.getArguments().get(1) instanceof LiteralExpression - || literalExpressionWrappedByCast(func)) + || literalExpressionWrappedByCast(func))) || isMultiParameterQuery(func); } @@ -97,6 +98,65 @@ protected boolean literalExpressionWrappedByCast(FunctionExpression func) { return false; } + /** Date/time cast functions mapped to the type each one produces. */ + private static final Map DATE_CAST_TARGET_TYPES = + ImmutableMap.builder() + .put(BuiltinFunctionName.CAST_TO_TIMESTAMP.getName(), ExprCoreType.TIMESTAMP) + .put(BuiltinFunctionName.TIMESTAMP.getName(), ExprCoreType.TIMESTAMP) + .put(BuiltinFunctionName.CAST_TO_DATE.getName(), ExprCoreType.DATE) + .put(BuiltinFunctionName.DATE.getName(), ExprCoreType.DATE) + .put(BuiltinFunctionName.CAST_TO_TIME.getName(), ExprCoreType.TIME) + .put(BuiltinFunctionName.TIME.getName(), ExprCoreType.TIME) + .build(); + + /** + * Check if the left operand is a date/time cast (or the {@code timestamp()}/{@code date()}/{@code + * time()} builtin) applied to a reference of the same date/time type. Such a wrap is a + * no-op, so it can be unwrapped, letting the predicate push down to a native range/term query + * instead of falling back to a per-document script. + * + *

The cast target must match the field type exactly. A cast that changes the date/time type is + * a real conversion and must not be folded: {@code date()} truncates the time + * component (so {@code date(ts) <= '2024-01-15'} is not {@code ts <= '2024-01-15'}), and {@code + * time()} extracts the time of day, which is not even monotonic with respect to + * the timestamp. + * + * @param arg left operand of the comparison. + * @return true if the operand is a redundant, order-preserving date/time cast over a reference. + */ + protected boolean referenceWrappedByRedundantDateCast(Expression arg) { + if (!(arg instanceof FunctionExpression)) { + return false; + } + FunctionExpression fn = (FunctionExpression) arg; + ExprCoreType castTarget = DATE_CAST_TARGET_TYPES.get(fn.getFunctionName()); + if (castTarget == null || fn.getArguments().size() != 1) { + return false; + } + Expression inner = fn.getArguments().get(0); + return inner instanceof ReferenceExpression + && inner.type() instanceof OpenSearchDateType dateType + && castTarget.equals(dateType.getExprCoreType()); + } + + /** + * Return the underlying reference of the left operand, unwrapping a redundant date/time cast if + * present (see {@link #referenceWrappedByRedundantDateCast}). Callers must ensure {@link + * #canSupport} returned true for the enclosing function; otherwise an {@link + * IllegalStateException} is thrown rather than allowing an unchecked cast to fail. + */ + private ReferenceExpression unwrapReference(Expression arg) { + if (arg instanceof ReferenceExpression) { + return (ReferenceExpression) arg; + } + if (referenceWrappedByRedundantDateCast(arg)) { + return (ReferenceExpression) ((FunctionExpression) arg).getArguments().get(0); + } + throw new IllegalStateException( + "Left operand must be a reference or a redundant date/time cast over a reference; " + + "canSupport() must be checked before build()"); + } + /** * Build Lucene query from function expression. The cast function is converted to literal * expressions before generating DSL. @@ -105,7 +165,7 @@ protected boolean literalExpressionWrappedByCast(FunctionExpression func) { * @return query */ public QueryBuilder build(FunctionExpression func) { - ReferenceExpression ref = (ReferenceExpression) func.getArguments().get(0); + ReferenceExpression ref = unwrapReference(func.getArguments().get(0)); Expression expr = func.getArguments().get(1); ExprValue literalValue = expr instanceof LiteralExpression ? expr.valueOf() : cast((FunctionExpression) expr, ref); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/PredicateAnalyzerTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/PredicateAnalyzerTest.java index 01c5e6108ef..baeb7f0e989 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/PredicateAnalyzerTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/PredicateAnalyzerTest.java @@ -1304,6 +1304,85 @@ void gte_generatesRangeQueryWithFormatForDateTime() throws ExpressionNotAnalyzab result.toString()); } + @Test + void gte_redundantTimestampCastOverTimestampField_generatesRangeQuery() + throws ExpressionNotAnalyzableException { + // timestamp() is a no-op wrap, so the comparison must still push down to a + // native range query instead of falling back to a per-document script. + RexNode wrapped = PPLFuncImpTable.INSTANCE.resolve(builder, "timestamp", field4); + RexNode call = + builder.makeCall(SqlStdOperatorTable.GREATER_THAN_OR_EQUAL, wrapped, dateTimeLiteral); + QueryBuilder result = PredicateAnalyzer.analyze(call, schema, fieldTypes); + + assertInstanceOf(RangeQueryBuilder.class, result); + assertEquals( + """ + { + "range" : { + "d" : { + "from" : "1987-02-03T04:34:56.000Z", + "to" : null, + "include_lower" : true, + "include_upper" : true, + "format" : "date_time", + "boost" : 1.0 + } + } + }\ + """, + result.toString()); + } + + @Test + void equals_lastDayOverTimestampField_isNotFoldedAndFallsBackToScript() + throws ExpressionNotAnalyzableException { + // LAST_DAY takes a single date/time argument and returns a date/time type, but it changes the + // value, so it must not be treated as a redundant conversion. Only CAST and the + // timestamp()/date()/time() conversion operators are foldable. + final RelDataType rowType = + builder + .getTypeFactory() + .builder() + .kind(StructKind.FULLY_QUALIFIED) + .add("a", builder.getTypeFactory().createSqlType(SqlTypeName.BIGINT)) + .add("b", builder.getTypeFactory().createSqlType(SqlTypeName.VARCHAR)) + .add("c", builder.getTypeFactory().createSqlType(SqlTypeName.VARCHAR)) + .add("d", typeFactory.createUDT(ExprUDT.EXPR_TIMESTAMP)) + .build(); + RexNode wrapped = PPLFuncImpTable.INSTANCE.resolve(builder, "last_day", field4); + RexNode call = builder.makeCall(SqlStdOperatorTable.EQUALS, wrapped, dateTimeLiteral); + QueryBuilder result = + PredicateAnalyzer.analyzeExpression(call, schema, fieldTypes, rowType, cluster).builder(); + + assertInstanceOf(ScriptQueryBuilder.class, result); + } + + @Test + void lte_dateCastOverTimestampField_isNotFoldedAndFallsBackToScript() + throws ExpressionNotAnalyzableException { + // date() truncates the time component, so it is NOT a redundant wrap and must + // not be folded to the bare field: `date(ts) <= '1987-02-03'` is not `ts <= '1987-02-03'`. + // It therefore stays on the script path rather than becoming a range query. + final RelDataType rowType = + builder + .getTypeFactory() + .builder() + .kind(StructKind.FULLY_QUALIFIED) + .add("a", builder.getTypeFactory().createSqlType(SqlTypeName.BIGINT)) + .add("b", builder.getTypeFactory().createSqlType(SqlTypeName.VARCHAR)) + .add("c", builder.getTypeFactory().createSqlType(SqlTypeName.VARCHAR)) + .add("d", typeFactory.createUDT(ExprUDT.EXPR_TIMESTAMP)) + .build(); + RexNode wrapped = PPLFuncImpTable.INSTANCE.resolve(builder, "date", field4); + RexNode dateLiteral = + builder.makeLiteral("1987-02-03", typeFactory.createUDT(ExprUDT.EXPR_DATE), true); + RexNode call = builder.makeCall(SqlStdOperatorTable.LESS_THAN_OR_EQUAL, wrapped, dateLiteral); + QueryBuilder result = + PredicateAnalyzer.analyzeExpression(call, schema, fieldTypes, rowType, cluster).builder(); + + assertInstanceOf(ScriptQueryBuilder.class, result); + } + @Test void isTrue_booleanField_generatesTermQuery() throws ExpressionNotAnalyzableException { // IS_TRUE(boolean_field) should generate a term query with value true diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java index e930056474a..b44381ac74e 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java @@ -6,6 +6,7 @@ package org.opensearch.sql.opensearch.storage.script.filter; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -53,6 +54,7 @@ import org.opensearch.sql.expression.LiteralExpression; import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; +import org.opensearch.sql.opensearch.data.type.OpenSearchDateType; import org.opensearch.sql.opensearch.data.type.OpenSearchTextType; import org.opensearch.sql.opensearch.storage.serde.ExpressionSerializer; @@ -158,6 +160,60 @@ void should_build_range_query_for_comparison_expression() { buildQuery(expr))); } + @Test + void should_push_down_range_query_when_date_field_wrapped_by_redundant_date_cast() { + // Wrapping an already date-typed field in timestamp()/CAST(... AS TIMESTAMP) is redundant and + // range-preserving, so `timestamp() >= ` must push down to a native range + // query rather than fall back to a per-document script. + OpenSearchDateType dateType = OpenSearchDateType.of(TIMESTAMP); + Expression[] predicates = { + DSL.gte( + DSL.timestamp(ref("datetime", dateType)), + DSL.castTimestamp(literal("2021-11-08 17:00:00"))), + DSL.gte( + DSL.castTimestamp(ref("datetime", dateType)), + DSL.castTimestamp(literal("2021-11-08 17:00:00"))) + }; + for (Expression predicate : predicates) { + String query = buildQuery(predicate); + assertTrue(query.contains("\"range\""), query); + assertTrue(query.contains("datetime"), query); + assertFalse(query.contains("script"), query); + } + } + + @Test + void should_not_push_down_when_date_cast_changes_the_date_type() { + // date()/time() over a timestamp field are real conversions, not no-ops: date() truncates the + // time component and time() extracts the time of day (not monotonic in the timestamp), so they + // must keep using the script path rather than being folded to the bare field. + mockToStringSerializer(); + OpenSearchDateType dateType = OpenSearchDateType.of(TIMESTAMP); + Expression[] predicates = { + DSL.lte(DSL.date(ref("datetime", dateType)), DSL.castDate(literal("2021-11-08"))), + DSL.gte(DSL.time(ref("datetime", dateType)), DSL.castTime(literal("17:00:00"))) + }; + for (Expression predicate : predicates) { + String query = buildQuery(predicate); + assertTrue(query.contains("script"), query); + assertFalse(query.contains("\"range\""), query); + } + } + + @Test + void should_not_push_down_when_date_cast_wraps_non_date_field() { + // Casting a non-date field to a timestamp is a real, not necessarily order-preserving, + // conversion, so it must stay on the script path (the fold only applies to date-typed fields). + mockToStringSerializer(); + String query = + buildQuery( + DSL.gte( + DSL.castTimestamp(ref("string_value", STRING)), + DSL.castTimestamp(literal("2021-11-08 17:00:00")))); + assertTrue(query.contains("script"), query); + assertFalse(query.contains("\"range\""), query); + } + @Test void should_build_wildcard_query_for_like_expression() { assertJsonEquals(