Skip to content

Add prefix and wildcard query support to DSL query executor#22525

Draft
ask-kamal-nayan wants to merge 8 commits into
opensearch-project:mainfrom
ask-kamal-nayan:prefix-wildcard-dsl-support
Draft

Add prefix and wildcard query support to DSL query executor#22525
ask-kamal-nayan wants to merge 8 commits into
opensearch-project:mainfrom
ask-kamal-nayan:prefix-wildcard-dsl-support

Conversation

@ask-kamal-nayan

@ask-kamal-nayan ask-kamal-nayan commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Description

Adds DSL-to-Calcite translation for prefix and wildcard queries in the sandbox dsl-query-executor plugin.

This PR supersedes #21362 and carries forward @abhishek00159's original implementation rebased onto current main with a correctness fix for wildcard backslash-escape handling and hardened tests.

Summary

Implements PrefixQueryTranslator and WildcardQueryTranslator that convert OpenSearch prefix and wildcard queries to Calcite LIKE expressions with full support for case-insensitive matching and proper SQL special character escaping.


Features

Prefix Query

Converts prefix queries to SQL LIKE with trailing wildcard:

{"prefix": {"name": "lap"}}
// Converts to: name LIKE 'lap%'

{"prefix": {"name": {"value": "LAP", "case_insensitive": true}}}
// Converts to: LOWER(name) LIKE 'lap%'

Supported parameters:

  • value — The prefix string
  • case_insensitive — Case-insensitive matching (default: false)

Unsupported parameters (throw ConversionException):

  • boost — Query boosting not supported
  • rewrite — Rewrite method not supported

Wildcard Query

Converts wildcard patterns to SQL LIKE with pattern translation:

{"wildcard": {"name": "lap*"}}
// Converts to: name LIKE 'lap%'

{"wildcard": {"name": "l?ptop"}}
// Converts to: name LIKE 'l_ptop'

{"wildcard": {"name": {"value": "BOOK*", "case_insensitive": true}}}
// Converts to: LOWER(name) LIKE 'book%'

Wildcard characters:

Input LIKE output Meaning
* % match any character sequence
? _ match any single character
\* * literal asterisk
\? ? literal question mark
\\ escaped \ literal backslash
trailing lone \ escaped \ literal backslash (legacy parity)
% / _ in value escaped user data, not LIKE metacharacters

Supported parameters:

  • value — The wildcard pattern
  • case_insensitive — Case-insensitive matching (default: false)

Unsupported parameters (throw ConversionException):

  • boost — Query boosting not supported
  • rewrite — Rewrite method not supported

Special Character Escaping

Both translators properly escape SQL LIKE special characters in user data:

  • %\% (prevents unintended SQL wildcard)
  • _\_ (prevents unintended SQL single-char match)
  • \\\ (escape character itself)

Example:

{"prefix": {"path": "C:\\test_"}}
// Converts to: path LIKE 'C:\\test\_%'

{"wildcard": {"name": "a\\*b_c*"}}
// Converts to: name LIKE 'a*b\_c%'

Case-Insensitive Support

When case_insensitive: true:

  • Applies LOWER() function to the field reference
  • Converts the pattern to lowercase
  • Works for both prefix and wildcard queries

Wildcard Escape Fix (new in this PR)

The original implementation escaped \ without lookahead, so \* became \\% — an escaped backslash followed by a match-anything wildcard — instead of a literal *. The converter is now a single-pass state machine matching WildcardQueryBuilder semantics correctly. This was verified TDD-first: 6 red tests confirmed the bug before the fix, then went green after.


Examples

Prefix in Bool Query:

{
  "bool": {
    "must": [{"prefix": {"category": "electron"}}],
    "should": [
      {"prefix": {"brand": "sam"}},
      {"prefix": {"brand": "app"}}
    ]
  }
}

Converts to: (category LIKE 'electron%') AND ((brand LIKE 'sam%') OR (brand LIKE 'app%'))

Complex Wildcard Pattern:

{"wildcard": {"sku": "*-2021-*"}}

Converts to: sku LIKE '%-2021-%'


Testing

Unit Tests:

  • PrefixQueryTranslator tests — basic patterns, case sensitivity, escaping, edge cases, error handling
  • WildcardQueryTranslator tests — pattern translation, escape sequences (\*, \?, \\), mixed patterns, error handling
  • Full module suite: 178 tests, 0 failures

