Skip to content
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

replace with getField #7855

Merged
merged 8 commits into from Jul 10, 2023
Merged

replace with getField #7855

merged 8 commits into from Jul 10, 2023

Conversation

marevol
Copy link
Contributor

@marevol marevol commented May 31, 2023

Description

In FlatObjectFieldMapper#parseValueAddFields, getFields is called, but getField needs to be used.

Related Issues

Resolves #7835

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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
Copy link
Contributor

github-actions bot commented Jun 1, 2023

Gradle Check (Jenkins) Run Completed with:

@reta
Copy link
Collaborator

reta commented Jun 1, 2023

Thanks for the change, @marevol , it would be great to have a test case that showcased this issue (and the fix), could you craft one please? Thank you.

@marevol
Copy link
Contributor Author

marevol commented Jun 1, 2023

Our test case is:

  1. Preparing 20,000,000 documents containing flat_object data
  2. Inserting the documents and measuring the indexing time
    (we compared it with the flattened type in Elasticsearch. The indexing time for the flattened type was 3 hours, but for flat_object, it was 12 hours...)
  3. Checking the result from the Nodes hot threads API
    'getFields' appears to be time-consuming in the results.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2023

Gradle Check (Jenkins) Run Completed with:

@reta
Copy link
Collaborator

reta commented Jun 1, 2023

Our test case is:

@marevol That's certainly a good one, do you see the way the issue could have been caught in unit / integration tests since it obviously does use wrong API? thank you.

@marevol
Copy link
Contributor Author

marevol commented Jun 1, 2023

We found this issue in performance tests.
The behavior of flat_object type looked okay.
So it might be difficult to find it in JUnit tests.

@reta
Copy link
Collaborator

reta commented Jun 1, 2023

Please add the entry to CHANGELOG file, under [Unreleased 2.x] / Fixed section

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2023

Gradle Check (Jenkins) Run Completed with:

@noCharger
Copy link
Contributor

noCharger commented Jul 6, 2023

org.opensearch.painless.LangPainlessClientYamlTestSuiteIT > test {yaml=painless/30_search/Flat-object fields from within the scripting} FAILED
    java.lang.AssertionError: Failure at [painless/30_search:559]: field [hits.hits] doesn't have length [1]
    Expected: <1>
         but: was <0>
        at __randomizedtesting.SeedInfo.seed([DACE9185F5EE8DF:85F8D6C2F1A28527]:0)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.executeSection(OpenSearchClientYamlSuiteTestCase.java:460)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.test(OpenSearchClientYamlSuiteTestCase.java:433)
        at java.****/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
        at java.****/java.lang.reflect.Method.invoke(Method.java:578)
        at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1750)
        at com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:938)
        at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:974)
        at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:988)
        at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)

75bb3ef
https://github.com/opensearch-project/OpenSearch/blame/bcaf49455fc6dacd1e19e32b8457a8bc33991c7a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/30_search.yml#L488

@mingshl
Copy link
Contributor

mingshl commented Jul 6, 2023

@marevol we can remove this painless from L488 test. It should help you with the gradle ceck.

I remembered @lukas-vlcek was nice to add in this painless test. But since we agree on FlatObject currently do not support painless script, and we have a new issue here assigned to lukas for future development , so it's fine to remove this test .

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2023

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Mingshi Liu <mingshl@amazon.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.SegmentReplicationRemoteStoreIT.testRestartPrimary

@mingshl
Copy link
Contributor

mingshl commented Jul 10, 2023

@reta can we get reviews again? Trying to see if we can make this in for. 2. 9?

@reta
Copy link
Collaborator

reta commented Jul 10, 2023

@mingshl sign off from you before merge? thank you

@reta reta merged commit 828730f into opensearch-project:main Jul 10, 2023
7 checks passed
@reta reta added the backport 2.x Backport to 2.x branch label Jul 10, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-7855-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 828730f23377e78dd1df0fa53939ed7f2486800e
# Push it to GitHub
git push --set-upstream origin backport/backport-7855-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-7855-to-2.x.

@reta
Copy link
Collaborator

reta commented Jul 10, 2023

@mingshl mind please backporting manually ? thank you

mingshl pushed a commit to mingshl/OpenSearch-Mingshl that referenced this pull request Jul 10, 2023
* replace with getField

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* Update server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>

