diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/SearchAnd.java b/core/src/main/java/org/opensearch/sql/ast/expression/SearchAnd.java index bdfbd9fda39..6af199ddaf8 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/SearchAnd.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/SearchAnd.java @@ -7,10 +7,12 @@ import java.util.Arrays; import java.util.List; +import java.util.function.Function; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.ToString; +import org.opensearch.sql.data.type.ExprType; /** Search expression for AND operator. */ @Getter @@ -23,8 +25,8 @@ public class SearchAnd extends SearchExpression { private final SearchExpression right; @Override - public String toQueryString() { - return left.toQueryString() + " AND " + right.toQueryString(); + public String toQueryString(Function fieldTypeResolver) { + return left.toQueryString(fieldTypeResolver) + " AND " + right.toQueryString(fieldTypeResolver); } @Override diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/SearchComparison.java b/core/src/main/java/org/opensearch/sql/ast/expression/SearchComparison.java index c429e6f66cc..a0006019b8f 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/SearchComparison.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/SearchComparison.java @@ -9,10 +9,12 @@ import java.util.Arrays; import java.util.List; +import java.util.function.Function; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.ToString; +import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.utils.QueryStringUtils; /** Search expression for field comparisons. */ @@ -46,9 +48,11 @@ public String getSymbol() { private final SearchLiteral value; @Override - public String toQueryString() { - String fieldName = QueryStringUtils.escapeFieldName(field.getField().toString()); - String valueStr = value.toQueryString(); + public String toQueryString(Function fieldTypeResolver) { + String rawFieldName = field.getField().toString(); + String fieldName = QueryStringUtils.escapeFieldName(rawFieldName); + ExprType resolvedType = fieldTypeResolver.apply(rawFieldName); + String valueStr = value.toQueryString(resolvedType); switch (operator) { case NOT_EQUALS: return "( _exists_:" + fieldName + " AND NOT " + fieldName + ":" + valueStr + " )"; diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/SearchExpression.java b/core/src/main/java/org/opensearch/sql/ast/expression/SearchExpression.java index b705909445f..a68163959c7 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/SearchExpression.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/SearchExpression.java @@ -5,17 +5,33 @@ package org.opensearch.sql.ast.expression; +import java.util.function.Function; import org.opensearch.sql.ast.AbstractNodeVisitor; +import org.opensearch.sql.data.type.ExprType; /** Base class for search expressions that get converted to query_string syntax. */ public abstract class SearchExpression extends UnresolvedExpression { /** - * Convert this search expression to query_string syntax. + * Convert this search expression to query_string syntax without field-type awareness. * * @return the query string representation */ - public abstract String toQueryString(); + public String toQueryString() { + return toQueryString(f -> null); + } + + /** + * Convert this search expression to query_string syntax, using {@code fieldTypeResolver} to + * resolve the OpenSearch type of a field when the emission depends on whether the field is + * keyword vs. text. When the resolver returns {@code null}, emission falls back to the + * field-type-agnostic form (same as {@link #toQueryString()}). + * + * @param fieldTypeResolver maps a field name to its resolved {@link ExprType}, or null when + * unknown + * @return the query string representation + */ + public abstract String toQueryString(Function fieldTypeResolver); /** * Convert the search expression to anonymized string diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/SearchGroup.java b/core/src/main/java/org/opensearch/sql/ast/expression/SearchGroup.java index 09197202dc0..ebd8ad9b0ef 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/SearchGroup.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/SearchGroup.java @@ -7,10 +7,12 @@ import java.util.Collections; import java.util.List; +import java.util.function.Function; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.ToString; +import org.opensearch.sql.data.type.ExprType; /** Search expression for grouped expressions (parentheses). */ @Getter @@ -22,8 +24,8 @@ public class SearchGroup extends SearchExpression { private final SearchExpression expression; @Override - public String toQueryString() { - return "(" + expression.toQueryString() + ")"; + public String toQueryString(Function fieldTypeResolver) { + return "(" + expression.toQueryString(fieldTypeResolver) + ")"; } @Override diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/SearchIn.java b/core/src/main/java/org/opensearch/sql/ast/expression/SearchIn.java index 8291d130dff..3ce113a5e39 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/SearchIn.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/SearchIn.java @@ -7,11 +7,13 @@ import java.util.ArrayList; import java.util.List; +import java.util.function.Function; import java.util.stream.Collectors; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.ToString; +import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.utils.QueryStringUtils; /** Search expression for IN operator. */ @@ -25,10 +27,12 @@ public class SearchIn extends SearchExpression { private final List values; @Override - public String toQueryString() { + public String toQueryString(Function fieldTypeResolver) { + String rawFieldName = field.getField().toString(); String fieldName = QueryStringUtils.escapeFieldName(field.getField().toString()); + ExprType resolvedType = fieldTypeResolver.apply(rawFieldName); String valueList = - values.stream().map(SearchLiteral::toQueryString).collect(Collectors.joining(" OR ")); + values.stream().map(v -> v.toQueryString(resolvedType)).collect(Collectors.joining(" OR ")); return fieldName + ":( " + valueList + " )"; } diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/SearchLiteral.java b/core/src/main/java/org/opensearch/sql/ast/expression/SearchLiteral.java index 460615afa64..1932e943bb0 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/SearchLiteral.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/SearchLiteral.java @@ -7,10 +7,12 @@ import java.util.Collections; import java.util.List; +import java.util.function.Function; import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.ToString; +import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.utils.QueryStringUtils; /** Search expression for standalone literals. */ @@ -21,10 +23,40 @@ public class SearchLiteral extends SearchExpression { private final UnresolvedExpression literal; - private final boolean isPhrase; + + /** + * Whether the user wrote this value inside quotes in the PPL query. On a text field this is the + * user's explicit request for phrase semantics — see {@link #toQueryString(ExprType)}. + */ + private final boolean userQuoted; @Override - public String toQueryString() { + public String toQueryString(Function fieldTypeResolver) { + // Unfielded literal: no enclosing field, so no index type. Take the field-agnostic branch. + return toQueryString((ExprType) null); + } + + /** + * Emits the query_string form for a literal on the RHS of {@link SearchComparison} or inside + * {@link SearchIn}. Emission is driven by the enclosing field's index mapping, because the + * mapping decides whether the value gets analyzed: + * + *
    + *
  • keyword family with a wildcard — the analyzer is a no-op, so the value has to + * reach Lucene as one term for the pattern to apply to the whole stored value. Emitted + * unquoted with whitespace escaped. + *
  • text — honor the user's quoting. Unquoted passes through, so {@code *} and {@code + * ?} stay query_string operators. Quoted becomes a phrase, which is how a user asks for + * "this whole value, in order" against the analyzed tokens. + *
  • everything else — keyword without a wildcard, plus date, numeric, ip, boolean and + * unresolved fields. Legacy behavior. Note that quoting genuinely carries no information on + * keyword: with a no-op analyzer a quoted phrase and a bare term both resolve to the same + * single term, so there is nothing to gain by rewriting the emission here. + *
+ * + * @param indexType the enclosing field's OpenSearch index-mapping type, or null if unresolved + */ + public String toQueryString(ExprType indexType) { if (literal instanceof Literal) { Literal lit = (Literal) literal; Object val = lit.getValue(); @@ -38,21 +70,69 @@ public String toQueryString() { if (val instanceof String) { String str = (String) val; - // Phrase search - preserve quotes - if (isPhrase) { - // Escape special chars inside the phrase - str = QueryStringUtils.escapeLuceneSpecialCharacters(str); - return "\"" + str + "\""; + // A keyword-family field carries whole-value semantics, so a value holding a wildcard has + // to reach Lucene as a single term. Escaping the whitespace keeps query_string from + // splitting at the space and dropping the field binding on the tail. + if (isKeywordLike(indexType) && hasUnescapedWildcard(str)) { + return unquoted(str).replace(" ", "\\ "); + } + + if (isTextLike(indexType)) { + // Quoting requests phrase semantics. One exception: a whitespace-free value carrying an + // unescaped wildcard is emitted unquoted, because quoting would let the analyzer discard + // the wildcard (`foo*` would stop matching `foobar`). That is only safe without + // whitespace — with a space, an unquoted value would be split into separate clauses and + // the tail would lose its field binding, so those stay phrases. + boolean wildcardTerm = hasUnescapedWildcard(str) && !str.contains(" "); + return userQuoted && !wildcardTerm ? quoted(str) : unquoted(str); } - // Regular string - escape special characters - return QueryStringUtils.escapeLuceneSpecialCharacters(str); + // Everything else — keyword without a wildcard, plus date, numeric, ip, boolean and + // unresolved fields: legacy behavior, byte-identical to before this change. + return str.contains(" ") ? quoted(str) : unquoted(str); } } - // Default: escape the text representation - String text = literal.toString(); - return QueryStringUtils.escapeLuceneSpecialCharacters(text); + return unquoted(literal.toString()); + } + + private static String quoted(String str) { + return "\"" + QueryStringUtils.escapeLuceneSpecialCharacters(str) + "\""; + } + + private static String unquoted(String str) { + return QueryStringUtils.escapeLuceneSpecialCharacters(str); + } + + private static boolean isTextLike(ExprType type) { + if (type == null) { + return false; + } + String legacyName = type.getOriginalExprType().legacyTypeName(); + return "TEXT".equalsIgnoreCase(legacyName) || "MATCH_ONLY_TEXT".equalsIgnoreCase(legacyName); + } + + private static boolean isKeywordLike(ExprType type) { + if (type == null) { + return false; + } + String legacyName = type.getOriginalExprType().legacyTypeName(); + return "KEYWORD".equalsIgnoreCase(legacyName) + || "CONSTANT_KEYWORD".equalsIgnoreCase(legacyName); + } + + private static boolean hasUnescapedWildcard(String s) { + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + if (c == '\\' && i + 1 < s.length()) { + i++; + continue; + } + if (c == '*' || c == '?') { + return true; + } + } + return false; } @Override diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/SearchNot.java b/core/src/main/java/org/opensearch/sql/ast/expression/SearchNot.java index b9ea7b416b4..ed20a59cf48 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/SearchNot.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/SearchNot.java @@ -7,10 +7,12 @@ import java.util.Collections; import java.util.List; +import java.util.function.Function; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.ToString; +import org.opensearch.sql.data.type.ExprType; /** Search expression for NOT operator. */ @Getter @@ -22,8 +24,8 @@ public class SearchNot extends SearchExpression { private final SearchExpression expression; @Override - public String toQueryString() { - return "NOT(" + expression.toQueryString() + ")"; + public String toQueryString(Function fieldTypeResolver) { + return "NOT(" + expression.toQueryString(fieldTypeResolver) + ")"; } @Override diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/SearchOr.java b/core/src/main/java/org/opensearch/sql/ast/expression/SearchOr.java index 1a9e95e89a2..258cd09d3eb 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/SearchOr.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/SearchOr.java @@ -7,10 +7,12 @@ import java.util.Arrays; import java.util.List; +import java.util.function.Function; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.ToString; +import org.opensearch.sql.data.type.ExprType; /** Search expression for OR operator. */ @Getter @@ -23,8 +25,8 @@ public class SearchOr extends SearchExpression { private final SearchExpression right; @Override - public String toQueryString() { - return left.toQueryString() + " OR " + right.toQueryString(); + public String toQueryString(Function fieldTypeResolver) { + return left.toQueryString(fieldTypeResolver) + " OR " + right.toQueryString(fieldTypeResolver); } @Override diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java index 9995895bfa1..2afb0eb35fb 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -173,6 +173,7 @@ import org.opensearch.sql.ast.tree.Values; import org.opensearch.sql.ast.tree.Window; import org.opensearch.sql.ast.tree.Xyseries; +import org.opensearch.sql.calcite.plan.AbstractOpenSearchTable; import org.opensearch.sql.calcite.plan.AliasFieldsWrappable; import org.opensearch.sql.calcite.plan.HighlightPushDown; import org.opensearch.sql.calcite.plan.OpenSearchConstants; @@ -192,6 +193,7 @@ import org.opensearch.sql.common.patterns.PatternUtils; import org.opensearch.sql.common.utils.StringUtils; import org.opensearch.sql.data.type.ExprCoreType; +import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.datasource.DataSourceService; import org.opensearch.sql.exception.CalciteUnsupportedException; import org.opensearch.sql.exception.SemanticCheckException; @@ -297,11 +299,33 @@ private RelBuilder scan(RelOptTable tableSchema, CalcitePlanContext context) { public RelNode visitSearch(Search node, CalcitePlanContext context) { // Visit the Relation child to get the scan node.getChild().get(0).accept(this, context); + // Resolve query_string from the structured expression when available so we can consult the + // OpenSearch table's field-type map for per-field text/keyword awareness (e.g. escape + // space + wildcard on keyword vs. quoted phrase on text). Falls back to the pre-computed + // string for callers that never populated the structured expression. + String queryString; + if (node.getOriginalExpression() != null) { + // TODO: index-mapping type (text/keyword) is storage metadata, not a data type — the right + // home is a field/scan annotation on RelDataType, but that needs a Calcite rule-pipeline + // audit (rules rebuild row types and can drop custom fields). For now, unwrap the table + // and read the ExprType map directly. + java.util.Map typesByName = new java.util.HashMap<>(); + RelNode scan = context.relBuilder.peek(); + RelOptTable relOptTable = scan.getTable(); + if (relOptTable != null) { + AbstractOpenSearchTable osTable = relOptTable.unwrap(AbstractOpenSearchTable.class); + if (osTable != null) { + typesByName.putAll(osTable.getFieldTypes()); + } + } + queryString = node.getOriginalExpression().toQueryString(typesByName::get); + } else { + queryString = node.getQueryString(); + } // Create query_string function Function queryStringFunc = AstDSL.function( - "query_string", - AstDSL.unresolvedArg("query", AstDSL.stringLiteral(node.getQueryString()))); + "query_string", AstDSL.unresolvedArg("query", AstDSL.stringLiteral(queryString))); RexNode queryStringRex = rexVisitor.analyze(queryStringFunc, context); context.relBuilder.filter(queryStringRex); diff --git a/core/src/test/java/org/opensearch/sql/ast/expression/SearchLiteralTest.java b/core/src/test/java/org/opensearch/sql/ast/expression/SearchLiteralTest.java new file mode 100644 index 00000000000..c722916f339 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/ast/expression/SearchLiteralTest.java @@ -0,0 +1,232 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.ast.expression; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.data.type.ExprType; + +/** + * Index-mapping-aware emission for {@link SearchLiteral} and {@link SearchComparison}. + * + *

On text, the user's quoting is honored: unquoted keeps {@code *}/{@code ?} as + * query_string operators, quoted becomes a phrase. On the keyword family, quoting is + * irrelevant (the analyzer is a no-op) so we emit whole-value semantics — a wildcard pattern when + * the value holds an unescaped wildcard, otherwise an exact term. Other mappings keep legacy + * behavior. + */ +class SearchLiteralTest { + + /** Stub {@link ExprType} reporting a given legacyTypeName — enough for the mapping checks. */ + private static ExprType typeOf(String legacyName) { + return new ExprType() { + @Override + public String typeName() { + return legacyName; + } + + @Override + public String legacyTypeName() { + return legacyName; + } + }; + } + + private static final ExprType KEYWORD = typeOf("KEYWORD"); + private static final ExprType CONSTANT_KEYWORD = typeOf("CONSTANT_KEYWORD"); + private static final ExprType TEXT = typeOf("TEXT"); + private static final ExprType MATCH_ONLY_TEXT = typeOf("MATCH_ONLY_TEXT"); + private static final ExprType DATE = typeOf("TIMESTAMP"); + private static final ExprType LONG = typeOf("LONG"); + + /** Value the user wrote inside quotes. */ + private static SearchLiteral q(String value) { + return new SearchLiteral(new Literal(value, DataType.STRING), true); + } + + /** Value the user wrote bare. */ + private static SearchLiteral bare(String value) { + return new SearchLiteral(new Literal(value, DataType.STRING), false); + } + + // ------------------------------------------------------------------------- + // TEXT: honor the user's quoting. + // ------------------------------------------------------------------------- + + @Test + void text_quoted_emits_phrase() { + assertEquals("\"foo bar\"", q("foo bar").toQueryString(TEXT)); + } + + @Test + void text_unquoted_emits_bare_term() { + assertEquals("foo", bare("foo").toQueryString(TEXT)); + } + + @Test + void text_quoted_wildcard_term_stays_unquoted() { + // Whitespace-free value with a wildcard: emitted unquoted so '*' keeps operator meaning. + // Quoting it would let the analyzer discard the wildcard. + assertEquals("foo*", q("foo*").toQueryString(TEXT)); + assertEquals("f?o", q("f?o").toQueryString(TEXT)); + } + + @Test + void text_quoted_wildcard_with_whitespace_emits_phrase() { + // With whitespace, unquoted would split into separate clauses and the tail would lose its + // field binding, so the phrase form is kept. + assertEquals("\"foo bar*\"", q("foo bar*").toQueryString(TEXT)); + assertEquals("\"*foo bar\"", q("*foo bar").toQueryString(TEXT)); + } + + @Test + void text_escaped_wildcard_is_not_a_wildcard_term() { + // '*' is escaped, so it is a literal — the value has no unescaped wildcard and stays a phrase. + assertEquals("\"foo\\*\"", q("foo\\*").toQueryString(TEXT)); + } + + @Test + void text_unquoted_wildcard_keeps_operator() { + assertEquals("foo*", bare("foo*").toQueryString(TEXT)); + } + + @Test + void text_match_only_text_behaves_like_text() { + assertEquals("\"foo bar\"", q("foo bar").toQueryString(MATCH_ONLY_TEXT)); + assertEquals("foo*", bare("foo*").toQueryString(MATCH_ONLY_TEXT)); + } + + /** + * A quoted value the analyzer splits must not become an OR of tokens. These values hold no + * whitespace, so before the mapping split they were emitted unquoted. + */ + @Test + void text_quoted_value_the_analyzer_splits_emits_phrase() { + assertEquals("\"foo\\-bar\"", q("foo-bar").toQueryString(TEXT)); + assertEquals("\"foo=bar\"", q("foo=bar").toQueryString(TEXT)); + } + + // ------------------------------------------------------------------------- + // KEYWORD FAMILY: quoting is irrelevant; wildcard presence selects the form. + // ------------------------------------------------------------------------- + + @Test + void keyword_no_wildcard_keeps_legacy_emission() { + // Quoting carries no information on keyword — a quoted phrase and a bare term both resolve to + // the same single term — so the emission is left exactly as it was. + assertEquals("foo", bare("foo").toQueryString(KEYWORD)); + assertEquals("foo", q("foo").toQueryString(KEYWORD)); + assertEquals("foo\\-bar", q("foo-bar").toQueryString(KEYWORD)); + assertEquals("\"foo bar\"", q("foo bar").toQueryString(KEYWORD)); + } + + @Test + void keyword_no_wildcard_quoting_is_irrelevant() { + assertEquals(q("foo-bar").toQueryString(KEYWORD), bare("foo-bar").toQueryString(KEYWORD)); + } + + @Test + void keyword_wildcard_with_space_escapes_the_space() { + // The reported bug (#5682): whole-value pattern, not a phrase with a literal '*'. + assertEquals("foo\\ bar*", q("foo bar*").toQueryString(KEYWORD)); + } + + @Test + void keyword_wildcard_without_space_emits_bare_pattern() { + assertEquals("foo*", q("foo*").toQueryString(KEYWORD)); + assertEquals("foo*", bare("foo*").toQueryString(KEYWORD)); + } + + @Test + void keyword_leading_and_interior_wildcards_escape_spaces() { + assertEquals("*foo\\ bar", q("*foo bar").toQueryString(KEYWORD)); + assertEquals("foo\\ *baz", q("foo *baz").toQueryString(KEYWORD)); + assertEquals("foo\\ b?r", q("foo b?r").toQueryString(KEYWORD)); + } + + @Test + void keyword_wildcard_with_special_chars_escapes_all() { + assertEquals( + "POST\\ \\/test\\-logs\\/_search*", q("POST /test-logs/_search*").toQueryString(KEYWORD)); + } + + @Test + void keyword_escaped_wildcard_is_an_exact_term() { + // The user escaped '*', so it is a literal — no wildcard, so the exact-term form applies. + assertEquals("\"foo \\*\"", q("foo \\*").toQueryString(KEYWORD)); + } + + @Test + void constant_keyword_behaves_like_keyword() { + assertEquals("\"foo bar\"", q("foo bar").toQueryString(CONSTANT_KEYWORD)); + assertEquals("foo\\ bar*", q("foo bar*").toQueryString(CONSTANT_KEYWORD)); + } + + // ------------------------------------------------------------------------- + // OTHER MAPPINGS + unresolved: legacy behavior (space check), untouched. + // ------------------------------------------------------------------------- + + @Test + void date_field_keeps_legacy_unquoted_form() { + // Hyphens are escaped by the legacy path too — this is unchanged from before the mapping + // split, and query_string still parses the value as a date. + assertEquals("2024\\-01\\-15", q("2024-01-15").toQueryString(DATE)); + } + + @Test + void numeric_field_keeps_legacy_unquoted_form() { + assertEquals("not\\-a\\-number", q("not-a-number").toQueryString(LONG)); + } + + @Test + void unresolved_field_keeps_legacy_space_check() { + assertEquals("\"foo bar*\"", q("foo bar*").toQueryString((ExprType) null)); + assertEquals("foo*", q("foo*").toQueryString((ExprType) null)); + } + + @Test + void argless_toQueryString_matches_null_type_branch() { + SearchLiteral lit = q("foo bar*"); + assertEquals(lit.toQueryString((ExprType) null), lit.toQueryString()); + } + + // ------------------------------------------------------------------------- + // End-to-end via SearchComparison + resolver — the actual planner call path. + // ------------------------------------------------------------------------- + + private static SearchComparison cmp(String field, SearchLiteral value) { + return new SearchComparison( + new Field(new QualifiedName(field), List.of()), SearchComparison.Operator.EQUALS, value); + } + + @Test + void comparison_emits_wildcard_pattern_on_keyword() { + Function resolver = Map.of("name", KEYWORD)::get; + assertEquals("name:foo\\ bar*", cmp("name", q("foo bar*")).toQueryString(resolver)); + } + + @Test + void comparison_emits_phrase_on_text_when_quoted() { + Function resolver = Map.of("body", TEXT)::get; + assertEquals("body:\"foo=bar\"", cmp("body", q("foo=bar")).toQueryString(resolver)); + } + + @Test + void comparison_emits_bare_term_on_text_when_unquoted() { + Function resolver = Map.of("body", TEXT)::get; + assertEquals("body:foo*", cmp("body", bare("foo*")).toQueryString(resolver)); + } + + @Test + void comparison_unknown_field_falls_back_to_legacy_form() { + Function resolver = f -> null; + assertEquals("unmapped:\"foo bar*\"", cmp("unmapped", q("foo bar*")).toQueryString(resolver)); + } +} diff --git a/docs/user/ppl/cmd/search.md b/docs/user/ppl/cmd/search.md index 7ebd30fc932..f7e440e8fc6 100644 --- a/docs/user/ppl/cmd/search.md +++ b/docs/user/ppl/cmd/search.md @@ -270,7 +270,7 @@ fetched rows / total rows = 11/11 Combine conditions with `AND` to require all criteria to match: ```ppl -search severityText="INFO" AND `resource.attributes.service.name`="cart-service" source=otellogs +search severityText="INFO" AND `resource.attributes.service.name`="cart*" source=otellogs | fields body | head 1 ``` @@ -330,12 +330,13 @@ search instrumentationScope.name!="@opentelemetry/instrumentation-http" source=o The query returns the following results: ```text -fetched rows / total rows = 1/1 -+------------------------------+ -| instrumentationScope.name | -|------------------------------| -| Microsoft.Extensions.Hosting | -+------------------------------+ +fetched rows / total rows = 2/2 ++-----------------------------------------------------------------------------+ +| instrumentationScope.name | +|-----------------------------------------------------------------------------| +| Microsoft.Extensions.Hosting | +| go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc | ++-----------------------------------------------------------------------------+ ``` **`NOT` operator** @@ -352,15 +353,15 @@ The query returns the following results: ```text fetched rows / total rows = 5/5 -+------------------------------+ -| instrumentationScope.name | -|------------------------------| -| Microsoft.Extensions.Hosting | -| null | -| null | -| null | -| null | -+------------------------------+ ++-----------------------------------------------------------------------------+ +| instrumentationScope.name | +|-----------------------------------------------------------------------------| +| Microsoft.Extensions.Hosting | +| go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc | +| null | +| null | +| null | ++-----------------------------------------------------------------------------+ ``` ## Example 5: Querying ranges diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteSearchCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteSearchCommandIT.java index e1743b5fc26..5cd5dc19e11 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteSearchCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteSearchCommandIT.java @@ -5,12 +5,430 @@ package org.opensearch.sql.calcite.remote; +import static org.opensearch.sql.util.MatcherUtils.rows; +import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; +import static org.opensearch.sql.util.MatcherUtils.verifyNumOfRows; + +import java.io.IOException; +import org.json.JSONObject; +import org.junit.jupiter.api.Test; +import org.opensearch.client.Request; +import org.opensearch.client.ResponseException; import org.opensearch.sql.ppl.SearchCommandIT; public class CalciteSearchCommandIT extends SearchCommandIT { + + private static final String IDX_KEYWORD = "test_5682_keyword"; + private static final String IDX_TEXT = "test_5682_text"; + + /** Group 1-6 matrix: one value per wildcard-placement / special-character combination. */ + private static final String MATRIX_DOCS = + "{\"index\":{}}\n{\"name\":\"foo\"}\n" + + "{\"index\":{}}\n{\"name\":\"foobar\"}\n" + + "{\"index\":{}}\n{\"name\":\"food\"}\n" + + "{\"index\":{}}\n{\"name\":\"FOO\"}\n" + + "{\"index\":{}}\n{\"name\":\"foo bar\"}\n" + + "{\"index\":{}}\n{\"name\":\"foo barbaz\"}\n" + + "{\"index\":{}}\n{\"name\":\"foo-bar\"}\n" + + "{\"index\":{}}\n{\"name\":\"foo_bar\"}\n" + + "{\"index\":{}}\n{\"name\":\"foo.bar\"}\n" + + "{\"index\":{}}\n{\"name\":\"foo/bar\"}\n" + + "{\"index\":{}}\n{\"name\":\"foo@bar\"}\n"; + + private static final String IDX_SPLIT_KEYWORD = "test_5682_split_keyword"; + private static final String IDX_SPLIT_TEXT = "test_5682_split_text"; + + /** + * Group 7 fixture. {@code foo=bar} holds no whitespace, yet the text analyzer still splits it + * into [foo, bar] — so the single-token {@code foo} and {@code bar} docs used to OR-match a + * search for {@code "foo=bar"}. + */ + private static final String SPLIT_DOCS = + "{\"index\":{}}\n{\"name\":\"foo=bar\"}\n" + + "{\"index\":{}}\n{\"name\":\"foo bar\"}\n" + + "{\"index\":{}}\n{\"name\":\"foo\"}\n" + + "{\"index\":{}}\n{\"name\":\"bar\"}\n" + + "{\"index\":{}}\n{\"name\":\"baz\"}\n"; + @Override public void init() throws Exception { super.init(); enableCalcite(); + setupSpecIndices(); + } + + private void setupSpecIndices() throws IOException { + createSpecIndex(IDX_KEYWORD, "keyword", MATRIX_DOCS); + createSpecIndex(IDX_TEXT, "text", MATRIX_DOCS); + createSpecIndex(IDX_SPLIT_KEYWORD, "keyword", SPLIT_DOCS); + createSpecIndex(IDX_SPLIT_TEXT, "text", SPLIT_DOCS); + } + + private void createSpecIndex(String indexName, String fieldType, String docs) throws IOException { + try { + client().performRequest(new Request("DELETE", "/" + indexName)); + } catch (ResponseException ignore) { + // ok + } + + Request createIndex = new Request("PUT", "/" + indexName); + createIndex.setJsonEntity( + "{\n" + + " \"settings\": {\"number_of_shards\": 1, \"number_of_replicas\": 0},\n" + + " \"mappings\": {\n" + + " \"properties\": {\n" + + " \"name\": {\"type\": \"" + + fieldType + + "\"}\n" + + " }\n" + + " }\n" + + "}"); + client().performRequest(createIndex); + + Request bulk = new Request("POST", "/" + indexName + "/_bulk?refresh=true"); + bulk.setJsonEntity(docs); + client().performRequest(bulk); + } + + private JSONObject search(String indexName, String predicate) throws IOException { + // Escape embedded quotes for JSON body wrapping done by executeQuery helper. + String query = + "search source=" + indexName + " " + predicate.replace("\"", "\\\"") + " | fields name"; + return executeQuery(query); + } + + // ============================================================================= + // Group 1 — no special chars, no wildcards + // ============================================================================= + + @Test + public void testGroup1_1_keyword_foo() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=foo"), 1); + } + + @Test + public void testGroup1_1_text_foo() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=foo"), 7); + } + + @Test + public void testGroup1_2_keyword_quoted_foo() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo\""), 1); + } + + @Test + public void testGroup1_2_text_quoted_foo() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo\""), 7); + } + + // ============================================================================= + // Group 2 — special chars, no wildcards + // ============================================================================= + + @Test + public void testGroup2_1_keyword_underscore() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo_bar\""), 1); + } + + @Test + public void testGroup2_1_text_underscore() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo_bar\""), 1); + } + + @Test + public void testGroup2_2_keyword_dot() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo.bar\""), 1); + } + + @Test + public void testGroup2_2_text_dot() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo.bar\""), 1); + } + + @Test + public void testGroup2_3_keyword_hyphen() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo-bar\""), 1); + } + + @Test + public void testGroup2_3_text_hyphen() throws IOException { + // Quoted with no wildcard -> phrase. Analyzer yields [foo, bar]; PhraseQuery requires them + // adjacent, so this no longer OR-matches every doc holding either token. + verifyNumOfRows(search(IDX_TEXT, "name=\"foo-bar\""), 4); + } + + @Test + public void testGroup2_4_keyword_slash() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo/bar\""), 1); + } + + @Test + public void testGroup2_4_text_slash() throws IOException { + // Quoted with no wildcard -> phrase over [foo, bar]. + verifyNumOfRows(search(IDX_TEXT, "name=\"foo/bar\""), 4); + } + + @Test + public void testGroup2_5_keyword_at() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo@bar\""), 1); + } + + @Test + public void testGroup2_5_text_at() throws IOException { + // Quoted with no wildcard -> phrase over [foo, bar]. '@' splits in the analyzer even though it + // is not a query_string reserved character. + verifyNumOfRows(search(IDX_TEXT, "name=\"foo@bar\""), 4); + } + + @Test + public void testGroup2_6_keyword_space() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo bar\""), 1); + } + + @Test + public void testGroup2_6_text_space() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo bar\""), 4); + } + + // ============================================================================= + // Group 3 — trailing wildcard (postfix) + // ============================================================================= + + @Test + public void testGroup3_1_keyword_unquoted_foostar() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=foo*"), 10); + } + + @Test + public void testGroup3_1_text_unquoted_foostar() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=foo*"), 11); + } + + @Test + public void testGroup3_2_keyword_quoted_foostar() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo*\""), 10); + } + + @Test + public void testGroup3_2_text_quoted_foostar() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo*\""), 11); + } + + @Test + public void testGroup3_3_keyword_foo_underscore_star() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo_*\""), 1); + } + + @Test + public void testGroup3_3_text_foo_underscore_star() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo_*\""), 1); + } + + @Test + public void testGroup3_4_keyword_foo_dot_star() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo.*\""), 1); + } + + @Test + public void testGroup3_4_text_foo_dot_star() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo.*\""), 1); + } + + @Test + public void testGroup3_5_keyword_foo_hyphen_star() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo-*\""), 1); + } + + @Test + public void testGroup3_5_text_foo_hyphen_star() throws IOException { + // Emitted: name:foo\-* (unquoted, - escaped). This is a WildcardQuery over the analyzed + // token dictionary — text tokens don't contain '-'. 0 hits (accepted analyzer limit). + verifyNumOfRows(search(IDX_TEXT, "name=\"foo-*\""), 0); + } + + @Test + public void testGroup3_6_keyword_foo_slash_star() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo/*\""), 1); + } + + @Test + public void testGroup3_6_text_foo_slash_star() throws IOException { + // Emitted: name:foo\/* (unquoted, / escaped). WildcardQuery over analyzed tokens; + // text tokens don't contain '/'. 0 hits (accepted analyzer limit). + verifyNumOfRows(search(IDX_TEXT, "name=\"foo/*\""), 0); + } + + @Test + public void testGroup3_7_keyword_foo_space_barstar() throws IOException { + // Reported bug row (#5682): keyword whole-value pattern "foo bar*" should match + // "foo bar" and "foo barbaz". + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo bar*\""), 2); + } + + @Test + public void testGroup3_7_text_foo_space_barstar() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo bar*\""), 4); + } + + // ============================================================================= + // Group 4 — leading wildcard (prefix) + // ============================================================================= + + @Test + public void testGroup4_1_keyword_starfoo() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"*foo\""), 1); + } + + @Test + public void testGroup4_1_text_starfoo() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"*foo\""), 7); + } + + @Test + public void testGroup4_2_keyword_starbar() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"*bar\""), 7); + } + + @Test + public void testGroup4_2_text_starbar() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"*bar\""), 7); + } + + @Test + public void testGroup4_3_keyword_starfoo_bar() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"*foo bar\""), 1); + } + + @Test + public void testGroup4_3_text_starfoo_bar() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"*foo bar\""), 4); + } + + // ============================================================================= + // Group 5 — interior wildcard (* in-between) + // ============================================================================= + + @Test + public void testGroup5_1_keyword_fstar_r() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"f*r\""), 7); + } + + @Test + public void testGroup5_1_text_fstar_r() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"f*r\""), 3); + } + + @Test + public void testGroup5_2_keyword_foostar_bar() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo*bar\""), 7); + } + + @Test + public void testGroup5_2_text_foostar_bar() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo*bar\""), 3); + } + + @Test + public void testGroup5_3_keyword_foo_space_starbaz() throws IOException { + // Keyword whole-value wildcard: "foo *baz" matches "foo barbaz". + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo *baz\""), 1); + } + + @Test + public void testGroup5_3_text_foo_space_starbaz() throws IOException { + // Corpus-dependent 0 (no adjacent tokens foo→baz analyzed in this fixture). + verifyNumOfRows(search(IDX_TEXT, "name=\"foo *baz\""), 0); + } + + @Test + public void testGroup5_4_keyword_starfoo_barstar() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"*foo bar*\""), 2); + } + + @Test + public void testGroup5_4_text_starfoo_barstar() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"*foo bar*\""), 4); + } + + // ============================================================================= + // Group 6 — ? wildcard (exactly one character) + // ============================================================================= + + @Test + public void testGroup6_1_keyword_foo_qmark() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo?\""), 1); + } + + @Test + public void testGroup6_1_text_foo_qmark() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo?\""), 1); + } + + @Test + public void testGroup6_2_keyword_qmark_oo() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"?oo\""), 1); + } + + @Test + public void testGroup6_2_text_qmark_oo() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"?oo\""), 7); + } + + @Test + public void testGroup6_3_keyword_f_qmark_o() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"f?o\""), 1); + } + + @Test + public void testGroup6_3_text_f_qmark_o() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"f?o\""), 7); + } + + @Test + public void testGroup6_4_keyword_foo_qmark_bar() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo?bar\""), 6); + } + + @Test + public void testGroup6_4_text_foo_qmark_bar() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo?bar\""), 2); + } + + @Test + public void testGroup6_5_keyword_foo_space_bqmarkr() throws IOException { + verifyNumOfRows(search(IDX_KEYWORD, "name=\"foo b?r\""), 1); + } + + @Test + public void testGroup6_5_text_foo_space_bqmarkr() throws IOException { + verifyNumOfRows(search(IDX_TEXT, "name=\"foo b?r\""), 0); + } + + // ============================================================================= + // Group 7 — a quoted value must not OR-match its halves. Uses SPLIT_DOCS. + // ============================================================================= + + @Test + public void testGroup7_1_text_quoted_split_value_excludes_halves() throws IOException { + // The `foo` and `bar` docs hold one half each and must be absent. + verifyDataRows(search(IDX_SPLIT_TEXT, "name=\"foo=bar\""), rows("foo=bar"), rows("foo bar")); + } + + @Test + public void testGroup7_2_keyword_quoted_split_value_is_exact() throws IOException { + verifyDataRows(search(IDX_SPLIT_KEYWORD, "name=\"foo=bar\""), rows("foo=bar")); + } + + @Test + public void testGroup7_3_text_quoted_wildcard_term_keeps_wildcard() throws IOException { + // Exception to 7.1: a wildcard with no whitespace stays unquoted, so `foo*` still matches + // `foobar` and `food` rather than collapsing to the phrase [foo]. + verifyNumOfRows(search(IDX_TEXT, "name=\"foo*\""), 11); + } + + @Test + public void testGroup7_4_keyword_quoting_is_irrelevant() throws IOException { + verifyDataRows(search(IDX_KEYWORD, "name=foo-bar"), rows("foo-bar")); + verifyDataRows(search(IDX_KEYWORD, "name=\"foo-bar\""), rows("foo-bar")); } } diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java index b7031a1bd68..43259b59316 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java @@ -1138,7 +1138,9 @@ public SearchLiteral visitSearchLiteral(OpenSearchPPLParser.SearchLiteralContext // Use visit method to properly handle escaping Literal stringLit = (Literal) visit(ctx.stringLiteral()); String content = (String) stringLit.getValue(); - return new SearchLiteral(new Literal(content, DataType.STRING), content.contains(" ")); + // The value came from a quoted literal. On a text field that is the user asking for phrase + // semantics; SearchLiteral decides what to emit per index mapping. + return new SearchLiteral(new Literal(content, DataType.STRING), true); } else if (ctx.numericLiteral() != null) { Literal numericLiteral = (Literal) visit(ctx.numericLiteral()); return new SearchLiteral(numericLiteral, false);