Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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());
}
}
}
30 changes: 30 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
@@ -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)])
Original file line number Diff line number Diff line change
@@ -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)])
Original file line number Diff line number Diff line change
@@ -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]])
Original file line number Diff line number Diff line change
@@ -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]])
Original file line number Diff line number Diff line change
@@ -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: []
Original file line number Diff line number Diff line change
@@ -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: []
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> DATE_CONVERSION_OPERATORS = Set.of("TIMESTAMP", "DATE", "TIME");

/** Traverses {@link RexNode} tree and builds OpenSearch query. */
static class Visitor extends RexVisitorImpl<Expression> {

Expand Down Expand Up @@ -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(<timestamp field>) or CAST(<timestamp field> 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);
Expand Down Expand Up @@ -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 <b>the same</b> 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.
*
* <p>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(<timestamp field>)} truncates the
* time component (so {@code date(ts) <= '2024-01-15'} is not {@code ts <= '2024-01-15'}), and
* {@code time(<timestamp field>)} 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);
}
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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<FunctionName, ExprCoreType> DATE_CAST_TARGET_TYPES =
ImmutableMap.<FunctionName, ExprCoreType>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 <b>the same</b> 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.
*
* <p>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(<timestamp field>)} truncates the time
* component (so {@code date(ts) <= '2024-01-15'} is not {@code ts <= '2024-01-15'}), and {@code
* time(<timestamp field>)} 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.
Expand All @@ -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);
Expand Down
Loading