Skip to content

Honor quotes on unfielded PPL search terms without whitespace - #5872

Open
randym13 wants to merge 1 commit into
opensearch-project:mainfrom
randym13:fix/search-unfielded-quoted-phrase
Open

randym13 wants to merge 1 commit into
opensearch-project:mainfrom
randym13:fix/search-unfielded-quoted-phrase

Conversation

@randym13

Copy link
Copy Markdown

Description

A quoted free-text term in the search command was matched as an OR of its tokens whenever the value had no spaces.

source=logs "axya3-xya3-bahh-aaa"

returned every document containing any one of axya3, xya3, bahh or aaa, instead of only documents containing the whole value.

Cause

#5697 made emission depend on the field's index mapping and on whether the user quoted the value, which fixed this over-match for fielded comparisons on text fields (body="foo=bar"). Unfielded literals have no field type, so SearchLiteral.toQueryString(Function) sent them to the legacy branch, which quotes only when the value contains a space. A quoted value without spaces went out as a bare term, the analyzer split it, and default_operator=OR joined the pieces.

Change

SearchLiteral.toQueryString(Function) now emits a phrase for any user-quoted unfielded string. It keeps the same exception the text branch already has: a whitespace-free value with an unescaped wildcard stays unquoted, so "log_*" still works as a pattern.

An unfielded term is matched against index.query.default_field, which normally includes analyzed text, so honoring the quotes here follows the same reasoning as the text branch. On fields with a no-op analyzer a quoted phrase and a bare term resolve to the same single term, so those are unaffected.

Emitted query_string for unfielded literals, before and after:

PPL term Before After
"foo-bar" foo\-bar "foo\-bar"
"foo=bar" foo=bar "foo=bar"
"user@example.com" user@example.com "user@example.com"
"error" error "error"
"foo bar" "foo bar" "foo bar"
"foo*" foo* foo*
foo-bar (unquoted) foo\-bar foo\-bar

Fielded comparisons and IN lists are untouched; they call toQueryString(ExprType) directly.

This also changes the v2 engine's output for quoted unfielded terms, because it reaches emission through the same method.

Testing

  • Added unfielded cases to SearchLiteralTest: values the analyzer splits, values with whitespace, wildcard terms, escaped wildcards, unquoted terms, and a numeric literal.
  • Updated AstSearchExpressionTest.testSimpleSearchTerm, which expected search "error" to emit error. Its comment already read "Double quotes are preserved for phrase search".
  • :core:test, :ppl:test, :opensearch:test and :api:test pass locally (9,515 tests), along with spotlessCheck for core and ppl.
  • Verified against a live cluster: applied the same change to the 3.9.0.0 tag, replaced the plugin's core jar in an OpenSearch 3.9.0 container, and ran the query from the issue against a text field. It returned 1 of 4 documents, where the stock 3.9.0 plugin returned 3. Quoted single words, quoted wildcards, quoted phrases with spaces, and fielded comparisons returned the same rows on both.
  • I did not run the integration tests locally. SearchCommandIT and CalciteCrossClusterSearchIT contain quoted free-text terms without spaces ("@example.com", "email", "user", "john.doe+newsletter@company.com", "Hattie"); from reading them I expect the asserted rows to be unchanged.

Not addressed

The unquoted form search foo-bar still matches either token. The docs list hyphenated values as valid unquoted terms, so this may be worth a follow-up, but without quotes there is no explicit request for a phrase and I left it alone.

Related Issues

Resolves #5871

Follow-up to #5697 / #5682.

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • New PPL command checklist all confirmed.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff or -s.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

A quoted free-text term in the search command, such as
`search "foo-bar" source=logs`, was emitted to query_string without its
quotes unless the value contained a space. The analyzer then split the
value into tokens and default_operator=OR matched any one of them.

The same over-match was fixed for fielded text comparisons in opensearch-project#5697, but
unfielded literals were left on the legacy whitespace check. Emit a
phrase for any user-quoted unfielded string, keeping the existing
exception for a whitespace-free value with an unescaped wildcard.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Randy Miller <randym13@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Check for all whitespace types

The condition checks for space using str.contains(" ") but should check for any
whitespace characters (tabs, newlines, etc.). Use str.trim().length() ==
str.length() or a regex pattern to detect all whitespace types, ensuring consistent
behavior with the documented "whitespace-free" logic.

core/src/main/java/org/opensearch/sql/ast/expression/SearchLiteral.java [40-50]

 if (userQuoted
     && literal instanceof Literal
     && ((Literal) literal).getValue() instanceof String) {
   String str = (String) ((Literal) literal).getValue();
   // Same exception as on text: a whitespace-free wildcard value stays unquoted so `*` and `?`
   // keep their operator meaning.
-  boolean wildcardTerm = hasUnescapedWildcard(str) && !str.contains(" ");
+  boolean wildcardTerm = hasUnescapedWildcard(str) && !str.matches(".*\\s.*");
   if (!wildcardTerm) {
     return quoted(str);
   }
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that str.contains(" ") only checks for space characters and not other whitespace types like tabs or newlines. Using a regex pattern to detect all whitespace types would make the implementation more robust and consistent with the "whitespace-free" terminology used in comments and documentation.

Medium

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] PPL search command ignores quotes on a free-text term without spaces, so it matches any token

1 participant