* add import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* add PR to CHANGELOG

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* move SortedSetDocValuesField addition for field type

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove a test case for flat-object field

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove getField

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* remove unused import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

---------

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>
Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
Co-authored-by: Mingshi Liu <mingshl@amazon.com>
(cherry picked from commit 828730f)
mingshl pushed a commit to mingshl/OpenSearch-Mingshl that referenced this pull request Jul 11, 2023
* replace with getField

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* Update server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>

* add import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* add PR to CHANGELOG

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* move SortedSetDocValuesField addition for field type

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove a test case for flat-object field

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove getField

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* remove unused import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

---------

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>
Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
Co-authored-by: Mingshi Liu <mingshl@amazon.com>
(cherry picked from commit 828730f)
reta pushed a commit that referenced this pull request Jul 11, 2023
* replace with getField

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* Update server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>

* add import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* add PR to CHANGELOG

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* move SortedSetDocValuesField addition for field type

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove a test case for flat-object field

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove getField

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* remove unused import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

---------

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>
Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
Co-authored-by: Mingshi Liu <mingshl@amazon.com>
(cherry picked from commit 828730f)

Co-authored-by: Shinsuke Sugaya <shinsuke@apache.org>
vikasvb90 pushed a commit to raghuvanshraj/OpenSearch that referenced this pull request Jul 12, 2023
* replace with getField

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* Update server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>

* add import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* add PR to CHANGELOG

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* move SortedSetDocValuesField addition for field type

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove a test case for flat-object field

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove getField

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* remove unused import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

---------

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>
Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
Co-authored-by: Mingshi Liu <mingshl@amazon.com>
raghuvanshraj pushed a commit to raghuvanshraj/OpenSearch that referenced this pull request Jul 12, 2023
* replace with getField

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* Update server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>

* add import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* add PR to CHANGELOG

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* move SortedSetDocValuesField addition for field type

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove a test case for flat-object field

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove getField

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* remove unused import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

---------

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>
Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
Co-authored-by: Mingshi Liu <mingshl@amazon.com>
dzane17 pushed a commit to dzane17/OpenSearch that referenced this pull request Jul 12, 2023
* replace with getField

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* Update server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>

* add import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* add PR to CHANGELOG

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* move SortedSetDocValuesField addition for field type

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove a test case for flat-object field

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove getField

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* remove unused import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

---------

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>
Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
Co-authored-by: Mingshi Liu <mingshl@amazon.com>
buddharajusahil pushed a commit to buddharajusahil/OpenSearch that referenced this pull request Jul 18, 2023
* replace with getField

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* Update server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>

* add import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* add PR to CHANGELOG

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* move SortedSetDocValuesField addition for field type

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove a test case for flat-object field

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove getField

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* remove unused import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

---------

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>
Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
Co-authored-by: Mingshi Liu <mingshl@amazon.com>
Signed-off-by: sahil buddharaju <sahilbud@amazon.com>
baba-devv pushed a commit to baba-devv/OpenSearch that referenced this pull request Jul 29, 2023
* replace with getField

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* Update server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>

* add import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* add PR to CHANGELOG

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* move SortedSetDocValuesField addition for field type

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove a test case for flat-object field

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove getField

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* remove unused import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

---------

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>
Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
Co-authored-by: Mingshi Liu <mingshl@amazon.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
* replace with getField

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* Update server/src/main/java/org/opensearch/index/mapper/FlatObjectFieldMapper.java

Co-authored-by: Andriy Redko <drreta@gmail.com>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>

* add import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* add PR to CHANGELOG

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* move SortedSetDocValuesField addition for field type

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove a test case for flat-object field

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

* remove getField

Signed-off-by: Mingshi Liu <mingshl@amazon.com>

* remove unused import

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>

---------

Signed-off-by: Shinsuke Sugaya <shinsuke@apache.org>
Signed-off-by: Shinsuke Sugaya <shinsuke.sugaya@gmail.com>
Signed-off-by: Mingshi Liu <mingshl@amazon.com>
Co-authored-by: Andriy Redko <drreta@gmail.com>
Co-authored-by: Mingshi Liu <mingshl@amazon.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch v2.9.0 'Issues and PRs related to version v2.9.0'
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] FlatObjectFieldMapper calls getFields, not getField
4 participants