Add DSL Regex Query Support.

[ask-kamal-nayan]

Description

Adds a translation-and-delegation path for DSL regexp queries in the analytics engine. Without this, a DSL regexp query crashes with IllegalStateException("Unrecognized filter operator") because ScalarFunction.fromSqlOperatorWithFallback returns null for UNRESOLVED_QUERY. The pattern is delegated verbatim to Lucene's RegexpQueryBuilder so runtime semantics match vanilla _search by construction — non-scoring filter context only.

What / Why / Where

Scope: translate → serialize → delegate DSL regexp to Lucene, filter context only.

Module	Role
sandbox/plugins/dsl-query-executor	Translator, golden fixtures, integration test
sandbox/plugins/analytics-backend-lucene	Serializer and capability wiring
sandbox/libs/analytics-framework	New ScalarFunction.REGEXP_QUERY enum constant

---

Supported queries

All four parameter variants with verbatim pattern passthrough:

{ "query": { "regexp": { "hostname": { "value": "web-[0-9]{2}\\.prod\\.example\\.com" } } } }

Lucene-only automaton syntax (<n-m> intervals, & intersection):

{ "query": { "regexp": { "sku": { "value": "A<100-999>&[A-Z].*", "flags": "INTERVAL|INTERSECTION" } } } }

Multiple optional parameters:

{ "query": { "regexp": { "email": { "value": "[a-z]+@example\\.(com|org)", "case_insensitive": true, "max_determinized_states": 20000, "rewrite": "constant_score_boolean" } } } }

Cross-engine bool/filter — regexp delegates to Lucene, range evaluates in DataFusion:

{ "query": { "bool": { "filter": [
  { "regexp": { "path": { "value": "/api/v[0-9]+/users/.*" } } },
  { "range":  { "status_code": { "gte": 400, "lt": 500 } } }
] } } }

---

Design

Verbatim delegation, not pattern translation — the pattern reaches RegexpQueryBuilder unchanged. A translation approach would reject or mistranslate four Lucene constructs with no RE2 equivalent (&, ~, <n-m>, <id>). By delegating, zero regex-dialect gaps.

New ScalarFunction.REGEXP_QUERY — the existing ScalarFunction.REGEXP routes to RegexpSerializer (PPL) which wraps patterns as .*pattern.* for substring-match semantics; reusing it would un-anchor every DSL pattern. A separate REGEXP_QUERY in Category.FULL_TEXT keeps the two paths isolated.

Emitted RexCall shape:

REGEXP_QUERY(
  MAP('field', $inputRef),
  MAP('query', '<pattern>'),
  [MAP('case_insensitive', 'true')],
  [MAP('flags', '<raw int>')],
  [MAP('max_determinized_states', '<n>')],
  [MAP('rewrite', '<method>')]
)

Optional params at operand index 2+, matching AbstractRelevanceSerializer.optionalParamsStartIndex(). Serializer uses ConversionUtils.extractRelevanceOperands and ConversionUtils.extractOptionalParams.

Flags as raw int bitmask — RegexpFlag.resolveValue("0") throws for flags=NONE (value 0), breaking a name-based round-trip. Raw int is lossless and mirrors vanilla's flags_value field.

Opaque delegated leaf — no Substrait function signature, no opensearch_scalar_functions.yaml entry, no DataFusion plugin change.

---

Execution path

1. SearchActionFilter reroutes _search to the analytics engine; RegexpQueryTranslator emits the REGEXP_QUERY RexCall.
2. OpenSearchFilterRule annotates the predicate; CapabilityRegistry finds REGEXP_QUERY in Lucene's FULL_TEXT_OPS × FULL_TEXT_TYPES — Lucene is the sole taker.
3. RegexpQueryDslSerializer rebuilds a RegexpQueryBuilder; the leaf becomes an opaque delegated predicate.
4. On the data node, Lucene compiles a RegexpQuery, intersects the automaton against the term dictionary, and produces a per-segment FixedBitSet that crosses to Rust via FFM → Parquet RowSelection.
5. Per Arrow batch the mask is AND-ed with any residual DataFusion predicate (cross-engine bitmap intersection for mixed bool/filter).

---

Parameter matrix

Parameter	Vanilla default	Handling
value (pattern)	— (required)	Passed verbatim to RegexpQueryBuilder
case_insensitive	false	Emitted only when true; omitted at default
flags	ALL	Emitted as raw int bitmask when non-default
max_determinized_states	10 000	Emitted when non-default
rewrite	null	Emitted when non-null
boost	1.0	Rejected — ConversionException (HTTP 500)
_name	null	Rejected — ConversionException (HTTP 500)

Rejection rationale: columnar path is filter-only (scoring has no semantics); matched_queries is never assembled.

---

Field-type support

Field type	Supported	Notes
keyword	Yes	Maps to VARCHAR; translator enforces VARCHAR gate
text	Yes	Maps to VARCHAR
match_only_text	Yes	Maps to VARCHAR
wildcard	No	Unstorable in engine mode — unreachable, not divergent
constant_keyword	No	Unstorable in engine mode — unreachable, not divergent
version	No	Unstorable in engine mode — unreachable, not divergent
flat_object	No	Unstorable in engine mode — unreachable, not divergent

Engine-mode limitation: Parquet rejects multi-valued keyword fields outright (Cannot accept multiple values for field: [tags] of type: [keyword]). Observed in E2E run; shared by every query type, not regexp-specific.

---

Gaps vs vanilla

• Zero regex-dialect gaps — pattern reaches RegexpQueryBuilder byte-for-byte unchanged; all Lucene automaton constructs (&, ~, <n-m>, <id>) work.
• search.allow_expensive_queries=false not honoured — Lucene backend injects a hardcoded always-true supplier; family-wide gap, not regexp-specific.
• No Parquet page pruning — delegated predicate contributes no min/max statistics; correctness preserved by residual re-evaluation.
• Non-scoring filter context — boost rejected; _name/matched_queries unavailable.
• Field types unstorable in engine mode — wildcard, constant_keyword, version, flat_object are unreachable rather than semantically divergent.
• Execution-time validation errors surface as HTTP 500 instead of 400 — parse-time validation is correct; errors thrown inside RegexpQueryBuilder.doToQuery on the data node propagate unmapped because LuceneScanInstructionHandler catches only IOException. Family-wide, not regexp-specific.
• Deprecation Warning headers raised on the data node never reach the client — HeaderWarning.addWarning attaches to the data node's ThreadContext, not the coordinating REST thread. The warning fires and is visible in the node log. Family-wide.

---

At parity (inherited free by delegating)

• index.max_regex_length — enforced by RegexpQueryBuilder.doToQuery before query construction.
• Keyword custom normalizers — applied via StringFieldType.regexpQuery → indexedValueForSearch.
• case_insensitive — passed to RegexpQueryBuilder.caseInsensitive; Lucene applies full Unicode case folding via CaseFolding.expand (the ASCII_CASE_INSENSITIVE constant name is legacy). Behaviour is vanilla's by construction.
• COMPLEMENT deprecation warning — still fires from RegexpQueryBuilder.doToQuery.
• failIfNotIndexed() — non-indexed field guard still applies.

---

Test evidence

Suite	Before	After	Failures
dsl-query-executor	140	154	0
analytics-backend-lucene	313	335	0

New: RegexpQueryTranslatorTests 14, RegexpQueryDslSerializerTests 19, 5 golden plan fixtures, +2 PPL-coexistence pinning tests that assert ScalarFunction.REGEXP still resolves to RegexpSerializer while REGEXP_QUERY resolves to RegexpQueryDslSerializer (the DSL path cannot silently reroute the PPL path). DslRegexpQueryIT is @AwaitsFix-parked (4 methods) because SearchResponseBuilder.build() returns empty hits repo-wide — not specific to this change.

Manual E2E validation (engine-mode cluster, not automated): confirmed full-string anchoring (substring pattern → zero hits); & intersection, <n-m> interval and flags semantics; all four optional parameters provably wired via falsifiability pairs (e.g. invalid rewrite → Failed to parse rewrite_method [not_a_method] from QueryParsers.parseRewriteMethod via RegexpQueryBuilder.doToQuery); OR-of-mixed-backends (INTERLEAVED) correctly includes the delegated regexp leaf, proven by three distinct result fingerprints for union / range-only / regexp-only. Differential-oracle comparison against vanilla was not possible — SearchActionFilter intercepts all _search unconditionally so a true vanilla index cannot be created with the plugin installed; semantic equivalence relies on the code-path argument (vanilla RegexpQueryBuilder constructs the query, so semantics are vanilla's by construction).

---

Out of scope / follow-ups

• RegexpSerializer (PPL) latent bug: unparenthesised .* wrap causes a|b → (.*a)|(b.*). Pre-existing; separate follow-up.
• SearchResponseBuilder empty-hits fix and page-pruning for delegated predicates — both family-wide.

Related Issues

Resolves #
Part of the DSL query-translator work for optimized engine mode.

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 01ddb04)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Silent Parameter Drop The default branch in applyParams silently ignores unrecognized parameter keys "for forward compatibility". Since the DSL translator only emits a known, closed set of keys, an unknown key here more likely indicates a bug (typo in translator, protocol mismatch) than a forward-compat scenario. Silently dropping it could cause queries to be executed with unintended semantics (e.g., a misspelled case_insensitive yielding a case-sensitive match without any error). Consider throwing on unknown params, or at least logging a warning. default -> { /* ignore unrecognized params for forward compatibility */ } } Possible Fixture Mismatch The golden fixture expects MAP('flags', '65568') for COMPLEMENT|INTERVAL. RegexpFlag.COMPLEMENT.value() is 0x10000 (65536) and INTERVAL.value() is 0x20 (32), so the OR is 65568 — consistent. However, verify this matches what RegexpQueryBuilder.parse actually produces when it consumes the string "COMPLEMENT|INTERVAL" from the DSL; if parsing yields a different int (e.g., due to deprecation handling of COMPLEMENT), the expected plan will drift from actual output. "LogicalFilter(condition=[REGEXP_QUERY(MAP('field', $0), MAP('query', 'lap.*'), MAP('flags', '65568'))])",

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 01ddb04

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Reject unrecognized parameters instead of ignoring Silently ignoring unrecognized parameters can mask typos and lead to queries with unintended semantics (e.g. a misspelled case_insensitiv would be dropped without warning). Consider throwing IllegalArgumentException on unknown keys so callers get immediate feedback, since the translator emits a fixed known set. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/RegexpQueryDslSerializer.java [58-59] case "max_determinized_states" -> { try { regexpQb.maxDeterminizedStates(Integer.parseInt(entry.getValue())); } catch (NumberFormatException e) { throw new IllegalArgumentException( "Invalid integer value for 'max_determinized_states': [" + entry.getValue() + "]", e ); } } case "rewrite" -> regexpQb.rewrite(entry.getValue()); -default -> { - /* ignore unrecognized params for forward compatibility */ } +default -> throw new IllegalArgumentException("Unrecognized parameter for regexp_query: [" + entry.getKey() + "]"); Suggestion importance[1-10]: 4 __ Why: The comment explicitly states the ignore is for "forward compatibility", indicating a deliberate design choice. Throwing on unknown keys could break forward compatibility as new params are added, though it would catch typos. Impact is moderate and debatable.	Low

---

Previous suggestions

Suggestions up to commit 637f6d7

Category	Suggestion	Impact
Possible issue	Guard against null field lookup ctx.getField(fieldName) may return null if the field is unknown, causing a NullPointerException instead of a clean ConversionException. The test testUnknownFieldThrows expects a ConversionException; add a null check to guarantee that contract regardless of ConversionContext implementation. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/RegexpQueryTranslator.java [64-66] String fieldName = regexpQuery.fieldName(); RelDataTypeField field = ctx.getField(fieldName); +if (field == null) { + throw new ConversionException("Unknown field [" + fieldName + "]"); +} if (field.getType().getSqlTypeName() != SqlTypeName.VARCHAR) { Suggestion importance[1-10]: 5 __ Why: Defensive null check improves robustness, though the existing test passes suggests ctx.getField likely throws ConversionException itself. Moderate value.	Low
General	Fail fast on unrecognized parameters Silently ignoring unrecognized params can mask serialization/versioning bugs and lead to queries that appear to accept parameters but ignore them. Consider logging a warning or throwing to surface misconfigurations early, since the emit side (RegexpQueryTranslator) only emits a fixed known set. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/RegexpQueryDslSerializer.java [47-60] case "max_determinized_states" -> { try { regexpQb.maxDeterminizedStates(Integer.parseInt(entry.getValue())); } catch (NumberFormatException e) { throw new IllegalArgumentException( "Invalid integer value for 'max_determinized_states': [" + entry.getValue() + "]", e ); } } case "rewrite" -> regexpQb.rewrite(entry.getValue()); -default -> { - /* ignore unrecognized params for forward compatibility */ } +default -> throw new IllegalArgumentException("Unrecognized parameter for regexp_query: [" + entry.getKey() + "]"); Suggestion importance[1-10]: 4 __ Why: Reasonable design trade-off, but the comment explicitly states the intent is forward compatibility. Changing to throw could break future versioning; moderate value.	Low
	Handle null parameter values defensively parseStrictBoolean throws IllegalArgumentException for malformed input, but Integer.parseInt in the other branches is wrapped to produce parameter-specific error messages. For consistency and to avoid throwing raw NPE when a param value is null, guard against null values before parsing. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/RegexpQueryDslSerializer.java [34-38] @Override protected void applyParams(QueryBuilder qb, Map<String, String> params) { RegexpQueryBuilder regexpQb = (RegexpQueryBuilder) qb; for (Map.Entry<String, String> entry : params.entrySet()) { + if (entry.getValue() == null) { + throw new IllegalArgumentException("Null value for regexp_query parameter '" + entry.getKey() + "'"); + } switch (entry.getKey()) { case "case_insensitive" -> regexpQb.caseInsensitive(parseStrictBoolean("case_insensitive", entry.getValue())); Suggestion importance[1-10]: 3 __ Why: Null values in the params map are unlikely given the emit side, but the defensive check adds minor robustness. Low-to-moderate impact.	Low

Suggestions up to commit c5f0a5f

Category	Suggestion	Impact
Possible issue	Null field lookup causes NPE instead of ConversionException ctx.getField(fieldName) may return null if the field does not exist in the schema, which would cause a NullPointerException rather than the expected ConversionException. The test testUnknownFieldThrows expects a ConversionException, but if getField returns null, a NullPointerException will be thrown instead. Add a null check before accessing field.getType(). sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/RegexpQueryTranslator.java [65-74] RelDataTypeField field = ctx.getField(fieldName); +if (field == null) { + throw new ConversionException("Unknown field: [" + fieldName + "]"); +} if (field.getType().getSqlTypeName() != SqlTypeName.VARCHAR) { throw new ConversionException( "Can only use regexp queries on keyword and text fields - not on [" + fieldName + "] which is of type [" + field.getType().getSqlTypeName() + "]" ); } Suggestion importance[1-10]: 7 __ Why: The test testUnknownFieldThrows expects a ConversionException when a nonexistent field is used, but if ctx.getField() returns null, a NullPointerException would be thrown instead. Adding a null check ensures the correct exception type is thrown and improves robustness.	Medium
	Verify default flags sentinel value correctness The condition regexpQuery.flags() != RegexpQueryBuilder.DEFAULT_FLAGS_VALUE will not emit the flags operand when the user explicitly sets flags to the default value. However, the golden test file regexp_query_flags_hits.json shows "flags": "COMPLEMENT|INTERVAL" mapping to 65568, which suggests the default value check is correct. The real risk is if DEFAULT_FLAGS_VALUE is -1 (all flags), which is the OpenSearch default — in that case, a user explicitly passing RegexpFlag.ALL would have their flags silently dropped. Verify that DEFAULT_FLAGS_VALUE matches the sentinel used by RegexpQueryBuilder and that the omission of default-valued flags is intentional and safe. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/RegexpQueryTranslator.java [86-90] if (regexpQuery.flags() != RegexpQueryBuilder.DEFAULT_FLAGS_VALUE) { - // Carry the raw int bitmask — lossless by construction. Matches the vanilla - // flags_value JSON field parsed by RegexpQueryBuilder.fromXContent. operands.add(makeMapLiteralOperand(rexBuilder, "flags", String.valueOf(regexpQuery.flags()))); } Suggestion importance[1-10]: 4 __ Why: The suggestion asks to verify that DEFAULT_FLAGS_VALUE is the correct sentinel, but the improved_code is essentially the same as existing_code (just with the comment removed). This is a verification suggestion rather than a concrete fix, and the existing code appears correct based on the test coverage present in the PR.	Low
General	Unknown parameters silently ignored instead of rejected Silently ignoring unrecognized parameters can mask typos in parameter names (e.g., case_insenstive instead of case_insensitive), leading to queries that silently ignore user intent. Consider logging a warning or throwing an IllegalArgumentException for unrecognized parameters, similar to how other serializers handle this. At minimum, a debug-level log would help diagnose misconfigured queries. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/RegexpQueryDslSerializer.java [58-59] -default -> { - /* ignore unrecognized params for forward compatibility */ } +default -> throw new IllegalArgumentException( + "Unknown parameter '" + entry.getKey() + "' for regexp_query; " + + "supported parameters: case_insensitive, flags, max_determinized_states, rewrite" +); Suggestion importance[1-10]: 5 __ Why: Silently ignoring unknown parameters can mask typos and misconfigurations. However, the existing code explicitly comments "forward compatibility" as the rationale, and throwing on unknown params would be a breaking change for future parameter additions. The suggestion is valid but conflicts with the stated design intent.	Low

Suggestions up to commit b38c169

Category	Suggestion	Impact
General	Handle unknown field with clear error ctx.getField(fieldName) may return null when the field is unknown, in which case dereferencing field.getType() will throw a NullPointerException instead of a meaningful ConversionException. The test testUnknownFieldThrows currently passes because of the NPE, but the intent is clearly to raise ConversionException. Add an explicit null check with a descriptive message. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/RegexpQueryTranslator.java [64-66] String fieldName = regexpQuery.fieldName(); RelDataTypeField field = ctx.getField(fieldName); +if (field == null) { + throw new ConversionException("Unknown field [" + fieldName + "] for regexp query"); +} if (field.getType().getSqlTypeName() != SqlTypeName.VARCHAR) { Suggestion importance[1-10]: 6 __ Why: Reasonable improvement to raise a meaningful ConversionException instead of relying on an NPE for unknown fields, aligning behavior with test intent. However, ctx.getField behavior for unknown fields is not visible in the diff, so it's an assumption.	Low
	Reject negative integer flag values Integer.parseInt accepts a leading -, which would silently pass a negative bitmask to RegexpQueryBuilder.flags(int). Since flags are defined as a non-negative OR of RegexpFlag values, reject negative values explicitly to fail fast and match the strictness applied to boolean parsing. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/RegexpQueryDslSerializer.java [39-46] case "flags" -> { // Raw int bitmask — lossless passthrough matching RegexpQueryBuilder.flags(int). try { - regexpQb.flags(Integer.parseInt(entry.getValue())); + int flagsVal = Integer.parseInt(entry.getValue()); + if (flagsVal < 0) { + throw new IllegalArgumentException("Invalid value for 'flags': [" + entry.getValue() + "]; must be non-negative"); + } + regexpQb.flags(flagsVal); } catch (NumberFormatException e) { throw new IllegalArgumentException("Invalid integer value for 'flags': [" + entry.getValue() + "]", e); } } Suggestion importance[1-10]: 4 __ Why: Adds defensive validation against negative flag values, matching the strict boolean parsing style. Minor robustness improvement; unlikely to occur in practice since the translator emits raw ints from valid RegexpFlag values.	Low

Suggestions up to commit 667b8eb

Category	Suggestion	Impact
Possible issue	Guard against null field lookup ctx.getField(fieldName) may return null for unknown fields (as suggested by testUnknownFieldThrows expecting a ConversionException). Dereferencing a null field would produce a NullPointerException rather than a ConversionException. Add a null check that throws ConversionException with a clear message before accessing field.getType(). sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/RegexpQueryTranslator.java [64-66] String fieldName = regexpQuery.fieldName(); RelDataTypeField field = ctx.getField(fieldName); +if (field == null) { + throw new ConversionException("Unknown field [" + fieldName + "] for regexp query"); +} if (field.getType().getSqlTypeName() != SqlTypeName.VARCHAR) { Suggestion importance[1-10]: 5 __ Why: Reasonable defensive check to produce a clear ConversionException instead of NPE for unknown fields, though the behavior of ctx.getField isn't fully visible and it may already throw appropriately.	Low
General	Fail on unknown parameters Silently ignoring unrecognized parameters can hide translator/serializer contract mismatches (e.g., a typo in a new parameter name would go undetected and produce incorrect query semantics). Consider logging a warning or throwing for unknown keys, at minimum during development; forward-compat can be handled by explicit versioning rather than silent drop. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/RegexpQueryDslSerializer.java [57-59] case "rewrite" -> regexpQb.rewrite(entry.getValue()); -default -> { - /* ignore unrecognized params for forward compatibility */ } +default -> throw new IllegalArgumentException("Unknown regexp_query parameter: [" + entry.getKey() + "]"); Suggestion importance[1-10]: 4 __ Why: A valid design trade-off discussion; strict validation would help catch bugs, but the current code explicitly chose forward-compat via silent drop. Not a clear bug.	Low
	Accept symbolic flags in addition to int The case "flags" branch parses the value as an integer, but the golden test file regexp_query_flags_hits.json uses a symbolic form like "COMPLEMENT|INTERVAL". If the translator ever emits a symbolic string (or if flags are ever serialized non-numerically), Integer.parseInt will throw. Consider falling back to RegexpFlag.resolveValue for symbolic input to be robust against both raw-int and symbolic representations. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/RegexpQueryDslSerializer.java [39-46] case "flags" -> { // Raw int bitmask — lossless passthrough matching RegexpQueryBuilder.flags(int). + String v = entry.getValue(); try { - regexpQb.flags(Integer.parseInt(entry.getValue())); + regexpQb.flags(Integer.parseInt(v)); } catch (NumberFormatException e) { - throw new IllegalArgumentException("Invalid integer value for 'flags': [" + entry.getValue() + "]", e); + try { + regexpQb.flags(RegexpFlag.resolveValue(v)); + } catch (IllegalArgumentException iae) { + throw new IllegalArgumentException("Invalid value for 'flags': [" + v + "]", iae); + } } } Suggestion importance[1-10]: 3 __ Why: The translator explicitly emits flags as raw int bitmask (see RegexpQueryTranslator and tests confirming lossless int passthrough). Adding symbolic fallback is defensive but not required by the current contract; the golden JSON's symbolic form is input DSL, not serialized output.	Low

Suggestions up to commit 6f4c3a2

Category	Suggestion	Impact
Possible issue	Guard against null field lookup ctx.getField(fieldName) may return null for an unknown field, which would cause a NullPointerException on the subsequent field.getType() call rather than a clean ConversionException. Add an explicit null check to surface a descriptive error, consistent with the "unknown field throws" test expectation. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/RegexpQueryTranslator.java [65-66] String fieldName = regexpQuery.fieldName(); RelDataTypeField field = ctx.getField(fieldName); +if (field == null) { + throw new ConversionException("Unknown field [" + fieldName + "] for regexp query"); +} if (field.getType().getSqlTypeName() != SqlTypeName.VARCHAR) { Suggestion importance[1-10]: 5 __ Why: Adding a null check would provide a clearer error message, but ctx.getField may already throw ConversionException for unknown fields (as the testUnknownFieldThrows test expects). Without seeing ConversionContext.getField's contract, the concern is speculative.	Low
General	Fail-fast on unknown parameters Silently ignoring unrecognized parameters can mask serializer/translator mismatches (e.g., a typo in a key name) and hide bugs where an option is dropped without effect. Consider throwing IllegalArgumentException for unknown keys instead of silent forward-compat, since new params should be added on both sides in lockstep. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/RegexpQueryDslSerializer.java [58-59] case "max_determinized_states" -> { try { regexpQb.maxDeterminizedStates(Integer.parseInt(entry.getValue())); } catch (NumberFormatException e) { throw new IllegalArgumentException( "Invalid integer value for 'max_determinized_states': [" + entry.getValue() + "]", e ); } } case "rewrite" -> regexpQb.rewrite(entry.getValue()); -default -> { - /* ignore unrecognized params for forward compatibility */ } +default -> throw new IllegalArgumentException("Unknown parameter for regexp_query: [" + entry.getKey() + "]"); Suggestion importance[1-10]: 4 __ Why: Reasonable design point — the current code documents intentional forward-compat by ignoring unknown params. Failing fast could improve debuggability but contradicts the stated intent, so this is a moderate stylistic tradeoff.	Low
	Reject negative flag bitmask values Integer.parseInt accepts negative values, but RegexpQueryBuilder.flags(int) expects a non-negative bitmask; a negative value could produce undefined Lucene automaton behavior. Validate that the parsed value is non-negative before applying it. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/RegexpQueryDslSerializer.java [39-46] case "flags" -> { // Raw int bitmask — lossless passthrough matching RegexpQueryBuilder.flags(int). try { - regexpQb.flags(Integer.parseInt(entry.getValue())); + int flagsVal = Integer.parseInt(entry.getValue()); + if (flagsVal < 0) { + throw new IllegalArgumentException("Invalid 'flags' value: [" + entry.getValue() + "]; must be non-negative"); + } + regexpQb.flags(flagsVal); } catch (NumberFormatException e) { throw new IllegalArgumentException("Invalid integer value for 'flags': [" + entry.getValue() + "]", e); } } Suggestion importance[1-10]: 3 __ Why: Since the translator produces the flags value from a known bitmask, malicious/negative values are unlikely in practice. The extra validation is defensive but low impact.	Low

[github-actions]

❌ Gradle check result for 6f4c3a2: null

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?

[github-actions]

Persistent review updated to latest commit 667b8eb

[github-actions]

✅ Gradle check result for 667b8eb: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.52%. Comparing base (0240b35) to head (01ddb04).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #22693      +/-   ##
============================================
+ Coverage     71.48%   71.52%   +0.04%     
+ Complexity    77022    76955      -67     
============================================
  Files          6156     6139      -17     
  Lines        358482   358059     -423     
  Branches      52246    52218      -28     
============================================
- Hits         256255   256102     -153     
+ Misses        81810    81578     -232     
+ Partials      20417    20379      -38     

☔ 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]

Persistent review updated to latest commit b38c169

[github-actions]

✅ Gradle check result for b38c169: SUCCESS

[github-actions]

Persistent review updated to latest commit c5f0a5f

[github-actions]

❌ Gradle check result for c5f0a5f: 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?

[github-actions]

Persistent review updated to latest commit 637f6d7

[github-actions]

❌ Gradle check result for 637f6d7: null

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?

[github-actions]

Persistent review updated to latest commit 01ddb04

[github-actions]

✅ Gradle check result for 01ddb04: SUCCESS