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

XContentBuilder to handle BigInteger and BigDecimal #32888

Conversation

mayya-sharipova
Copy link
Contributor

Although we allow to index BigInteger and BigDecimal into a keyword
field, source filtering on these fields would fail
as XContentBuilder was not able to deserialize BigInteger and BigDecimal
to json.

This modifies XContentBuilder to allow to handle BigInteger and
BigDecimal.

Closes #32395

Although we allow to index BigInteger and BigDecimal into a keyword
field, source filtering on these fields would fail
as XContentBuilder was not able to deserialize BigInteger and BigDecimal
to json.

This modifies XContentBuilder to allow to handle BigInteger and
BigDecimal.

Closes elastic#32395
@mayya-sharipova mayya-sharipova added :Search/Search Search-related issues that do not fall into other categories v7.0.0 v6.5.0 >bug review labels Aug 15, 2018
@smalleyd
Copy link

Do you know when this fix will be released? I'm using 6.4.1 and am still having the problem. Thanks.

@mayya-sharipova
Copy link
Contributor Author

@javanna Hi Luca! Would you please review this PR if you have time?

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

It makes sense to me.

@mayya-sharipova
Copy link
Contributor Author

@elasticmachine test this please

Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

LGTM

@mayya-sharipova
Copy link
Contributor Author

@elasticmachine test this please

@mayya-sharipova mayya-sharipova merged commit 80c5d30 into elastic:master Sep 26, 2018
@mayya-sharipova mayya-sharipova deleted the xcontentbuilder-handle-bigintegerdecimal branch September 26, 2018 18:24
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this pull request Sep 26, 2018
Although we allow to index BigInteger and BigDecimal into a keyword
field, source filtering on these fields would fail
as XContentBuilder was not able to deserialize BigInteger and BigDecimal
to json.

This modifies XContentBuilder to allow to handle BigInteger and
BigDecimal.

Closes elastic#32395
mayya-sharipova added a commit that referenced this pull request Sep 26, 2018
Although we allow to index BigInteger and BigDecimal into a keyword
field, source filtering on these fields would fail
as XContentBuilder was not able to deserialize BigInteger and BigDecimal
to json.

This modifies XContentBuilder to allow to handle BigInteger and
BigDecimal.

Closes #32395
@mayya-sharipova
Copy link
Contributor Author

@smalleyd We have merged this PR and backported it to 6.x. The patch should be available with the next 6.x release.

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 27, 2018
…fallback

* elastic/master:
  TEST: Add engine is closed as expected failure msg
  Adjust bwc version for max_seq_no_of_updates
  Build DocStats from SegmentInfos in ReadOnlyEngine (elastic#34079)
  When creating wildcard queries, use MatchNoDocsQuery when the field type doesn't exist. (elastic#34093)
  [DOCS] Moves graph to docs folder (elastic#33472)
  Mute MovAvgIT#testHoltWintersNotEnoughData
  Security: use default scroll keepalive (elastic#33639)
  Calculate changed roles on roles.yml reload (elastic#33525)
  Scripting: Reflect factory signatures in painless classloader (elastic#34088)
  XContentBuilder to handle BigInteger and BigDecimal (elastic#32888)
  Delegate wildcard query creation to MappedFieldType. (elastic#34062)
  Painless: Cleanup Cache (elastic#33963)
@smalleyd
Copy link

Thanks so much! I'll be on the look up for it in 6.4.2.

@mayya-sharipova
Copy link
Contributor Author

@smalleyd I am not sure if this patch can make into 6.4.2, but it should be available for 6.5

@smalleyd
Copy link

Thanks for the heads up. We'll be sure to bump up to 6.5 if necessary.

kcm pushed a commit that referenced this pull request Oct 30, 2018
Although we allow to index BigInteger and BigDecimal into a keyword
field, source filtering on these fields would fail
as XContentBuilder was not able to deserialize BigInteger and BigDecimal
to json.

This modifies XContentBuilder to allow to handle BigInteger and
BigDecimal.

Closes #32395
@ramafasa
Copy link

Hi @mayya-sharipova
I want to check if this fix is available in 6.x version. We're using 6.8.2 and still having this issue.

jimczi added a commit to jimczi/elasticsearch that referenced this pull request Oct 16, 2019
The support for BigInteger and BigDecimal was added for XContent in
elastic#32888. However the SearchAfterBuilder
xcontent parser doesn't expect them to be present so it throws an AssertionError.
This change fixes this discrepancy by changing the AssertionError into an
IllegalArgumentException that will not cause the node to die when thrown.

Closes elastic#48074
jimczi added a commit that referenced this pull request Oct 23, 2019
* Do not throw errors on unknown types in SearchAfterBuilder

The support for BigInteger and BigDecimal was added for XContent in
#32888. However the SearchAfterBuilder
xcontent parser doesn't expect them to be present so it throws an AssertionError.
This change fixes this discrepancy by changing the AssertionError into an
IllegalArgumentException that will not cause the node to die when thrown.

Closes #48074
jimczi added a commit that referenced this pull request Oct 23, 2019
* Do not throw errors on unknown types in SearchAfterBuilder

The support for BigInteger and BigDecimal was added for XContent in
#32888. However the SearchAfterBuilder
xcontent parser doesn't expect them to be present so it throws an AssertionError.
This change fixes this discrepancy by changing the AssertionError into an
IllegalArgumentException that will not cause the node to die when thrown.

Closes #48074
jimczi added a commit that referenced this pull request Oct 23, 2019
* Do not throw errors on unknown types in SearchAfterBuilder

The support for BigInteger and BigDecimal was added for XContent in
#32888. However the SearchAfterBuilder
xcontent parser doesn't expect them to be present so it throws an AssertionError.
This change fixes this discrepancy by changing the AssertionError into an
IllegalArgumentException that will not cause the node to die when thrown.

Closes #48074
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception when using source filtering on a large integer field
6 participants