[GRPC] Implement Boosting and SimpleQueryString queries - #20487
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR upgrades the opensearchprotobufs dependency from 1.2.0 to 1.3.0 and introduces gRPC proto converter implementations for BoostingQuery and SimpleQueryString queries, including comprehensive utilities and unit tests. Error message constants are extracted into query builder classes for reusability across proto and XContent parsing paths. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Converter as QueryBuilderProtoConverter
participant Registry as ProtoConverterRegistry
participant Utils as ProtoUtils
participant Builder as QueryBuilder
Client->>Converter: fromProto(QueryContainer)
activate Converter
Converter->>Converter: Validate input type
Converter->>Utils: fromProto(Query, Registry)
activate Utils
Utils->>Registry: convertToQueryBuilder(positiveQuery)
activate Registry
Registry->>Converter: recursively convert sub-queries
Converter-->>Registry: QueryBuilder result
deactivate Registry
Utils->>Registry: convertToQueryBuilder(negativeQuery)
activate Registry
Registry->>Converter: recursively convert sub-queries
Converter-->>Registry: QueryBuilder result
deactivate Registry
Utils->>Utils: Validate parameters & flags
Utils->>Builder: Construct QueryBuilder
Builder-->>Utils: QueryBuilder instance
deactivate Utils
Utils-->>Converter: QueryBuilder
Converter-->>Client: QueryBuilder result
deactivate Converter
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
2eb2551 to
e8fc397
Compare
|
❌ Gradle check result for e8fc397: 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? |
karenyrx
changed the title
karenyrx
changed the title
cfa2d46 to
295dd80
Compare
|
❌ Gradle check result for 295dd80: 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? |
|
❌ Gradle check result for b14bdbb: 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? |
d01c912 to
93b322b
Compare
|
❗ AI-powered Code-Diff-Analyzer found issues on commit 93b322b.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
93b322b to
29ff77c
Compare
|
❗ AI-powered Code-Diff-Analyzer found issues on commit 29ff77c.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
29ff77c to
3661cef
Compare
|
❗ AI-powered Code-Diff-Analyzer found issues on commit 3661cef.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
karenyrx
added
the
skip-diff-analyzer
|
Skipping the diff analyzer as the protobufs version upgrade is legitimate, and comment recommended to "bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions." 
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
CHANGELOG.mdgradle/libs.versions.tomlmodules/transport-grpc/licenses/protobufs-1.2.0.jar.sha1modules/transport-grpc/licenses/protobufs-1.3.0.jar.sha1modules/transport-grpc/spi/licenses/protobufs-1.2.0.jar.sha1modules/transport-grpc/spi/licenses/protobufs-1.3.0.jar.sha1modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoConverter.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoUtils.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryImpl.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoConverter.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoConverterTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoUtilsTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoConverterTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtilsTests.javaserver/src/main/java/org/opensearch/index/query/BoostingQueryBuilder.javaserver/src/main/java/org/opensearch/index/query/SimpleQueryStringBuilder.javaserver/src/main/java/org/opensearch/index/query/SimpleQueryStringFlag.java
💤 Files with no reviewable changes (2)
- modules/transport-grpc/licenses/protobufs-1.2.0.jar.sha1
- modules/transport-grpc/spi/licenses/protobufs-1.2.0.jar.sha1
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: The gRPC search API in OpenSearch is marked as "experimental" in official documentation, so changes to proto schemas that remove previously unsupported fields (those throwing UnsupportedOperationException) are not considered breaking changes.
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: In the transport-grpc module, suggest and aggregations protos were removed from SearchRequestBody in protobufs 1.0.0 because they haven't been vetted for accuracy in the API specification. The URL parameter suggest support (suggest_field, suggest_mode, suggest_size, suggest_text) is a minimized subset and not intended as a replacement for full Suggester functionality.
📚 Learning: 2026-01-02T19:23:29.698Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: In the transport-grpc module, suggest and aggregations protos were removed from SearchRequestBody in protobufs 1.0.0 because they haven't been vetted for accuracy in the API specification. The URL parameter suggest support (suggest_field, suggest_mode, suggest_size, suggest_text) is a minimized subset and not intended as a replacement for full Suggester functionality.
Applied to files:
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoConverter.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoConverterTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryImpl.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtilsTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoConverter.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtils.java
📚 Learning: 2026-01-02T19:23:16.689Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:16.689Z
Learning: In OpenSearch's gRPC transport module, treat removals of previously-unsupported fields (those throwing UnsupportedOperationException) from experimental proto schemas as non-breaking changes. When reviewing changes to proto-related Java code in this module, document that such removals do not count as breaking API changes, and ensure tests reflect compatibility expectations accordingly.
Applied to files:
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoConverter.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoConverterTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoUtils.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoUtilsTests.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoConverterTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryImpl.javamodules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtilsTests.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoConverter.javamodules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtils.java
📚 Learning: 2026-01-13T17:40:27.167Z
Learnt from: reta
Repo: opensearch-project/OpenSearch PR: 20411
File: server/src/main/java/org/opensearch/index/codec/CodecService.java:112-133
Timestamp: 2026-01-13T17:40:27.167Z
Learning: Avoid capturing or evaluating a supplier (e.g., this::defaultCodec) upfront when passing it to a registry during object construction. If registries may replace defaults during iteration (as in EnginePlugin.getAdditionalCodecs), pass the supplier itself and only resolve it at use time. This ensures dynamic behavior is preserved during initialization and prevents premature binding of defaults in codecs/registry setup. This pattern should apply to similar initialization paths in Java server code where registries may mutate defaults during construction.
Applied to files:
server/src/main/java/org/opensearch/index/query/BoostingQueryBuilder.javaserver/src/main/java/org/opensearch/index/query/SimpleQueryStringBuilder.javaserver/src/main/java/org/opensearch/index/query/SimpleQueryStringFlag.java
📚 Learning: 2026-01-02T19:23:29.698Z
Learnt from: karenyrx
Repo: opensearch-project/OpenSearch PR: 20335
File: modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/SearchSourceBuilderProtoUtils.java:155-172
Timestamp: 2026-01-02T19:23:29.698Z
Learning: The gRPC search API in OpenSearch is marked as "experimental" in official documentation, so changes to proto schemas that remove previously unsupported fields (those throwing UnsupportedOperationException) are not considered breaking changes.
Applied to files:
CHANGELOG.md
🧬 Code graph analysis (7)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoConverter.java (1)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoUtils.java (1)
BoostingQueryBuilderProtoUtils(22-85)
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoConverterTests.java (1)
server/src/main/java/org/opensearch/index/query/BoostingQueryBuilder.java (1)
BoostingQueryBuilder(63-270)
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoUtilsTests.java (3)
server/src/main/java/org/opensearch/index/query/BoostingQueryBuilder.java (1)
BoostingQueryBuilder(63-270)server/src/main/java/org/opensearch/index/query/TermQueryBuilder.java (1)
TermQueryBuilder(57-251)modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoUtils.java (1)
BoostingQueryBuilderProtoUtils(22-85)
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoConverterTests.java (1)
server/src/main/java/org/opensearch/index/query/SimpleQueryStringBuilder.java (1)
SimpleQueryStringBuilder(91-621)
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtilsTests.java (1)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtils.java (1)
SimpleQueryStringBuilderProtoUtils(25-302)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoConverter.java (1)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtils.java (1)
SimpleQueryStringBuilderProtoUtils(25-302)
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtils.java (3)
server/src/main/java/org/opensearch/index/query/AbstractQueryBuilder.java (1)
AbstractQueryBuilder(69-424)server/src/main/java/org/opensearch/index/query/SimpleQueryStringBuilder.java (1)
SimpleQueryStringBuilder(91-621)modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/OperatorProtoUtils.java (1)
OperatorProtoUtils(18-39)
🪛 markdownlint-cli2 (0.20.0)
CHANGELOG.md
[warning] 62-62: Link and image reference definitions should be needed
Duplicate link or image reference definition: "unreleased 3.x"
(MD053, link-image-reference-definitions)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: gradle-check
🔇 Additional comments (49)
modules/transport-grpc/licenses/protobufs-1.3.0.jar.sha1 (1)
1-1: LGTM — checksum file added as expected.
The single-line SHA-1 entry looks correct for a checksum artifact.modules/transport-grpc/spi/licenses/protobufs-1.3.0.jar.sha1 (1)
1-1: LGTM — checksum file added as expected.
The single-line SHA-1 entry looks correct for a checksum artifact.server/src/main/java/org/opensearch/index/query/BoostingQueryBuilder.java (1)
197-205: Consistent parsing messages.Using shared constants in
fromXContentkeeps validation messages aligned across parsing paths.modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtils.java (1)
182-301: Flags helpers look solid.The conversion helpers mirror
SimpleQueryStringFlagbehavior and keep the proto path aligned with server semantics.modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoConverter.java (1)
39-46: Clear validation and delegation.The converter checks container type and cleanly delegates to the utils implementation.
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoConverter.java (1)
39-46: LGTM.Clear container validation and clean delegation to the utils.
modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/QueryBuilderProtoConverterRegistryImpl.java (1)
72-73: Registry wiring looks good.New converters are registered alongside existing built-ins in the expected initialization flow.
server/src/main/java/org/opensearch/index/query/SimpleQueryStringFlag.java (1)
90-90: Consistent error messaging.Using the shared prefix keeps unknown-flag errors aligned across parsing paths.
server/src/main/java/org/opensearch/index/query/SimpleQueryStringBuilder.java (1)
111-113: Consistent error messaging via shared constants.Nice consolidation of validation messages across constructor and XContent parsing.
Also applies to: 159-161, 579-582
modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoConverterTests.java (5)
18-24: Clean test setup.
26-33: Handled query case test looks good.
35-56: Happy-path conversion assertions are solid.
58-69: Invalid container guardrail is covered.
72-81: Null container case is covered.modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoConverterTests.java (5)
21-30: Setup wiring is clear and sufficient.
32-35: Handled query case test is straightforward.
37-64: Happy-path conversion assertions are solid.
66-78: Invalid container behavior is covered.
80-89: Null container behavior is covered.modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtilsTests.java (19)
22-54: Baseline conversion coverage is thorough.
56-111: Full-field mapping test is comprehensive.
113-124: Missing query validation test is solid.
126-137: Empty query validation test is solid.
139-153: Field parsing without boosts is covered.
155-167: Invalid boost notation behavior is tested.
169-198: Single-flag handling coverage looks good.
200-211: Multiple flags (pipe-delimited) case covered.
213-223: ALL flag handling covered.
225-235: NONE flag handling covered.
237-252: Invalid flags error path covered.
254-263: Minimum-should-match int mapping covered.
265-274: Minimum-should-match string mapping covered.
276-285: Default operator OR mapping covered.
287-296: Default operator AND mapping covered.
298-308: Empty flags string defaulting covered.
310-320: Flags parsing with spaces covered.
322-331: Empty fields list defaulting covered.
333-346: Fuzzy settings mapping covered.modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoUtilsTests.java (11)
23-30: Setup is clean and focused.
32-61: All-fields conversion test is comprehensive.
63-81: Minimal-fields conversion test is solid.
83-102: Missing positive query validation is covered.
104-120: Missing negative query validation is covered.
122-134: Zero negative-boost behavior is covered.
136-155: Negative negative-boost validation is covered.
157-173: Boost-only mapping test is solid.
175-191: Name-only mapping test is solid.
193-231: Complex nested queries mapping is well tested.
233-242: Helper builders keep tests tidy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 15-29: The CHANGELOG.md includes many unrelated "Added" bullets;
trim this section to only the entries introduced by this PR (specifically keep
the "- Implement gRPC Boosting and SimpleQueryString queries ([`#20487`])" entry)
and remove or move other unrelated bullets (e.g., items like HTTP/3, netty4,
tracing, index-level-encryption, etc.) so the release notes accurately reflect
only the changes in this PR; update the "Added" list to contain just the PR
20487 entry (or any directly related entries) and, if needed, reference or
relocate the other entries to their respective PR changelogs.
- Around line 59-62: The CHANGELOG.md contains duplicate reference definitions
for the label "[Unreleased 3.x]"; remove the redundant reference block so only a
single "[Unreleased 3.x]:" reference remains (update the remaining definition to
the correct compare range as needed) and ensure all occurrences of the label
resolve to that single definition.
In `@gradle/libs.versions.toml`:
- Line 27: The dependency version for the opensearch protobufs artifact is
invalid: update the opensearchprotobufs entry in gradle/libs.versions.toml to a
version that exists on Maven Central (e.g., change "1.3.0" to a published
version such as "1.1.0") or wait until the desired release is published, and
ensure the chosen version matches the published artifact in Maven Central per
the transport-grpc README workflow; verify by checking Maven Central for
org.opensearch:protobufs before committing.
In
`@modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/BoostingQueryBuilderProtoUtils.java`:
- Around line 47-65: The proto fields are not presence-checked before
conversion, so add guards in BoostingQueryBuilderProtoUtils: call
boostingQueryProto.hasPositive() and if false throw new
IllegalArgumentException(BoostingQueryBuilder.POSITIVE_QUERY_REQUIRED) before
invoking registry.fromProto(positiveContainer); do the same for
boostingQueryProto.hasNegative() with
BoostingQueryBuilder.NEGATIVE_QUERY_REQUIRED; for negativeBoost, check
boostingQueryProto.hasNegativeBoost() and throw
BoostingQueryBuilder.NEGATIVE_BOOST_POSITIVE_VALUE_REQUIRED if missing, then
read negativeBoost and validate it is > 0 (throw the same constant if <= 0);
after registry.fromProto(...) still validate the returned QueryBuilder is not
null and throw the same respective REQUIRED constant if it is.
In
`@modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/query/SimpleQueryStringBuilderProtoUtils.java`:
- Around line 63-83: The proto validation currently rejects empty query strings
(using SimpleQueryStringBuilder.QUERY_TEXT_MISSING) and silently defaults
invalid "field^boost" values to AbstractQueryBuilder.DEFAULT_BOOST; change it to
mirror server XContent behavior: only reject null queryBody (do not treat empty
as missing) and when processing fields in simpleQueryStringProto.getFieldsList()
(populating fieldsAndWeights) throw on invalid boost formats instead of
defaulting—i.e., on NumberFormatException for the boost part, rethrow an
IllegalArgumentException (or propagate a parsing exception consistent with
QueryParserHelper.parseFieldsAndWeights) so invalid boosts are reported rather
than replaced silently.
In `@server/src/main/java/org/opensearch/index/query/BoostingQueryBuilder.java`:
- Around line 66-70: In BoostingQueryBuilder, three error message constants
(POSITIVE_QUERY_REQUIRED, NEGATIVE_QUERY_REQUIRED,
NEGATIVE_BOOST_POSITIVE_VALUE_REQUIRED) include an extra trailing single-quote
character; remove the trailing "'" from each string literal so the messages read
correctly without the stray quote, keeping the rest of the text unchanged and
ensuring the constants remain public static final Strings.
|
❌ Gradle check result for 3661cef: 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? |
|
❌ Gradle check result for 3661cef: 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: Karen X <karenxyr@gmail.com>
3661cef to
1799143
Compare
20ec154 to
213f2cd
Compare
|
❕ Gradle check result for 213f2cd: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #20487 +/- ##
============================================
+ Coverage 73.19% 73.35% +0.15%
- Complexity 71924 72098 +174
============================================
Files 5781 5785 +4
Lines 329292 329559 +267
Branches 47514 47560 +46
============================================
+ Hits 241026 241732 +706
+ Misses 68925 68429 -496
- Partials 19341 19398 +57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| - Add indices to search request slowlog ([#20588](https://github.com/opensearch-project/OpenSearch/pull/20588)) | ||
| - Add support for fields containing dots in their name as literals ([#19958](https://github.com/opensearch-project/OpenSearch/pull/19958)) | ||
| - Add support for forward translog reading ([#20163](https://github.com/opensearch-project/OpenSearch/pull/20163)) | ||
| - Added public getter method in `SourceFieldMapper` to return excluded field ([#20205](https://github.com/opensearch-project/OpenSearch/pull/20205)) |
There was a problem hiding this comment.
Yep, was removed in an earlier commit!
varunbharadwaj
left a comment
varunbharadwaj
left a comment
There was a problem hiding this comment.
Looks good overall
| // Note: XContent only checks for null (allows empty strings) | ||
| // Proto strings are never null (default to empty), so this check will never trigger | ||
| // but we keep it for consistency with XContent pattern | ||
| if (queryBody == null) { |
There was a problem hiding this comment.
If it'll never be null and default to empty, should we check for empty string instead here?
| * @return Array of SimpleQueryStringFlag | ||
| */ | ||
| private static SimpleQueryStringFlag[] convertIntToFlags(int flagsValue) { | ||
| if (flagsValue == -1) { |
There was a problem hiding this comment.
Would something like flagsValue == SimpleQueryStringFlag.ALL.value() be better here?
| SimpleQueryStringFlag flag = SimpleQueryStringFlag.valueOf(s.toUpperCase(java.util.Locale.ROOT)); | ||
| switch (flag) { | ||
| case NONE: | ||
| return 0; |
There was a problem hiding this comment.
Same here - use enum to retrieve the values?
3 participants
Description
Paired with opensearch-project/opensearch-protobufs#376
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
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.