Integration Tests:

  • DslPrefixQueryIT — end-to-end validation with real index data
  • DslWildcardQueryIT — end-to-end validation with real index data
  • Parked with @AwaitsFix pending analytics engine E2E pipeline wiring (consistent with all sibling ITs in the module)

Manual testing (from original PR):

Prefix:

curl -X GET "localhost:9200/parquet_test/_search" -H "Content-Type: application/json" -d '
{
  "query": {
    "prefix": {
      "name": {
        "value": "ali"
      }
    }
  }
}'
[2026-04-28T18:37:55,451][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] row[0]=[30, seattle, alice, 95.5]
curl -X GET "localhost:9200/parquet_test/_search" -H "Content-Type: application/json" -d '
{
  "query": {
    "prefix": {
      "city": {
        "value": "po"
      }
    }
  }
}'
[2026-04-28T18:43:34,574][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] Query result rowCount=2
[2026-04-28T18:43:34,574][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] row[0]=[25, portland, bob, 88.0]
[2026-04-28T18:43:34,574][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] row[1]=[28, portland, dave, 76.8]

Wildcard:

curl -X GET "localhost:9200/parquet_test/_search" -H "Content-Type: application/json" -d '
{
  "query": {
    "wildcard": {
      "name": {
        "value": "*a*"
      }
    }
  }
}'
[2026-04-28T18:45:30,598][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] Query result rowCount=3
[2026-04-28T18:45:30,598][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] row[0]=[30, seattle, alice, 95.5]
[2026-04-28T18:45:30,598][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] row[1]=[35, seattle, carol, 92.3]
[2026-04-28T18:45:30,598][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] row[2]=[28, portland, dave, 76.8]
curl -X GET "localhost:9200/parquet_test/_search" -H "Content-Type: application/json" -d '
{
  "query": {
    "wildcard": {
      "name": {
        "value": "A*",
        "case_insensitive": true
      }
    }
  }
}'
[2026-04-28T18:46:31,555][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] Query result rowCount=1
[2026-04-28T18:46:31,555][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] row[0]=[30, seattle, alice, 95.5]
curl -X GET "localhost:9200/parquet_test/_search" -H "Content-Type: application/json" -d '
{
  "query": {
    "wildcard": {
      "name": {
        "value": "d?ve"
      }
    }
  }
}'
[2026-04-28T18:47:28,792][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] Query result rowCount=1
[2026-04-28T18:47:28,792][INFO ][o.o.d.e.DslQueryPlanExecutor] [runTask-0] row[0]=[28, portland, dave, 76.8]

Known Gaps (deliberate, consistent with sibling translators)

  • boost and rewrite are rejected rather than emulated
  • ITs parked pending E2E pipeline wiring (fragment conversion + shard execution + Arrow Flight drain)

Related Issues

Check List

  • Functionality includes testing
  • Commits are signed per the DCO using --signoff

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.

Abhishek Som and others added 3 commits July 21, 2026 09:30
…ueries

(cherry picked from commit 68b4363)
Signed-off-by: Kamal Nayan <askkamal@amazon.com>
Signed-off-by: Kamal Nayan <askkamal@amazon.com>
Signed-off-by: Kamal Nayan <askkamal@amazon.com>
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit f165637)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes

Sub-PR theme: Add prefix query translator

Relevant files:

  • sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/PrefixQueryTranslator.java
  • sandbox/plugins/dsl-query-executor/src/internalClusterTest/java/org/opensearch/dsl/DslPrefixQueryIT.java
  • sandbox/plugins/dsl-query-executor/src/test/java/org/opensearch/dsl/query/PrefixQueryTranslatorTests.java
  • sandbox/plugins/dsl-query-executor/src/test/resources/golden/prefix_query_hits.json
  • sandbox/plugins/dsl-query-executor/src/test/resources/golden/prefix_query_case_insensitive_hits.json

Sub-PR theme: Add wildcard query translator

Relevant files:

  • sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/WildcardQueryTranslator.java
  • sandbox/plugins/dsl-query-executor/src/internalClusterTest/java/org/opensearch/dsl/DslWildcardQueryIT.java
  • sandbox/plugins/dsl-query-executor/src/test/java/org/opensearch/dsl/query/WildcardQueryTranslatorTests.java
  • sandbox/plugins/dsl-query-executor/src/test/resources/golden/wildcard_query_hits.json
  • sandbox/plugins/dsl-query-executor/src/test/resources/golden/wildcard_query_case_insensitive_hits.json
  • sandbox/plugins/dsl-query-executor/src/test/resources/golden/wildcard_query_escaped_hits.json

