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

[DerivedField] object type support in mappings and new settings for derived field #13717

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

rishabhmaurya
Copy link
Contributor

@rishabhmaurya rishabhmaurya commented May 16, 2024

Description

Introduces object type definition and following settings -

"properties": {
   <field>: <type>
},
"source_indexed_field": <field_name>
"format": <date-format>
"ignore_malformed": <boolean>

source_indexed_field: This indexed field should be of type text and will be used to filter documents in 2 phased iterator approach before executing the expensive DerivedFieldQuery fetching from source, running script, deriving value, creating memory index. This is to improve the performance for object type.
properties: fieldname and field type can be specified explicity to avoid inferring the type of sub fields within object type.
format: date format to be used when type is inferred. Unrelated to object type and applicable to date type for both nested and non-nested fields.
ignore_malformed: ignores malformed documents for cases when field value cannot be derived or there is a type mismatch. If set to true, the query will ignore such documents and proceeds with rest of the matching documents.

Example -

{
  "mappings": {
    "properties": {
      "specs_json": { "type": "text" }
    },
    "derived": {
      "derived_specs": {
        "type": "object",
        "script": "emit(params._source[\"specs_json\"])",
        "format": "dd-MM-yyyy",
        "ignore_malformed": true,
        "properties": {
                "details.edition": "keyword",
                "details.pages": "long"
        },
        "source_indexed_field": "specs_json"
      }
    }
  }
}

Diff will reduce once preceding PR is merged: #13592

Related Issues

Resolves #13715

#13592

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • API changes companion pull request created.
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • [ ] Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • [ ] Public documentation issue/PR created

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.

Copy link
Contributor

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

Copy link
Contributor

✅ Gradle check result for 635c12f: SUCCESS

Copy link

codecov bot commented May 16, 2024

Codecov Report

Attention: Patch coverage is 84.80000% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 71.73%. Comparing base (b15cb0c) to head (36cae9d).
Report is 308 commits behind head on main.

Files Patch % Lines
...java/org/opensearch/index/mapper/DerivedField.java 78.57% 7 Missing and 2 partials ⚠️
...rg/opensearch/index/mapper/FieldTypeInference.java 87.83% 4 Missing and 5 partials ⚠️
...opensearch/search/builder/SearchSourceBuilder.java 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13717      +/-   ##
============================================
+ Coverage     71.42%   71.73%   +0.31%     
- Complexity    59978    61409    +1431     
============================================
  Files          4985     5064      +79     
  Lines        282275   288151    +5876     
  Branches      40946    41734     +788     
============================================
+ Hits         201603   206718    +5115     
- Misses        63999    64388     +389     
- Partials      16673    17045     +372     

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

}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(name);
out.writeString(type);
script.writeTo(out);
if (properties == null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I should also check version here before adding a boolean flag to StreamOutput for backward compatibility? As previous version doesn't have the properties and other fields introduced here.

Copy link
Contributor

❌ Gradle check result for 1c1ddbf: 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: Rishabh Maurya <rishabhmaurya05@gmail.com>
Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.com>
Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.com>
Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.com>
Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.com>
Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.com>
Copy link
Contributor

✅ Gradle check result for 36cae9d: SUCCESS

Copy link
Collaborator

@msfroh msfroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good!

Once we merge #13592, I think you'll need to rebase this (to avoid the redundant classes). I'll let you know once I've merged that one, then you can let me know once you've rebased this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request Search:Query Capabilities skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Support mapping for object type and new settings for derived fields
3 participants