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.

Sign up for GitHub

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

Jump to bottom

[BUG] ScriptProcessor fails with Byte and Short data types #14379

Closed
andrross opened this issue Jun 15, 2024 · 3 comments · Fixed by #14380
Closed

[BUG] ScriptProcessor fails with Byte and Short data types #14379

andrross opened this issue Jun 15, 2024 · 3 comments · Fixed by #14380
Labels
bug Something isn't working Indexing Indexing, Bulk Indexing and anything related to indexing untriaged

Comments

@andrross
Copy link
Member

Describe the bug

#11725 introduced a regression in to OpenSearch 2.8 where the following ingest processor will fail:

{
  "description": "_description",
  "processors": [
    {
      "script" : {
        "source" : "ctx.byte = (byte)127;ctx.short = (short)32767"
      }
    },
    {
      "script" : {
        "source" : "ctx.other_field = 'other_field'"
      }
    }
  ]
}

Related component

Indexing

To Reproduce

Index a document using the pipeline defined above in OpenSearch 2.8 or newer

Expected behavior

Request should not fail

Additional Details

No response
@andrross andrross added bug Something isn't working untriaged labels Jun 15, 2024
@github-actions github-actions bot added the Indexing Indexing, Bulk Indexing and anything related to indexing label Jun 15, 2024
andrross added a commit to andrross/OpenSearch that referenced this issue Jun 15, 2024
@andrross
Add missing data types to IngestDocument deep copy
dc0ccdc 
PR opensearch-project#11725 added a new deep copy in the ScriptProcessor flow. If a script
uses a Short or Byte data type then this new deep copy introduced a
regression. This commit fixes that regression.

However, it appears there has been an existing bug where using a
Character type in the same way will fail (this failed before PR 11725).
The failure is different, and appears to be related to something deeping
in the XContent serialization layer. For now, I have fixed the
regression but not yet dug into the failure with the Character data
type. I have added a test that expects this failure.

Resolves opensearch-project#14379

Signed-off-by: Andrew Ross <andrross@amazon.com>
@andrross andrross mentioned this issue Jun 15, 2024
Merged
1 task
@andrross
Copy link
Member Author

FYI @vikasvb90

andrross added a commit to andrross/OpenSearch that referenced this issue Jun 15, 2024
@andrross
Add missing data types to IngestDocument deep copy
40b9e07 
PR opensearch-project#11725 added a new deep copy in the ScriptProcessor flow. If a script
uses a Short or Byte data type then this new deep copy introduced a
regression. This commit fixes that regression.

However, it appears there has been an existing bug where using a
Character type in the same way will fail (this failed before PR 11725).
The failure is different, and appears to be related to something deeping
in the XContent serialization layer. For now, I have fixed the
regression but not yet dug into the failure with the Character data
type. I have added a test that expects this failure.

Resolves opensearch-project#14379

Signed-off-by: Andrew Ross <andrross@amazon.com>
@andrross andrross mentioned this issue Jun 15, 2024
Open
@vikasvb90
Copy link
Contributor

@andrross I just reused the existing deepCopyMap to prevent self referencing objects in script. So, the problem has been there in the method even before my change. Data types were missing and surprisingly there were no tests to validate it as well.

@vikasvb90
Copy link
Contributor

vikasvb90 commented Jun 16, 2024 •
edited
Loading

I validated that this breaks without my change in toXContent in simulate flow here .

opensearch-trigger-bot bot pushed a commit that referenced this issue Jun 17, 2024
@github-actions
Add missing data types to IngestDocument deep copy (#14380)
3c94219 
PR #11725 added a new deep copy in the ScriptProcessor flow. If a script
uses a Short or Byte data type then this new deep copy introduced a
regression. This commit fixes that regression.

However, it appears there has been an existing bug where using a
Character type in the same way will fail (this failed before PR 11725).
The failure is different, and appears to be related to something deeping
in the XContent serialization layer. For now, I have fixed the
regression but not yet dug into the failure with the Character data
type. I have added a test that expects this failure.

Resolves #14379

Signed-off-by: Andrew Ross <andrross@amazon.com>
(cherry picked from commit 112704b)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dblock pushed a commit that referenced this issue Jun 18, 2024
@opensearch-trigger-bot @github-actions
Add missing data types to IngestDocument deep copy (#14380) (#14413)
6c11355 
PR #11725 added a new deep copy in the ScriptProcessor flow. If a script
uses a Short or Byte data type then this new deep copy introduced a
regression. This commit fixes that regression.

However, it appears there has been an existing bug where using a
Character type in the same way will fail (this failed before PR 11725).
The failure is different, and appears to be related to something deeping
in the XContent serialization layer. For now, I have fixed the
regression but not yet dug into the failure with the Character data
type. I have added a test that expects this failure.

Resolves #14379


(cherry picked from commit 112704b)

Signed-off-by: Andrew Ross <andrross@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
harshavamsi pushed a commit to harshavamsi/OpenSearch that referenced this issue Jul 12, 2024
@andrross @harshavamsi
Add missing data types to IngestDocument deep copy (opensearch-projec…
282555c 
…t#14380)

PR opensearch-project#11725 added a new deep copy in the ScriptProcessor flow. If a script
uses a Short or Byte data type then this new deep copy introduced a
regression. This commit fixes that regression.

However, it appears there has been an existing bug where using a
Character type in the same way will fail (this failed before PR 11725).
The failure is different, and appears to be related to something deeping
in the XContent serialization layer. For now, I have fixed the
regression but not yet dug into the failure with the Character data
type. I have added a test that expects this failure.

Resolves opensearch-project#14379

Signed-off-by: Andrew Ross <andrross@amazon.com>
kkewwei pushed a commit to kkewwei/OpenSearch that referenced this issue Jul 24, 2024
@opensearch-trigger-bot @github-actions @kkewwei
Add missing data types to IngestDocument deep copy (opensearch-projec…
e81a98c 
…t#14380) (opensearch-project#14413)

PR opensearch-project#11725 added a new deep copy in the ScriptProcessor flow. If a script
uses a Short or Byte data type then this new deep copy introduced a
regression. This commit fixes that regression.

However, it appears there has been an existing bug where using a
Character type in the same way will fail (this failed before PR 11725).
The failure is different, and appears to be related to something deeping
in the XContent serialization layer. For now, I have fixed the
regression but not yet dug into the failure with the Character data
type. I have added a test that expects this failure.

Resolves opensearch-project#14379

(cherry picked from commit 112704b)

Signed-off-by: Andrew Ross <andrross@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: kkewwei <kkewwei@163.com>
This was referenced Jul 25, 2024
Merged
Merged
@BrewTestBot BrewTestBot mentioned this issue Aug 8, 2024
Merged
wdongyu pushed a commit to wdongyu/OpenSearch that referenced this issue Aug 22, 2024
@andrross
Add missing data types to IngestDocument deep copy (opensearch-projec…
54667c0 
…t#14380)

PR opensearch-project#11725 added a new deep copy in the ScriptProcessor flow. If a script
uses a Short or Byte data type then this new deep copy introduced a
regression. This commit fixes that regression.

However, it appears there has been an existing bug where using a
Character type in the same way will fail (this failed before PR 11725).
The failure is different, and appears to be related to something deeping
in the XContent serialization layer. For now, I have fixed the
regression but not yet dug into the failure with the Character data
type. I have added a test that expects this failure.

Resolves opensearch-project#14379

Signed-off-by: Andrew Ross <andrross@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working Indexing Indexing, Bulk Indexing and anything related to indexing untriaged
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add missing data types to IngestDocument deep copy andrross/OpenSearch
2 participants
@andrross @vikasvb90