⚡ Recommended focus areas for review

Inconsistent Backslash Escaping

For an escaped non-special character like \n, when the next char is % or _, the code emits \\ followed by \ then the char, producing three backslashes plus the metachar. This looks incorrect: the intent seems to be a literal backslash followed by an escaped metachar, which should be \\\% (two chars: literal \ escaped as \\, then % escaped as \%). The current output \\ + \ + % = \\\% may be right by coincidence, but the logic path is unclear and not covered by a test. Consider adding a test for input like \% (Java "\\%") to verify the behavior and clarify the intent.

} else {
    // \<other> → literal backslash + character
    // The backslash is itself a LIKE escape char, so emit \\ for the literal backslash
    // then escape the character if it's a SQL LIKE metacharacter
    if (next == '%' || next == '_') {
        result.append("\\\\");
        result.append('\\');
    } else {
        result.append("\\\\");
    }
    result.append(next);
}
i++; // consume the next character
Null Prefix Value

prefixQuery.value() is not null-checked before being passed to escapeLikePattern. If a caller constructs a PrefixQueryBuilder with a null value (which the builder permits in some paths), this will throw NullPointerException rather than a ConversionException. Consider explicitly rejecting or handling null values consistently with the other validation errors.

String prefix = prefixQuery.value();
boolean caseInsensitive = prefixQuery.caseInsensitive();

// Validate field exists in schema
RelDataTypeField field = ctx.getRowType().getField(fieldName, false, false);
if (field == null) {
    throw new ConversionException("Field '" + fieldName + "' not found in schema");
}

// Create field reference
RexNode fieldRef = ctx.getRexBuilder().makeInputRef(field.getType(), field.getIndex());

// Apply LOWER() if case insensitive
if (caseInsensitive) {
    fieldRef = ctx.getRexBuilder().makeCall(SqlStdOperatorTable.LOWER, fieldRef);
    prefix = prefix.toLowerCase(Locale.ROOT);
}

// Create LIKE pattern: prefix + '%'
String likePattern = escapeLikePattern(prefix) + "%";

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Latest suggestions up to f165637

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Guard against null prefix value

prefixQuery.value() may return null if the query is constructed programmatically
without a value. Add a null check to avoid a NullPointerException in
escapeLikePattern or toLowerCase, and throw a clear ConversionException instead.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/PrefixQueryTranslator.java [88-90]

 String fieldName = prefixQuery.fieldName();
 String prefix = prefixQuery.value();
 boolean caseInsensitive = prefixQuery.caseInsensitive();
+if (prefix == null) {
+    throw new ConversionException("Prefix query value must not be null");
+}
Suggestion importance[1-10]: 4

__

Why: Adding a null check for prefix improves error messaging, but PrefixQueryBuilder typically validates the value at construction time, so this is a minor defensive improvement.

Low
Guard against null wildcard pattern

wildcardQuery.value() may return null; the subsequent calls to toLowerCase and
convertWildcardToLike will throw an unclear NullPointerException. Validate for null
and throw a ConversionException with a descriptive message.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/WildcardQueryTranslator.java [92-94]

 String fieldName = wildcardQuery.fieldName();
 String pattern = wildcardQuery.value();
 boolean caseInsensitive = wildcardQuery.caseInsensitive();
+if (pattern == null) {
+    throw new ConversionException("Wildcard query value must not be null");
+}
Suggestion importance[1-10]: 4

__

Why: Similar to the prefix case, adding a null check improves robustness slightly but WildcardQueryBuilder generally validates value input, making this a low-impact defensive change.

Low
General
Simplify backslash-plus-metachar escape logic

When the escaped character next is % or _, the code emits \ (literal backslash)
followed by </code> + the char. However, in a LIKE pattern with </code> as the escape char, the
sequence \% means: literal backslash, then escaped % (literal %). This is correct
semantically, but consider whether OpenSearch wildcard % should mean "literal
backslash + literal %" — verify this matches OpenSearch's actual escape semantics,
as OpenSearch typically only treats * and ? as escape sequences and leaves other
\x as literal \x.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/WildcardQueryTranslator.java [160-170]

 } else {
-    // \<other> → literal backslash + character
-    // The backslash is itself a LIKE escape char, so emit \\ for the literal backslash
-    // then escape the character if it's a SQL LIKE metacharacter
+    // \<other> → literal backslash + literal character
+    result.append("\\\\");
     if (next == '%' || next == '_') {
-        result.append("\\\\");
         result.append('\\');
-    } else {
-        result.append("\\\\");
     }
     result.append(next);
 }
