From 8ad2755a2ac8a9b25d2993c0716d35329bec2441 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Wed, 12 Aug 2026 14:33:13 -0700 Subject: [PATCH 1/3] Fix PPL search command dropping wildcards on values with whitespace (#5682) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the Calcite path, `search source=idx name="foo bar*"` against a keyword field returned 0 hits instead of matching the whole-value pattern `foo bar*`. The parser marked whitespace-containing literals as phrases, which emitted `name:"foo bar*"` — inside a Lucene phrase, `*` is a literal character, so it looked for docs containing `*` in the stored value and found none. Route emission per field mapping in SearchLiteral.toQueryString(ExprType): - text-like (text, match_only_text) → quoted phrase (unchanged) - non-text (keyword, etc.) with whitespace + unescaped wildcard → unquoted term with the space escaped, so query_string keeps the value as one whole-value pattern instead of splitting into two clauses - everything else (no whitespace, or phrase without wildcard) → legacy branches (unquoted-with-escapes, quoted phrase) The Calcite RelDataType round trip in CalciteRelNodeVisitor.visitSearch collapses `text` mapping to plain VARCHAR (OpenSearchTypeFactory:208), which erased the text/keyword distinction at the emitter. Read the ExprType map directly from AbstractOpenSearchTable.getFieldTypes() instead; TODO comment marks the follow-up to move this metadata onto a RelDataType/scan annotation once the Calcite rule pipeline is audited. Thread a `Function` resolver through the SearchExpression hierarchy (SearchComparison, SearchIn, SearchAnd/Or/Not/Group, SearchLiteral) so SearchLiteral can consult the resolved field's index type at emit time. Tests: 54 new Group1-Group6 tests in CalciteSearchCommandIT covering the full text × keyword × wildcard-placement matrix on a shared fixture, plus a core-level SearchLiteralTest for the emission decision table. Verified with `./gradlew doctest -DignorePrometheus` (85 tests) and `./gradlew -DignorePrometheus :integ-test:integTest` (30m36s, 0 failures). Signed-off-by: Peng Huo --- .../sql/ast/expression/SearchAnd.java | 6 +- .../sql/ast/expression/SearchComparison.java | 10 +- .../sql/ast/expression/SearchExpression.java | 20 +- .../sql/ast/expression/SearchGroup.java | 6 +- .../sql/ast/expression/SearchIn.java | 8 +- .../sql/ast/expression/SearchLiteral.java | 57 ++- .../sql/ast/expression/SearchNot.java | 6 +- .../sql/ast/expression/SearchOr.java | 6 +- .../sql/calcite/CalciteRelNodeVisitor.java | 28 +- .../sql/ast/expression/SearchLiteralTest.java | 203 ++++++++++ .../remote/CalciteSearchCommandIT.java | 370 ++++++++++++++++++ 11 files changed, 698 insertions(+), 22 deletions(-) create mode 100644 core/src/test/java/org/opensearch/sql/ast/expression/SearchLiteralTest.java 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..6e184286d4f 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. */ @@ -24,7 +26,23 @@ public class SearchLiteral extends SearchExpression { private final boolean isPhrase; @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}. The decision tree is documented in {@code + * docs/dev/ppl-search-command-contract-empirical.md} — briefly: whitespace + wildcard on a + * non-text index escapes the space so the parser keeps the value as one whole-value pattern; + * everything else falls through to phrase (with whitespace) or unquoted-escaped (without). + * + * @param indexType the enclosing field's OpenSearch index-mapping type (text/keyword/...) — used + * only to distinguish text-like from everything else; null means unknown, treated as + * text-like so we don't regress the phrase form. + */ + public String toQueryString(ExprType indexType) { if (literal instanceof Literal) { Literal lit = (Literal) literal; Object val = lit.getValue(); @@ -38,23 +56,52 @@ public String toQueryString() { if (val instanceof String) { String str = (String) val; - // Phrase search - preserve quotes + // [D] whitespace + wildcard on a non-text index: single term with space escaped, so the + // query_string parser keeps the value as one whole-value pattern (a raw space would + // split it into two clauses and drop the field binding on the right half). + if (isPhrase && !isTextLike(indexType) && hasUnescapedWildcard(str)) { + return QueryStringUtils.escapeLuceneSpecialCharacters(str).replace(" ", "\\ "); + } + + // [B]/[C] quoted phrase. if (isPhrase) { - // Escape special chars inside the phrase str = QueryStringUtils.escapeLuceneSpecialCharacters(str); return "\"" + str + "\""; } - // Regular string - escape special characters + // [A] unquoted; escape Lucene specials, wildcards preserved. return QueryStringUtils.escapeLuceneSpecialCharacters(str); } } - // Default: escape the text representation String text = literal.toString(); return QueryStringUtils.escapeLuceneSpecialCharacters(text); } + private static boolean isTextLike(ExprType type) { + if (type == null) { + // Unknown type → treat as text-like so we take the phrase branch and avoid a text + // regression when the resolver fails to identify the field. + return true; + } + String legacyName = type.getOriginalExprType().legacyTypeName(); + return "TEXT".equalsIgnoreCase(legacyName) || "MATCH_ONLY_TEXT".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 public String toAnonymizedString() { return "***"; 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..6efdb5b8b58 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/ast/expression/SearchLiteralTest.java @@ -0,0 +1,203 @@ +/* + * 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; + +/** + * Field-type-aware emission for {@link SearchLiteral} and {@link SearchComparison}. + * + *

Space + unescaped wildcard on a keyword field must not be phrase-quoted (the phrase form + * silently strips wildcard semantics inside quotes on keyword). On text (or when the type is + * unknown), the legacy phrase form is preserved to avoid regressing today's behavior — see the + * repro matrix in issue #5682. + */ +class SearchLiteralTest { + + /** Stub {@link ExprType} that reports a given legacyTypeName — enough for isTextLike(). */ + 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 TEXT = typeOf("TEXT"); + private static final ExprType MATCH_ONLY_TEXT = typeOf("MATCH_ONLY_TEXT"); + + private static SearchLiteral phrase(String value) { + return new SearchLiteral(new Literal(value, DataType.STRING), true); + } + + private static SearchLiteral bare(String value) { + return new SearchLiteral(new Literal(value, DataType.STRING), false); + } + + // ------------------------------------------------------------------------- + // Phrase without wildcard: unchanged on both field types (regression guard). + // ------------------------------------------------------------------------- + + @Test + void phrase_no_wildcard_keyword_stays_quoted() { + assertEquals("\"foo bar\"", phrase("foo bar").toQueryString(KEYWORD)); + } + + @Test + void phrase_no_wildcard_text_stays_quoted() { + assertEquals("\"foo bar\"", phrase("foo bar").toQueryString(TEXT)); + } + + // ------------------------------------------------------------------------- + // Phrase with unescaped wildcard on keyword: emit escaped-space wildcard term. + // These are the D-family rows fixed by issue #5682. + // ------------------------------------------------------------------------- + + @Test + void phrase_trailing_wildcard_keyword_emits_escaped_space_prefix() { + // P6-k: name="foo bar*" → PrefixQuery on whole keyword term + assertEquals("foo\\ bar*", phrase("foo bar*").toQueryString(KEYWORD)); + } + + @Test + void phrase_leading_wildcard_keyword_emits_escaped_space_wildcard() { + // L3-k: name="*foo bar" + assertEquals("*foo\\ bar", phrase("*foo bar").toQueryString(KEYWORD)); + } + + @Test + void phrase_interior_wildcard_keyword_emits_escaped_space_wildcard() { + // I3-k: name="foo *baz" + assertEquals("foo\\ *baz", phrase("foo *baz").toQueryString(KEYWORD)); + } + + @Test + void phrase_question_wildcard_keyword_emits_escaped_space_wildcard() { + // Q5-k: name="foo b?r" + assertEquals("foo\\ b?r", phrase("foo b?r").toQueryString(KEYWORD)); + } + + @Test + void phrase_with_special_chars_and_wildcard_keyword_escapes_all() { + // D3-k repro: name="POST /test-logs/_search*" → PrefixQuery, special chars still escaped. + assertEquals( + "POST\\ \\/test\\-logs\\/_search*", + phrase("POST /test-logs/_search*").toQueryString(KEYWORD)); + } + + // ------------------------------------------------------------------------- + // Phrase with unescaped wildcard on text / match_only_text / unknown: keep + // legacy phrase form (no regression on text; wildcard silently ignored by + // Lucene inside phrase, same as today). + // ------------------------------------------------------------------------- + + @Test + void phrase_wildcard_text_keeps_phrase_form() { + assertEquals("\"foo bar*\"", phrase("foo bar*").toQueryString(TEXT)); + } + + @Test + void phrase_wildcard_match_only_text_keeps_phrase_form() { + assertEquals("\"foo bar*\"", phrase("foo bar*").toQueryString(MATCH_ONLY_TEXT)); + } + + @Test + void phrase_wildcard_unknown_type_keeps_phrase_form() { + // Unknown → treat as text-like so we never regress an unresolvable field. + assertEquals("\"foo bar*\"", phrase("foo bar*").toQueryString((ExprType) null)); + } + + // ------------------------------------------------------------------------- + // Phrase with escaped wildcard: user asked for a LITERAL '*'/'?' — keep the + // phrase form even on keyword. The isPhrase flag was set for a reason. + // ------------------------------------------------------------------------- + + @Test + void phrase_escaped_wildcard_keyword_keeps_phrase_form() { + // Input string literally contains `foo \*` (backslash + '*'). hasUnescapedWildcard() sees the + // backslash and skips '*', so we treat this as a phrase with a literal '*' — no emission + // change. QueryStringUtils.escapeLuceneSpecialCharacters keeps '\\' and '*' untouched. + assertEquals("\"foo \\*\"", phrase("foo \\*").toQueryString(KEYWORD)); + } + + // ------------------------------------------------------------------------- + // Non-phrase (no space): field type does not matter — legacy code path. + // ------------------------------------------------------------------------- + + @Test + void bare_wildcard_keyword_emits_unquoted() { + // P1: name="foo*" — never a phrase, works today. + assertEquals("foo*", bare("foo*").toQueryString(KEYWORD)); + } + + @Test + void bare_wildcard_text_emits_unquoted() { + assertEquals("foo*", bare("foo*").toQueryString(TEXT)); + } + + // ------------------------------------------------------------------------- + // Backwards-compat: arg-less toQueryString() must match the null-type branch + // (legacy phrase form) so callers that never wire the resolver see no change. + // ------------------------------------------------------------------------- + + @Test + void argless_toQueryString_matches_null_type_branch() { + SearchLiteral lit = phrase("foo bar*"); + assertEquals(lit.toQueryString((ExprType) null), lit.toQueryString()); + } + + // ------------------------------------------------------------------------- + // End-to-end via SearchComparison + resolver — the actual planner call path. + // ------------------------------------------------------------------------- + + @Test + void comparison_resolves_field_type_and_emits_wildcard_on_keyword() { + // name="foo bar*" on a keyword field → name:foo\ bar* + SearchComparison cmp = + new SearchComparison( + new Field(new QualifiedName("name"), List.of()), + SearchComparison.Operator.EQUALS, + phrase("foo bar*")); + Function resolver = Map.of("name", KEYWORD)::get; + assertEquals("name:foo\\ bar*", cmp.toQueryString(resolver)); + } + + @Test + void comparison_resolves_field_type_and_keeps_phrase_on_text() { + SearchComparison cmp = + new SearchComparison( + new Field(new QualifiedName("name"), List.of()), + SearchComparison.Operator.EQUALS, + phrase("foo bar*")); + Function resolver = Map.of("name", TEXT)::get; + assertEquals("name:\"foo bar*\"", cmp.toQueryString(resolver)); + } + + @Test + void comparison_unknown_field_falls_back_to_phrase_form() { + SearchComparison cmp = + new SearchComparison( + new Field(new QualifiedName("unmapped"), List.of()), + SearchComparison.Operator.EQUALS, + phrase("foo bar*")); + // Resolver returns null for unknown fields. + Function resolver = f -> null; + assertEquals("unmapped:\"foo bar*\"", cmp.toQueryString(resolver)); + } +} 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..0d227280f72 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,382 @@ package org.opensearch.sql.calcite.remote; +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"; + @Override public void init() throws Exception { super.init(); enableCalcite(); + setupSpecIndices(); + } + + private void setupSpecIndices() throws IOException { + createSpecIndex(IDX_KEYWORD, "keyword"); + createSpecIndex(IDX_TEXT, "text"); + } + + private void createSpecIndex(String indexName, String fieldType) 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( + "{\"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"); + 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 { + // Emitted: name:foo\-bar (unquoted, - escaped so parser treats as literal). + // Analyzer tokenizes to [foo, bar]; boolean OR matches 7 docs whose analyzed tokens + // contain foo or bar. + verifyNumOfRows(search(IDX_TEXT, "name=\"foo-bar\""), 7); + } + + @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 { + // Emitted: name:foo\/bar (unquoted, / escaped). Analyzer tokenizes to [foo, bar]; + // boolean OR matches 7 docs. + verifyNumOfRows(search(IDX_TEXT, "name=\"foo/bar\""), 7); + } + + @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 { + // Emitted: name:foo@bar (unquoted; @ is not a Lucene special). Analyzer tokenizes + // to [foo, bar]; boolean OR matches 7 docs. + verifyNumOfRows(search(IDX_TEXT, "name=\"foo@bar\""), 7); + } + + @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); } } From d9d656839be331aa15db3478bb92417cded59e94 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Fri, 14 Aug 2026 15:06:09 -0700 Subject: [PATCH 2/3] Drive search command emission from the field's index mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the whitespace heuristic that decided phrase vs. term emission. `SearchLiteral.isPhrase` was set at parse time as `value.contains(" ")`, which is a syntactic test standing in for a semantic question: will the field's analyzer split this value into multiple tokens? Whitespace is a poor proxy — `foo=bar` and `foo-bar` hold none, yet the standard analyzer splits both. The consequence on a text field: the value was emitted unquoted, query_string kept it as one field-scoped term, the analyzer split it, and default_operator=OR combined the halves. `body="foo=bar"` therefore matched any document holding just `foo` or just `bar`. Emission is now selected by the enclosing field's mapping, read from AbstractOpenSearchTable.getFieldTypes(): - text / match_only_text: honor the user's quoting. Unquoted passes through so `*` and `?` stay query_string operators; quoted becomes a phrase. Exception: a whitespace-free value carrying a wildcard stays unquoted, because quoting would let the analyzer discard the wildcard (`foo*` must keep matching `foobar`). That is only safe without whitespace — with a space, unquoted would split into separate clauses and the tail would lose its field binding. - keyword / constant_keyword: quoting is irrelevant, since the analyzer is a no-op and a quoted phrase resolves to the same single term as a bare one. Emit whole-value semantics instead — a wildcard pattern when the value holds an unescaped wildcard, otherwise an exact term. Whitespace is escaped in the wildcard form so query_string keeps one clause. - date / numeric / ip / boolean / unresolved: legacy behavior, untouched. The v2 engine is unaffected. It reaches emission through the no-arg SearchExpression.toQueryString(), which passes a null-returning resolver and lands in the legacy branch. Behavior change, text fields only: a quoted value the analyzer splits is now a phrase rather than an OR over its tokens. On the test fixture, `name="foo-bar"` / `"foo/bar"` / `"foo@bar"` go from 7 hits to 4. Wildcard rows are unchanged. Three examples in docs/user/ppl/cmd/search.md documented the old over-matching and have been updated. Tests: Group 7 added to CalciteSearchCommandIT over a dedicated fixture (foo=bar, foo bar, foo, bar, baz) so the single-token documents that used to OR-match are asserted absent. SearchLiteralTest covers the three mapping branches. Verified with CalciteSearchCommandIT, SearchCommandIT (v2), :core:test, :ppl:test, doctest, and :integ-test:integTest. Signed-off-by: Peng Huo --- .../sql/ast/expression/SearchLiteral.java | 77 ++++--- .../sql/ast/expression/SearchLiteralTest.java | 199 ++++++++++-------- docs/user/ppl/cmd/search.md | 33 +-- .../remote/CalciteSearchCommandIT.java | 98 ++++++--- .../sql/ppl/parser/AstExpressionBuilder.java | 4 +- 5 files changed, 258 insertions(+), 153 deletions(-) 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 6e184286d4f..25a2cf0790e 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 @@ -23,7 +23,12 @@ 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(Function fieldTypeResolver) { @@ -33,14 +38,23 @@ public String toQueryString(Function fieldTypeResolver) { /** * Emits the query_string form for a literal on the RHS of {@link SearchComparison} or inside - * {@link SearchIn}. The decision tree is documented in {@code - * docs/dev/ppl-search-command-contract-empirical.md} — briefly: whitespace + wildcard on a - * non-text index escapes the space so the parser keeps the value as one whole-value pattern; - * everything else falls through to phrase (with whitespace) or unquoted-escaped (without). + * {@link SearchIn}. Emission is driven by the enclosing field's index mapping, because the + * mapping decides whether the value gets analyzed: + * + *

    + *
  • 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. + *
  • keyword family — quoting is irrelevant, because the analyzer is a no-op and a + * quoted phrase resolves to the same single term as a bare one. Emit whole-value semantics + * instead: a wildcard pattern when the value holds an unescaped wildcard, otherwise an + * exact term. Whitespace is escaped in the wildcard form so query_string keeps the value as + * one clause rather than splitting at the space and dropping the field binding. + *
  • anything else (date, numeric, ip, boolean, or unresolved) — legacy behavior, + * untouched. Those mappings do their own value parsing and are out of scope here. + *
* - * @param indexType the enclosing field's OpenSearch index-mapping type (text/keyword/...) — used - * only to distinguish text-like from everything else; null means unknown, treated as - * text-like so we don't regress the phrase form. + * @param indexType the enclosing field's OpenSearch index-mapping type, or null if unresolved */ public String toQueryString(ExprType indexType) { if (literal instanceof Literal) { @@ -56,38 +70,53 @@ public String toQueryString(ExprType indexType) { if (val instanceof String) { String str = (String) val; - // [D] whitespace + wildcard on a non-text index: single term with space escaped, so the - // query_string parser keeps the value as one whole-value pattern (a raw space would - // split it into two clauses and drop the field binding on the right half). - if (isPhrase && !isTextLike(indexType) && hasUnescapedWildcard(str)) { - return QueryStringUtils.escapeLuceneSpecialCharacters(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); } - // [B]/[C] quoted phrase. - if (isPhrase) { - str = QueryStringUtils.escapeLuceneSpecialCharacters(str); - return "\"" + str + "\""; + if (isKeywordLike(indexType)) { + return hasUnescapedWildcard(str) ? unquoted(str).replace(" ", "\\ ") : quoted(str); } - // [A] unquoted; escape Lucene specials, wildcards preserved. - return QueryStringUtils.escapeLuceneSpecialCharacters(str); + // Other mappings (date, numeric, ip, boolean) and unresolved fields: legacy behavior. + return str.contains(" ") ? quoted(str) : unquoted(str); } } - 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) { - // Unknown type → treat as text-like so we take the phrase branch and avoid a text - // regression when the resolver fails to identify the field. - return true; + 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); 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 index 6efdb5b8b58..904fc86deb8 100644 --- a/core/src/test/java/org/opensearch/sql/ast/expression/SearchLiteralTest.java +++ b/core/src/test/java/org/opensearch/sql/ast/expression/SearchLiteralTest.java @@ -14,16 +14,17 @@ import org.opensearch.sql.data.type.ExprType; /** - * Field-type-aware emission for {@link SearchLiteral} and {@link SearchComparison}. + * Index-mapping-aware emission for {@link SearchLiteral} and {@link SearchComparison}. * - *

Space + unescaped wildcard on a keyword field must not be phrase-quoted (the phrase form - * silently strips wildcard semantics inside quotes on keyword). On text (or when the type is - * unknown), the legacy phrase form is preserved to avoid regressing today's behavior — see the - * repro matrix in issue #5682. + *

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} that reports a given legacyTypeName — enough for isTextLike(). */ + /** Stub {@link ExprType} reporting a given legacyTypeName — enough for the mapping checks. */ private static ExprType typeOf(String legacyName) { return new ExprType() { @Override @@ -39,126 +40,156 @@ public String legacyTypeName() { } 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"); - private static SearchLiteral phrase(String value) { + /** 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); } // ------------------------------------------------------------------------- - // Phrase without wildcard: unchanged on both field types (regression guard). + // TEXT: honor the user's quoting. // ------------------------------------------------------------------------- @Test - void phrase_no_wildcard_keyword_stays_quoted() { - assertEquals("\"foo bar\"", phrase("foo bar").toQueryString(KEYWORD)); + void text_quoted_emits_phrase() { + assertEquals("\"foo bar\"", q("foo bar").toQueryString(TEXT)); } @Test - void phrase_no_wildcard_text_stays_quoted() { - assertEquals("\"foo bar\"", phrase("foo bar").toQueryString(TEXT)); + void text_unquoted_emits_bare_term() { + assertEquals("foo", bare("foo").toQueryString(TEXT)); } - // ------------------------------------------------------------------------- - // Phrase with unescaped wildcard on keyword: emit escaped-space wildcard term. - // These are the D-family rows fixed by issue #5682. - // ------------------------------------------------------------------------- + @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 phrase_trailing_wildcard_keyword_emits_escaped_space_prefix() { - // P6-k: name="foo bar*" → PrefixQuery on whole keyword term - assertEquals("foo\\ bar*", phrase("foo bar*").toQueryString(KEYWORD)); + 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 phrase_leading_wildcard_keyword_emits_escaped_space_wildcard() { - // L3-k: name="*foo bar" - assertEquals("*foo\\ bar", phrase("*foo bar").toQueryString(KEYWORD)); + 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 phrase_interior_wildcard_keyword_emits_escaped_space_wildcard() { - // I3-k: name="foo *baz" - assertEquals("foo\\ *baz", phrase("foo *baz").toQueryString(KEYWORD)); + void text_unquoted_wildcard_keeps_operator() { + assertEquals("foo*", bare("foo*").toQueryString(TEXT)); } @Test - void phrase_question_wildcard_keyword_emits_escaped_space_wildcard() { - // Q5-k: name="foo b?r" - assertEquals("foo\\ b?r", phrase("foo b?r").toQueryString(KEYWORD)); + 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 phrase_with_special_chars_and_wildcard_keyword_escapes_all() { - // D3-k repro: name="POST /test-logs/_search*" → PrefixQuery, special chars still escaped. - assertEquals( - "POST\\ \\/test\\-logs\\/_search*", - phrase("POST /test-logs/_search*").toQueryString(KEYWORD)); + 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)); } // ------------------------------------------------------------------------- - // Phrase with unescaped wildcard on text / match_only_text / unknown: keep - // legacy phrase form (no regression on text; wildcard silently ignored by - // Lucene inside phrase, same as today). + // KEYWORD FAMILY: quoting is irrelevant; wildcard presence selects the form. // ------------------------------------------------------------------------- @Test - void phrase_wildcard_text_keeps_phrase_form() { - assertEquals("\"foo bar*\"", phrase("foo bar*").toQueryString(TEXT)); + void keyword_no_wildcard_emits_exact_term_as_phrase() { + assertEquals("\"foo bar\"", q("foo bar").toQueryString(KEYWORD)); + assertEquals("\"foo\"", bare("foo").toQueryString(KEYWORD)); } @Test - void phrase_wildcard_match_only_text_keeps_phrase_form() { - assertEquals("\"foo bar*\"", phrase("foo bar*").toQueryString(MATCH_ONLY_TEXT)); + void keyword_no_wildcard_quoting_is_irrelevant() { + assertEquals(q("foo-bar").toQueryString(KEYWORD), bare("foo-bar").toQueryString(KEYWORD)); } @Test - void phrase_wildcard_unknown_type_keeps_phrase_form() { - // Unknown → treat as text-like so we never regress an unresolvable field. - assertEquals("\"foo bar*\"", phrase("foo bar*").toQueryString((ExprType) null)); + 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)); } - // ------------------------------------------------------------------------- - // Phrase with escaped wildcard: user asked for a LITERAL '*'/'?' — keep the - // phrase form even on keyword. The isPhrase flag was set for a reason. - // ------------------------------------------------------------------------- + @Test + void keyword_wildcard_without_space_emits_bare_pattern() { + assertEquals("foo*", q("foo*").toQueryString(KEYWORD)); + assertEquals("foo*", bare("foo*").toQueryString(KEYWORD)); + } @Test - void phrase_escaped_wildcard_keyword_keeps_phrase_form() { - // Input string literally contains `foo \*` (backslash + '*'). hasUnescapedWildcard() sees the - // backslash and skips '*', so we treat this as a phrase with a literal '*' — no emission - // change. QueryStringUtils.escapeLuceneSpecialCharacters keeps '\\' and '*' untouched. - assertEquals("\"foo \\*\"", phrase("foo \\*").toQueryString(KEYWORD)); + 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)); } - // ------------------------------------------------------------------------- - // Non-phrase (no space): field type does not matter — legacy code path. - // ------------------------------------------------------------------------- + @Test + void keyword_wildcard_with_special_chars_escapes_all() { + assertEquals( + "POST\\ \\/test\\-logs\\/_search*", q("POST /test-logs/_search*").toQueryString(KEYWORD)); + } @Test - void bare_wildcard_keyword_emits_unquoted() { - // P1: name="foo*" — never a phrase, works today. - assertEquals("foo*", bare("foo*").toQueryString(KEYWORD)); + 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 bare_wildcard_text_emits_unquoted() { - assertEquals("foo*", bare("foo*").toQueryString(TEXT)); + void constant_keyword_behaves_like_keyword() { + assertEquals("\"foo bar\"", q("foo bar").toQueryString(CONSTANT_KEYWORD)); + assertEquals("foo\\ bar*", q("foo bar*").toQueryString(CONSTANT_KEYWORD)); } // ------------------------------------------------------------------------- - // Backwards-compat: arg-less toQueryString() must match the null-type branch - // (legacy phrase form) so callers that never wire the resolver see no change. + // 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 = phrase("foo bar*"); + SearchLiteral lit = q("foo bar*"); assertEquals(lit.toQueryString((ExprType) null), lit.toQueryString()); } @@ -166,38 +197,32 @@ void argless_toQueryString_matches_null_type_branch() { // 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_resolves_field_type_and_emits_wildcard_on_keyword() { - // name="foo bar*" on a keyword field → name:foo\ bar* - SearchComparison cmp = - new SearchComparison( - new Field(new QualifiedName("name"), List.of()), - SearchComparison.Operator.EQUALS, - phrase("foo bar*")); + void comparison_emits_wildcard_pattern_on_keyword() { Function resolver = Map.of("name", KEYWORD)::get; - assertEquals("name:foo\\ bar*", cmp.toQueryString(resolver)); + 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_resolves_field_type_and_keeps_phrase_on_text() { - SearchComparison cmp = - new SearchComparison( - new Field(new QualifiedName("name"), List.of()), - SearchComparison.Operator.EQUALS, - phrase("foo bar*")); - Function resolver = Map.of("name", TEXT)::get; - assertEquals("name:\"foo bar*\"", cmp.toQueryString(resolver)); + 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_phrase_form() { - SearchComparison cmp = - new SearchComparison( - new Field(new QualifiedName("unmapped"), List.of()), - SearchComparison.Operator.EQUALS, - phrase("foo bar*")); - // Resolver returns null for unknown fields. + void comparison_unknown_field_falls_back_to_legacy_form() { Function resolver = f -> null; - assertEquals("unmapped:\"foo bar*\"", cmp.toQueryString(resolver)); + 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 0d227280f72..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,6 +5,8 @@ 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; @@ -19,6 +21,35 @@ 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(); @@ -27,11 +58,13 @@ public void init() throws Exception { } private void setupSpecIndices() throws IOException { - createSpecIndex(IDX_KEYWORD, "keyword"); - createSpecIndex(IDX_TEXT, "text"); + 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) throws IOException { + private void createSpecIndex(String indexName, String fieldType, String docs) throws IOException { try { client().performRequest(new Request("DELETE", "/" + indexName)); } catch (ResponseException ignore) { @@ -53,18 +86,7 @@ private void createSpecIndex(String indexName, String fieldType) throws IOExcept client().performRequest(createIndex); Request bulk = new Request("POST", "/" + indexName + "/_bulk?refresh=true"); - bulk.setJsonEntity( - "{\"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"); + bulk.setJsonEntity(docs); client().performRequest(bulk); } @@ -130,10 +152,9 @@ public void testGroup2_3_keyword_hyphen() throws IOException { @Test public void testGroup2_3_text_hyphen() throws IOException { - // Emitted: name:foo\-bar (unquoted, - escaped so parser treats as literal). - // Analyzer tokenizes to [foo, bar]; boolean OR matches 7 docs whose analyzed tokens - // contain foo or bar. - verifyNumOfRows(search(IDX_TEXT, "name=\"foo-bar\""), 7); + // 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 @@ -143,9 +164,8 @@ public void testGroup2_4_keyword_slash() throws IOException { @Test public void testGroup2_4_text_slash() throws IOException { - // Emitted: name:foo\/bar (unquoted, / escaped). Analyzer tokenizes to [foo, bar]; - // boolean OR matches 7 docs. - verifyNumOfRows(search(IDX_TEXT, "name=\"foo/bar\""), 7); + // Quoted with no wildcard -> phrase over [foo, bar]. + verifyNumOfRows(search(IDX_TEXT, "name=\"foo/bar\""), 4); } @Test @@ -155,9 +175,9 @@ public void testGroup2_5_keyword_at() throws IOException { @Test public void testGroup2_5_text_at() throws IOException { - // Emitted: name:foo@bar (unquoted; @ is not a Lucene special). Analyzer tokenizes - // to [foo, bar]; boolean OR matches 7 docs. - verifyNumOfRows(search(IDX_TEXT, "name=\"foo@bar\""), 7); + // 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 @@ -383,4 +403,32 @@ public void testGroup6_5_keyword_foo_space_bqmarkr() throws IOException { 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); From af99e7a122233f81dd32b532945f282b36077010 Mon Sep 17 00:00:00 2001 From: Peng Huo Date: Mon, 17 Aug 2026 10:00:59 -0700 Subject: [PATCH 3/3] Leave keyword emission untouched when the value has no wildcard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit rewrote non-wildcard keyword values from `field:value` to `field:"value"`. Both forms resolve to the same Lucene TermQuery — the keyword analyzer is a no-op, so Lucene returns from createFieldQuery at `numTokens == 1` before the quoted flag is read — but roughly a dozen expected-plan fixtures compare the emitted query_string as a string, and they broke. CalcitePPLBig5IT.sort_keyword_can_match_shortcut was the first to fail in CI on `process.name=kernel`. The rewrite was cosmetic. It was there to state "quoting is irrelevant on keyword" in code, but it fixed nothing: `x=y` and `a>b` already match correctly unquoted in term position, so there was no escaping gap to close either. Narrow the keyword branch to the case that is actually broken — a value holding an unescaped wildcard, which must reach Lucene as a single term for the pattern to apply to the whole stored value. Everything else on keyword falls through to the legacy branch and is byte-identical to before. The point about quoting being irrelevant now lives in the javadoc, as the reason not to rewrite the emission rather than something enforced by rewriting it. Text-side behavior is unchanged: a quoted value the analyzer splits is still a phrase rather than an OR over its tokens. No expected-output fixture is modified. Verified with CalcitePPLBig5IT, CalciteExplainIT, CalciteSearchCommandIT, :core:test, :ppl:test and doctest. Signed-off-by: Peng Huo --- .../sql/ast/expression/SearchLiteral.java | 28 +++++++++++-------- .../sql/ast/expression/SearchLiteralTest.java | 8 ++++-- 2 files changed, 22 insertions(+), 14 deletions(-) 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 25a2cf0790e..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 @@ -42,16 +42,16 @@ public String toQueryString(Function fieldTypeResolver) { * 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. - *
  • keyword family — quoting is irrelevant, because the analyzer is a no-op and a - * quoted phrase resolves to the same single term as a bare one. Emit whole-value semantics - * instead: a wildcard pattern when the value holds an unescaped wildcard, otherwise an - * exact term. Whitespace is escaped in the wildcard form so query_string keeps the value as - * one clause rather than splitting at the space and dropping the field binding. - *
  • anything else (date, numeric, ip, boolean, or unresolved) — legacy behavior, - * untouched. Those mappings do their own value parsing and are out of scope here. + *
  • 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 @@ -70,6 +70,13 @@ public String toQueryString(ExprType indexType) { if (val instanceof String) { String str = (String) val; + // 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 @@ -80,11 +87,8 @@ public String toQueryString(ExprType indexType) { return userQuoted && !wildcardTerm ? quoted(str) : unquoted(str); } - if (isKeywordLike(indexType)) { - return hasUnescapedWildcard(str) ? unquoted(str).replace(" ", "\\ ") : quoted(str); - } - - // Other mappings (date, numeric, ip, boolean) and unresolved fields: legacy behavior. + // 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); } } 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 index 904fc86deb8..c722916f339 100644 --- a/core/src/test/java/org/opensearch/sql/ast/expression/SearchLiteralTest.java +++ b/core/src/test/java/org/opensearch/sql/ast/expression/SearchLiteralTest.java @@ -118,9 +118,13 @@ void text_quoted_value_the_analyzer_splits_emits_phrase() { // ------------------------------------------------------------------------- @Test - void keyword_no_wildcard_emits_exact_term_as_phrase() { + 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)); - assertEquals("\"foo\"", bare("foo").toQueryString(KEYWORD)); } @Test