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

[BUG] update and upsert bulk actions do not include changes from document_root_key, exclude_keys, etc #3745

Closed
graytaylor0 opened this issue Dec 1, 2023 · 2 comments · Fixed by #3747
Assignees
Labels
bug Something isn't working
Milestone

Comments

@graytaylor0
Copy link
Member

Describe the bug
The update and upsert actions always pass the full Event to the bulk actions, instead of only passing the part of the Event that is filtered from document_root_key, exclude_keys, include_keys, etc.

For update we are passing the original jsonNode here (

), while for index we pass the document without the root key / exclude_keys here ( ).

To Reproduce
Steps to reproduce the behavior:

Send two items with the following structure and sink config

{"operation": "index", "item": { "doc_id": 110, "name": "Original Name" }}
{"operation": "update", "item": { "doc_id": 110, "name": "Updated Name" }}
        document_id_field: "item/doc_id"
        index: "actions-test-index"
        document_root_key: "item"
        actions:
          - type: "update"
            when: '/operation == "update"'
          - type: "index"

and the final document in OpenSearch will look like

{
        "_index": "actions-test-index",
        "_id": "110",
        "_score": 1,
        "_source": {
          "doc_id": 110,
          "name": "Original Name",
          "item": {
            "name": "Updated Name",
            "doc_id": 110
          },
          "operation": "update"
        }
}

Expected behavior

The updated document should look like

{
        "_index": "actions-test-index",
        "_id": "110",
        "_score": 1,
        "_source": {
          "doc_id": 110,
          "name": "Updated Name"
        }
}
@dlvenable
Copy link
Member

The update API in opensearch-java requires we pass in a JsonNode. So that part is what we need. Perhaps the problem is from where we get the JsonNode. Could we get this from the SerializedDocument object instead?

@dlvenable
Copy link
Member

Also, I think this would be a good opportunity to split this logic into another class. There is too much going on in there. If we had a class responsible for this mapping we could have some really solid unit tests to ensure that the output is what we expect for each case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
2 participants