Suggestion importance[1-10]: 3

__

Why: The suggestion is essentially a minor refactor that produces equivalent output; it doesn't fix a bug and only slightly cleans up branching.

Low
Reorder escape and lowercase for safety

When caseInsensitive is true, the prefix is lowercased before escaping. If the raw
prefix contains characters whose lowercase form differs in length or introduces LIKE
metacharacters, escaping order matters. Escape the pattern first, then lowercase, or
lowercase and then escape — but ensure the operation order is consistent and safe.
Currently lowercasing then escaping is fine, but consider applying toLowerCase after
escaping to avoid any locale-dependent character expansions affecting escape
sequences.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/PrefixQueryTranslator.java [102-108]

 // Apply LOWER() if case insensitive
 if (caseInsensitive) {
     fieldRef = ctx.getRexBuilder().makeCall(SqlStdOperatorTable.LOWER, fieldRef);
-    prefix = prefix.toLowerCase(Locale.ROOT);
 }
 
-// Create LIKE pattern: prefix + '%'
-String likePattern = escapeLikePattern(prefix) + "%";
+// Create LIKE pattern: escape then lowercase (if needed) + '%'
+String escaped = escapeLikePattern(prefix);
+if (caseInsensitive) {
+    escaped = escaped.toLowerCase(Locale.ROOT);
+}
+String likePattern = escaped + "%";
Suggestion importance[1-10]: 2

__

Why: The current order (lowercase then escape) is safe for ASCII LIKE metacharacters (%, _, \), which don't change under toLowerCase. The suggestion is speculative and provides only marginal theoretical benefit.

Low

Previous suggestions

Suggestions up to commit 84e30fc
CategorySuggestion                                                                                                                                    Impact
General
Simplify and verify backslash-metachar escaping logic

When the character after a backslash is a SQL LIKE metacharacter (% or _), the
current code emits \ followed by </code> and then the metacharacter, producing 4 chars
for % input (e.g. \%). Under LIKE with escape </code>, \ is a literal backslash and %
is a literal %, which is correct, but the intermediate lone </code> before appending next
results in \ + </code> + % = \% — verify this matches the test expectation a\%b in
testMixedEscaping, since a % input should yield literal backslash + literal % =
\%, not \%.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/WildcardQueryTranslator.java [160-171]

 } else {
     // \<other> → literal backslash + character
-    // The backslash is itself a LIKE escape char, so emit \\ for the literal backslash
-    // then escape the character if it's a SQL LIKE metacharacter
+    // Emit \\ for the literal backslash, then escape SQL LIKE metacharacter if needed
+    result.append("\\\\");
     if (next == '%' || next == '_') {
-        result.append("\\\\");
         result.append('\\');
-    } else {
-        result.append("\\\\");
     }
     result.append(next);
 }
Suggestion importance[1-10]: 4

__

Why: The suggestion questions the correctness of the escaping logic but the original code appears functionally equivalent to the proposed simplification; it's more of a refactor for clarity than a bug fix.

Low
Possible issue
Guard against null prefix value

prefixQuery.value() may return null if the query was built without a value; the
subsequent call to escapeLikePattern(prefix) would then throw NullPointerException.
Add a null check and throw a ConversionException with a clear message, or treat null
as empty string consistently.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/PrefixQueryTranslator.java [88-90]

 String fieldName = prefixQuery.fieldName();
 String prefix = prefixQuery.value();
+if (prefix == null) {
+    throw new ConversionException("Prefix query 'value' must not be null for field '" + fieldName + "'");
+}
 boolean caseInsensitive = prefixQuery.caseInsensitive();
Suggestion importance[1-10]: 4

__

Why: Adding a null check is a defensive improvement, but PrefixQueryBuilder typically validates non-null value at construction; the impact is minor.

Low
Guard against null wildcard pattern

wildcardQuery.value() can be null, which would cause a NullPointerException in
convertWildcardToLike when iterating over its characters. Add an explicit null check
to throw a ConversionException with an informative message rather than surfacing an
NPE to callers.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/WildcardQueryTranslator.java [92-94]

 String fieldName = wildcardQuery.fieldName();
 String pattern = wildcardQuery.value();
