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

Disable using unsigned_long in scripts #64523

Merged

Conversation

mayya-sharipova
Copy link
Contributor

Relates to #60050, and #64361

@mayya-sharipova mayya-sharipova added the :Search/Search Search-related issues that do not fall into other categories label Nov 3, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Search)

@elasticmachine elasticmachine added the Team:Search Meta label for search team label Nov 3, 2020
@mayya-sharipova mayya-sharipova added v7.10.0 v7.10.1 v8.0.0 blocker and removed Team:Search Meta label for search team labels Nov 3, 2020
@rjernst
Copy link
Member

rjernst commented Nov 3, 2020

@mayya-sharipova I believe this also needs a change in UnsignedLongLeafFieldData.getScriptValues() to throw an error? Otherwise using doc['some_unsigned_long_field'] would still return?

@mayya-sharipova
Copy link
Contributor Author

@rjernst Thanks Ryan, good comment, addressed in fe4409c. Please continue the review, when you have time.

Copy link
Contributor

@stu-elastic stu-elastic left a comment

Choose a reason for hiding this comment

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

Thanks @mayya-sharipova. The change reverted the added code. I've test that it throws a runtime error when a unsigned_long field is accessed from a script:

    "failed_shards": [
      {
        "shard": 0,
        "index": "ul_test",
        "node": "94nQZ39ZR5SQnah5MN0d_w",
        "reason": {
          "type": "script_exception",
          "reason": "runtime error",
          "script_stack": [
            "org.elasticsearch.xpack.unsignedlong.UnsignedLongLeafFieldData.getScriptValues(UnsignedLongLeafFieldData.java:77)",
            "org.elasticsearch.search.lookup.LeafDocLookup$1.run(LeafDocLookup.java:72)",
            "org.elasticsearch.search.lookup.LeafDocLookup$1.run(LeafDocLookup.java:69)",
            "java.base/java.security.AccessController.doPrivileged(Native Method)",
            "org.elasticsearch.search.lookup.LeafDocLookup.get(LeafDocLookup.java:69)",
            "org.elasticsearch.search.lookup.LeafDocLookup.get(LeafDocLookup.java:36)",
            "state.sqrd.add(doc.a.value * doc.a.value)",
            "                  ^---- HERE"
          ],
          "script": "state.sqrd.add(doc.a.value * doc.a.value)",
          "lang": "painless",
          "position": {
            "offset": 18,
            "start": 0,
            "end": 41
          },
          "caused_by": {
            "type": "unsupported_operation_exception",
            "reason": "Using unsigned_long in scripts is currently not supported!"
          }
        }
      }
    ]

@mayya-sharipova mayya-sharipova merged commit 0ffbcd3 into elastic:master Nov 3, 2020
@mayya-sharipova mayya-sharipova deleted the unsigned-long-disable-scripts branch November 3, 2020 19:20
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this pull request Nov 3, 2020
mayya-sharipova added a commit that referenced this pull request Nov 3, 2020
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this pull request Nov 3, 2020
mayya-sharipova added a commit that referenced this pull request Nov 3, 2020
@andreidan andreidan removed the v7.10.1 label Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker :Search/Search Search-related issues that do not fall into other categories v7.10.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants