-
Notifications
You must be signed in to change notification settings - Fork 193
[BugFix] Fix PPL nested field access inside disabled object fields (#4906) #5390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
qianheng-aws
wants to merge
6
commits into
opensearch-project:main
Choose a base branch
from
qianheng-aws:fix-4906-disabled-object-field
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
2ad9c97
[BugFix] Fix PPL nested field access inside disabled object fields (#…
qianheng-aws df4e4ac
Address review feedback: clean up Content abstraction and redundant b…
qianheng-aws dd1e295
[BugFix] Refactor parseContent/parseStruct per review (#4906)
qianheng-aws ac4afb0
[BugFix] Revert parseStructEntry extraction (#4906)
qianheng-aws 8aefdab
[BugFix] Address review comments on CalciteDisabledObjectFieldIT (#4906)
qianheng-aws 34f9c29
[Enhancement] Use EnterWorktree for ppl-bugfix follow-up
qianheng-aws File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
83 changes: 83 additions & 0 deletions
83
integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteDisabledObjectFieldIT.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.sql.calcite.remote; | ||
|
|
||
| import static org.opensearch.sql.util.MatcherUtils.rows; | ||
| import static org.opensearch.sql.util.MatcherUtils.schema; | ||
| import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; | ||
| import static org.opensearch.sql.util.MatcherUtils.verifySchema; | ||
| import static org.opensearch.sql.util.TestUtils.createIndexByRestClient; | ||
| import static org.opensearch.sql.util.TestUtils.isIndexExist; | ||
| import static org.opensearch.sql.util.TestUtils.performRequest; | ||
|
|
||
| import java.io.IOException; | ||
| import org.json.JSONObject; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.opensearch.client.Request; | ||
| import org.opensearch.sql.ppl.PPLIntegTestCase; | ||
|
|
||
| /** | ||
| * Integration tests for querying inner fields of an object that is declared with {@code "enabled": | ||
| * false} in the index mapping. Such objects are stored in {@code _source} but are not indexed, so | ||
| * the plugin cannot learn the shape of the object from the mapping. See GitHub issue #4906. | ||
| */ | ||
| public class CalciteDisabledObjectFieldIT extends PPLIntegTestCase { | ||
|
|
||
| private static final String DISABLED_OBJECT_INDEX = "test_disabled_object_4906"; | ||
|
|
||
| @Override | ||
| public void init() throws Exception { | ||
| super.init(); | ||
| enableCalcite(); | ||
| createTestIndex(); | ||
| } | ||
|
|
||
| private void createTestIndex() throws IOException { | ||
| if (!isIndexExist(client(), DISABLED_OBJECT_INDEX)) { | ||
| String mapping = | ||
| "{\"mappings\":{\"properties\":{" | ||
| + "\"log\":{\"type\":\"object\",\"enabled\":false}" | ||
| + "}}}"; | ||
| createIndexByRestClient(client(), DISABLED_OBJECT_INDEX, mapping); | ||
| Request bulkReq = new Request("POST", "/" + DISABLED_OBJECT_INDEX + "/_bulk?refresh=true"); | ||
| bulkReq.setJsonEntity( | ||
| "{\"index\":{\"_id\":\"1\"}}\n" + "{\"log\":{\"a\":1,\"c\":{\"d\":2}}}\n"); | ||
| performRequest(client(), bulkReq); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testSelectNestedFieldFromDisabledObject() throws IOException { | ||
| JSONObject result = | ||
| executeQuery(String.format("source=%s | fields log.c.d", DISABLED_OBJECT_INDEX)); | ||
| // Schema is undefined because disabled object fields have no field mapping. | ||
| // The important check is that the value is extracted correctly from _source. | ||
| verifyDataRows(result, rows(2)); | ||
| } | ||
|
|
||
| @Test | ||
| public void testSelectTopLevelFieldFromDisabledObject() throws IOException { | ||
| JSONObject result = | ||
| executeQuery(String.format("source=%s | fields log.a", DISABLED_OBJECT_INDEX)); | ||
| // Schema is undefined because disabled object fields have no field mapping. | ||
| // The important check is that the value is extracted correctly from _source. | ||
| verifyDataRows(result, rows(1)); | ||
|
qianheng-aws marked this conversation as resolved.
|
||
| } | ||
|
|
||
| @Test | ||
| public void testSelectEntireDisabledObject() throws IOException { | ||
| JSONObject result = | ||
| executeQuery(String.format("source=%s | fields log", DISABLED_OBJECT_INDEX)); | ||
| verifySchema(result, schema("log", "struct")); | ||
|
qianheng-aws marked this conversation as resolved.
|
||
| } | ||
|
|
||
| @Test | ||
| public void testSelectIntermediateFieldFromDisabledObject() throws IOException { | ||
| JSONObject result = | ||
| executeQuery(String.format("source=%s | fields log.c", DISABLED_OBJECT_INDEX)); | ||
| verifySchema(result, schema("log.c", "struct")); | ||
|
qianheng-aws marked this conversation as resolved.
|
||
| } | ||
| } | ||
50 changes: 50 additions & 0 deletions
50
integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/4906.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| setup: | ||
| - do: | ||
| indices.create: | ||
| index: test | ||
| body: | ||
| settings: | ||
| number_of_shards: 1 | ||
| number_of_replicas: 0 | ||
| mappings: | ||
| properties: | ||
| log: | ||
| type: object | ||
| enabled: false | ||
| - do: | ||
| query.settings: | ||
| body: | ||
| transient: | ||
| plugins.calcite.enabled : true | ||
|
|
||
| --- | ||
| teardown: | ||
| - do: | ||
| query.settings: | ||
| body: | ||
| transient: | ||
| plugins.calcite.enabled : false | ||
|
|
||
| --- | ||
| "Access nested field inside a disabled object": | ||
| - skip: | ||
| features: | ||
| - headers | ||
| - allowed_warnings | ||
| - do: | ||
| bulk: | ||
| index: test | ||
| refresh: true | ||
| body: | ||
| - '{"index": {}}' | ||
| - '{"log": {"a": 1, "c": {"d": 2}}}' | ||
| - do: | ||
| allowed_warnings: | ||
| - 'Loading the fielddata on the _id field is deprecated and will be removed in future versions. If you require sorting or aggregating on this field you should also include the id in the body of your documents, and map this field as a keyword field that has [doc_values] enabled' | ||
| headers: | ||
| Content-Type: 'application/json' | ||
| ppl: | ||
| body: | ||
| query: 'source=test | fields log.c.d' | ||
| - match: {"total": 1} | ||
| - match: {"datarows": [[2]]} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.