+if (pattern == null) {
+    throw new ConversionException("Wildcard query 'value' must not be null for field '" + fieldName + "'");
+}
 boolean caseInsensitive = wildcardQuery.caseInsensitive();
Suggestion importance[1-10]: 4

__

Why: Similar to the prefix suggestion, a null check adds defensive validation but WildcardQueryBuilder typically enforces non-null values; low impact.

Low
Suggestions up to commit ac8a7be
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix malformed LIKE escape sequence

When the next character is a SQL LIKE metacharacter (% or _), the produced pattern
is \ + </code> + char, which under the escape character </code> means: escaped backslash
(literal </code>), then a stray escape </code> followed by the metachar. This is
malformed/ambiguous. Emit the literal backslash escape and then the escaped metachar
as separate well-formed sequences: \ followed by % (or _).

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/WildcardQueryTranslator.java [160-171]

 } else {
     // \<other> → literal backslash + character
-    // The backslash is itself a LIKE escape char, so emit \\ for the literal backslash
-    // then escape the character if it's a SQL LIKE metacharacter
+    // Emit \\ for the literal backslash, then properly escape if metachar
+    result.append("\\\\");
     if (next == '%' || next == '_') {
-        result.append("\\\\");
-        result.append('\\');
+        result.append('\\').append(next);
     } else {
-        result.append("\\\\");
+        result.append(next);
     }
-    result.append(next);
 }
Suggestion importance[1-10]: 6

__

Why: The suggestion identifies a subtle but valid concern about the escape sequence order in the malformed edge case (\% or \_ in input). The improved code produces a cleaner, well-formed pattern, though the practical impact is limited to unusual inputs.

Low
Safely convert prefix value to String

PrefixQueryBuilder.value() returns Object, not String. Direct assignment may cause a
compilation error or a ClassCastException at runtime when the value is not a String.
Explicitly convert the value to String (e.g., via String.valueOf(...)) or cast it
after a type check.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/PrefixQueryTranslator.java [89]

 String fieldName = prefixQuery.fieldName();
-String prefix = prefixQuery.value();
+String prefix = String.valueOf(prefixQuery.value());
 boolean caseInsensitive = prefixQuery.caseInsensitive();
Suggestion importance[1-10]: 3

__

Why: PrefixQueryBuilder.value() in OpenSearch actually returns String, so the direct assignment is safe. The suggestion is based on an incorrect assumption and provides marginal defensive value.

Low
Safely convert wildcard value to String

WildcardQueryBuilder.value() may return Object (or a value that is not guaranteed to
be a String). Assigning it directly to String risks a compilation issue or runtime
ClassCastException. Convert explicitly using String.valueOf(...) to guard against
non-String values.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/WildcardQueryTranslator.java [93]

 String fieldName = wildcardQuery.fieldName();
-String pattern = wildcardQuery.value();
+String pattern = String.valueOf(wildcardQuery.value());
 boolean caseInsensitive = wildcardQuery.caseInsensitive();
Suggestion importance[1-10]: 3

__

Why: WildcardQueryBuilder.value() returns String in OpenSearch, so the direct assignment is safe. The suggestion adds unnecessary defensive conversion based on an incorrect premise.

Low
Suggestions up to commit f3cffa8
CategorySuggestion                                                                                                                                    Impact
General
Preserve literal backslash before escaped char

When the escaped character is a SQL LIKE metacharacter like % or _, the preceding
literal backslash from the input is silently dropped. To preserve semantics (input
% should represent a literal % while also preserving the original backslash as a
literal), the backslash itself should also be escaped for LIKE, not just the
metacharacter.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/WildcardQueryTranslator.java [156-163]

 } else {
-    // \<other> → the escaped character literally
-    // Escape it if it's a SQL LIKE metacharacter
+    // \<other> → literal backslash followed by the character
+    result.append("\\\\");
     if (next == '%' || next == '_') {
         result.append('\\');
     }
     result.append(next);
 }
Suggestion importance[1-10]: 4

__

Why: The suggestion raises a semantic ambiguity in how OpenSearch wildcard escaping of non-special chars is handled. However, the current behavior (treating \x as literal x) is consistent with typical wildcard escape semantics and matches the test testWildcardWithMixedEscaping. The proposed change would break existing tests, making it partially incorrect.

Low
Use Float.compare for boost check

Comparing floats with != can produce false positives due to precision. Since
AbstractQueryBuilder.DEFAULT_BOOST is 1.0f, this exact comparison is likely safe,
but using Float.compare is more robust against any future default changes or
serialization round-trips.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/PrefixQueryTranslator.java [80-82]

-if (prefixQuery.boost() != 1.0f) {
+if (Float.compare(prefixQuery.boost(), 1.0f) != 0) {
     throw new ConversionException("Prefix query parameter 'boost' is not supported");
 }
Suggestion importance[1-10]: 3

__

Why: Minor robustness improvement using Float.compare instead of !=. As the suggestion notes, the current comparison is likely safe since the default is a well-defined constant, so the impact is marginal.

Low
Suggestions up to commit 16b0cde
CategorySuggestion                                                                                                                                    Impact
Possible issue
Use locale-independent lowercase conversion

Using String.toLowerCase() without a locale is locale-sensitive and can yield
unexpected results (e.g., Turkish locale converts 'I' to 'ı'). Use Locale.ROOT to
ensure consistent, deterministic behavior matching SQL LOWER() semantics. The same
issue exists in WildcardQueryTranslator.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/PrefixQueryTranslator.java [99-102]

 // Apply LOWER() if case insensitive
 if (caseInsensitive) {
     fieldRef = ctx.getRexBuilder().makeCall(SqlStdOperatorTable.LOWER, fieldRef);
-    prefix = prefix.toLowerCase();
+    prefix = prefix.toLowerCase(java.util.Locale.ROOT);
 }
Suggestion importance[1-10]: 7

__

Why: Valid concern - toLowerCase() without a Locale can behave inconsistently across locales (e.g., Turkish locale), which could cause mismatches with SQL LOWER() semantics. Applies to both translators.

Medium
General
Preserve backslash for unknown escape sequences

A trailing lone backslash is emitted as \ (escaped backslash for LIKE), but earlier
in the switch a standalone \ (literal backslash) input path only exists via
lookahead — an isolated non-terminal backslash that is not followed by *, ?, or \
currently consumes the next character as a literal instead of treating the backslash
itself as literal. Consider clarifying/documenting that only *, ?, \ are
recognized escapes, or emit a literal backslash for unknown escape sequences to
avoid silently dropping the backslash.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/WildcardQueryTranslator.java [154-161]

 } else {
-    // Trailing lone backslash: treat as literal backslash
+    // Unknown escape: preserve backslash as literal, then emit next char
     result.append("\\\\");
+    if (next == '%' || next == '_') {
+        result.append('\\');
+    }
+    result.append(next);
 }
Suggestion importance[1-10]: 5

__

Why: Reasonable observation about escape semantics being potentially inconsistent with OpenSearch wildcard behavior, but the current behavior may match documented intent. Improvement is minor and semantic-dependent.

Low
Guard against null query value

prefixQuery.value() may return null if the query was constructed without a value,
which would cause a NullPointerException in escapeLikePattern or toLowerCase. Add a
null check and throw a ConversionException with a clear message, or treat null as
empty string consistently.

sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/PrefixQueryTranslator.java [85-87]

 String fieldName = prefixQuery.fieldName();
 String prefix = prefixQuery.value();
 boolean caseInsensitive = prefixQuery.caseInsensitive();
+if (prefix == null) {
+    throw new ConversionException("Prefix query 'value' must not be null");
+}
Suggestion importance[1-10]: 4

__

Why: Null-check for defensive programming. PrefixQueryBuilder typically validates the value at construction time, making a NPE unlikely in practice, but explicit handling improves robustness.

Low

@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 16b0cde: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Kamal Nayan <askkamal@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit f3cffa8

Signed-off-by: Kamal Nayan <askkamal@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for f3cffa8: SUCCESS

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.48%. Comparing base (0213338) to head (f3cffa8).

Additional details and impacted files
@@            Coverage Diff            @@
##               main   #22525   +/-   ##
=========================================
  Coverage     73.48%   73.48%           
- Complexity    76460    76468    +8     
=========================================
  Files          6104     6104           
  Lines        346582   346582           
  Branches      49879    49879           
=========================================
+ Hits         254675   254678    +3     
+ Misses        71664    71624   -40     
- Partials      20243    20280   +37     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit ac8a7be

Signed-off-by: Kamal Nayan <askkamal@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 84e30fc

Signed-off-by: Kamal Nayan <askkamal@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 84e30fc: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Kamal Nayan <askkamal@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit f165637

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for f165637: SUCCESS

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.

1 participant