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

Set ingest processor supports copying from one field to another #10529

Closed
wants to merge 5 commits into from

Conversation

gaobinlong
Copy link
Collaborator

@gaobinlong gaobinlong commented Oct 10, 2023

Description

When using template snippets like {}, the rendered result is always string, not the original field type, in order to implement copying from one field to another, this PR adds a new parameter named copy_from for set processor which can be set to one of the field in the incoming document, and the value of that field will be copied deeply and assigned to the specified field.

copy_from and value are mutually exclusive, either of them can be set.

Related Issues

#10134

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)
  • 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.

Signed-off-by: Gao Binlong <[email protected]>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 10, 2023

Compatibility status:

Checks if related components are compatible with change df21af1

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/sql.git]

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@gaobinlong
Copy link
Collaborator Author

The failed test is a flaky test, there's an issue tracking it.

@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Nov 11, 2023
Signed-off-by: Gao Binlong <[email protected]>
Copy link
Contributor

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

@gaobinlong
Copy link
Collaborator Author

Flaky test : #2775.

@opensearch-trigger-bot opensearch-trigger-bot bot removed the stalled Issues that have stalled label Nov 22, 2023
@dblock
Copy link
Member

dblock commented Nov 26, 2023

@gaobinlong I restarted gradle check and checked the boxes in the description for you, but please do go through the checklist - for next one you can force push to your branch after a flaky test, and it will kick those things for you

Copy link
Contributor

✅ Gradle check result for 6f3d0eb: SUCCESS

Copy link

codecov bot commented Nov 26, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (5ccd134) 71.30% compared to head (df21af1) 70.61%.

Files Patch % Lines
...ava/org/opensearch/ingest/common/SetProcessor.java 81.81% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #10529      +/-   ##
============================================
- Coverage     71.30%   70.61%   -0.69%     
+ Complexity    59157    58571     -586     
============================================
  Files          4906     4906              
  Lines        278198   278215      +17     
  Branches      40422    40428       +6     
============================================
- Hits         198358   196454    -1904     
- Misses        63414    65269    +1855     
- Partials      16426    16492      +66     

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

@gaobinlong
Copy link
Collaborator Author

@dblock thanks, now all checks have passed yet, could you help to review this PR?

@dblock
Copy link
Member

dblock commented Nov 28, 2023

Not very familiar with this part of the code, maybe another maintainer can take a look?

@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Dec 29, 2023
Signed-off-by: Gao Binlong <[email protected]>
Copy link
Contributor

github-actions bot commented Jan 2, 2024

❕ Gradle check result for df21af1: UNSTABLE

  • TEST FAILURES:
      2 org.opensearch.common.util.concurrent.QueueResizableOpenSearchThreadPoolExecutorTests.classMethod
      1 org.opensearch.remotestore.RemoteIndexPrimaryRelocationIT.testPrimaryRelocationWhileIndexing
      1 org.opensearch.common.util.concurrent.QueueResizableOpenSearchThreadPoolExecutorTests.testResizeQueueDown

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@opensearch-trigger-bot opensearch-trigger-bot bot removed the stalled Issues that have stalled label Jan 2, 2024
@gaobinlong
Copy link
Collaborator Author

Could anyone help to review this PR? It's been opened for months.

@reta
Copy link
Collaborator

reta commented Jan 4, 2024

Could anyone help to review this PR? It's been opened for months.

Why we are not introducing dedicated CopyProcessor (or CopyFormProcessor)?

@msfroh
Copy link
Collaborator

msfroh commented Jan 4, 2024

Why we are not introducing dedicated CopyProcessor (or CopyFromProcessor)?

While I get that the set processor is often used for "simple" copying cases (e.g. as a way of "flattening" an input document), I'm inclined to agree with @reta that this behavior is distinct enough that it probably warrants its own processor.

As a bonus, since you know that the source is definitely going to be another field, you could (for example) add a remove_original parameter to the new processor to let it behave like a "field move", rather than a field copy.

@gaobinlong
Copy link
Collaborator Author

Why we are not introducing dedicated CopyProcessor (or CopyFromProcessor)?

While I get that the set processor is often used for "simple" copying cases (e.g. as a way of "flattening" an input document), I'm inclined to agree with @reta that this behavior is distinct enough that it probably warrants its own processor.

As a bonus, since you know that the source is definitely going to be another field, you could (for example) add a remove_original parameter to the new processor to let it behave like a "field move", rather than a field copy.

OK, implementing a new processor makes things simpler, and will not confuse users, I'll open another PR to implement CopyProcessor, thanks!

@gaobinlong
Copy link
Collaborator Author

Close this PR because #11870 has been merged.

@gaobinlong gaobinlong closed this Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants