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

Introduce new 'unsigned_long' numeric field type support #3585

Merged
merged 7 commits into from
May 25, 2023
Merged

Introduce new 'unsigned_long' numeric field type support #3585

merged 7 commits into from
May 25, 2023

Conversation

reta
Copy link
Contributor

@reta reta commented Mar 27, 2023

Description

Introduce new 'unsigned_long' numeric field type support

Issues Resolved

Related to opensearch-project/OpenSearch#2083 and opensearch-project/OpenSearch#6237

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@reta reta mentioned this pull request Mar 27, 2023
Merged
14 tasks
@ariamarble
Copy link
Contributor

@reta I see that this is still in a draft state. Is this still being worked on from your end, or is this ready for review?

@reta
Copy link
Contributor Author

reta commented Mar 30, 2023 •
edited
Loading

@reta I see that this is still in a draft state. Is this still being worked on from your end, or is this ready for review?

@ariamarble thanks for looking, wrapping up the test cases and related fixes (opensearch-project/OpenSearch#6901, opensearch-project/OpenSearch#6237), should be ready today / tomorrow

@Naarcha-AWS
Copy link
Collaborator

@reta: Any updates on this? Anything we can do to help?

@reta
Copy link
Contributor Author

reta commented Apr 12, 2023

@reta: Any updates on this? Anything we can do to help?

Thanks @Naarcha-AWS the pull request in core is under review opensearch-project/OpenSearch#6237 but the feature won't make it till 2.8.0

@reta reta marked this pull request as ready for review April 12, 2023 16:01
@reta
Copy link
Contributor Author

reta commented May 5, 2023

@Naarcha-AWS the feature has been merged to main and 2.x (upcoming 2.8.0), thank you for patience !

@Naarcha-AWS Naarcha-AWS self-requested a review May 10, 2023 18:25
@Naarcha-AWS Naarcha-AWS added v2.8.0 1 - Backlog Issue: The issue is unassigned or assigned but not started labels May 10, 2023
@kolchfa-aws kolchfa-aws self-assigned this May 23, 2023
@kolchfa-aws
Copy link
Collaborator

Thank you, @reta! Would you prefer that I comment on the PR or edit directly so you can just review afterwards?

@reta
Copy link
Contributor Author

reta commented May 23, 2023

Thank you, @reta! Would you prefer that I comment on the PR or edit directly so you can just review afterwards?

@kolchfa-aws totally up to you, whatever is more convenient for you, thanks!

@kolchfa-aws
Copy link
Collaborator

@reta Done. Could you please look it over to confirm technical accuracy? Thanks!

@kolchfa-aws
Rename file and fix links
63f524d 
Signed-off-by: Fanit Kolchina <[email protected]>
@kolchfa-aws kolchfa-aws added 4 - Doc review PR: Doc review in progress and removed 1 - Backlog Issue: The issue is unassigned or assigned but not started labels May 24, 2023
vagimeli
vagimeli approved these changes May 24, 2023
View reviewed changes
Copy link
Contributor

@vagimeli vagimeli left a comment

Choose a reason for hiding this comment

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

Minor edit. Looks good.

@reta
Copy link
Contributor Author

reta commented May 24, 2023

@reta Done. Could you please look it over to confirm technical accuracy? Thanks!

Thanks @kolchfa-aws , LGTM!

@kolchfa-aws kolchfa-aws added 5 - Editorial review PR: Editorial review in progress and removed 4 - Doc review PR: Doc review in progress labels May 24, 2023
natebower
natebower reviewed May 25, 2023
View reviewed changes
Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

Just a few comments. Thanks!

_field-types/supported-field-types/unsigned-long.md Outdated
```
{% include copy-curl.html %}

If a field of type `unsigned_long` has the `store` parameter set to `true` (that is, the field is a stored field), it will be stored and returned as a string. `unsigned_long` values do not support the decimal part, so if supplied, the decimal part is truncated.
Copy link
Collaborator

Choose a reason for hiding this comment

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

In both instances of "the decimal part", do we mean something like "the decimal portion of the [noun]"?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We mean the decimal part of the number (where "of the number" is implicitly understood).

@natebower natebower removed the 5 - Editorial review PR: Editorial review in progress label May 25, 2023
@kolchfa-aws kolchfa-aws added the release-notes PR: Include this PR in the automated release notes label May 25, 2023
@kolchfa-aws kolchfa-aws merged commit 2745dd5 into opensearch-project:main May 25, 2023
vagimeli added a commit that referenced this pull request May 26, 2023
@reta @kolchfa-aws
@vagimeli @natebower
Introduce new 'unsigned_long' numeric field type support (#3585)
d95bb6c 
* Introduce new 'unsigned_long' numeric field type support

Signed-off-by: Andriy Redko <[email protected]>

* Readability improvements and style guide changes

Signed-off-by: Fanit Kolchina <[email protected]>

* Rename file and fix links

Signed-off-by: Fanit Kolchina <[email protected]>

* Update _field-types/supported-field-types/unsigned-long.md

Co-authored-by: Melissa Vagi <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>

* Apply suggestions from code review

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>

* Add trailing slash to link

Signed-off-by: Fanit Kolchina <[email protected]>

---------

Signed-off-by: Andriy Redko <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
Co-authored-by: Fanit Kolchina <[email protected]>
Co-authored-by: kolchfa-aws <[email protected]>
Co-authored-by: Melissa Vagi <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
harshavamsi pushed a commit to harshavamsi/documentation-website that referenced this pull request Oct 31, 2023
@reta @kolchfa-aws
@vagimeli @natebower @harshavamsi
Introduce new 'unsigned_long' numeric field type support (opensearch-…
1266f2a 
…project#3585)

* Introduce new 'unsigned_long' numeric field type support

Signed-off-by: Andriy Redko <[email protected]>

* Readability improvements and style guide changes

Signed-off-by: Fanit Kolchina <[email protected]>

* Rename file and fix links

Signed-off-by: Fanit Kolchina <[email protected]>

* Update _field-types/supported-field-types/unsigned-long.md

Co-authored-by: Melissa Vagi <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>

* Apply suggestions from code review

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>

* Add trailing slash to link

Signed-off-by: Fanit Kolchina <[email protected]>

---------

Signed-off-by: Andriy Redko <[email protected]>
Signed-off-by: Fanit Kolchina <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
Co-authored-by: Fanit Kolchina <[email protected]>
Co-authored-by: kolchfa-aws <[email protected]>
Co-authored-by: Melissa Vagi <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@natebower natebower natebower left review comments

@vagimeli vagimeli vagimeli approved these changes

@Naarcha-AWS Naarcha-AWS Awaiting requested review from Naarcha-AWS Naarcha-AWS is a code owner

@cwillum cwillum Awaiting requested review from cwillum

@hdhalter hdhalter Awaiting requested review from hdhalter

@kolchfa-aws kolchfa-aws Awaiting requested review from kolchfa-aws kolchfa-aws is a code owner

@ananzh ananzh Awaiting requested review from ananzh

@seanneumann seanneumann Awaiting requested review from seanneumann

@AMoo-Miki AMoo-Miki Awaiting requested review from AMoo-Miki AMoo-Miki is a code owner

Assignees

@kolchfa-aws kolchfa-aws

Labels
release-notes PR: Include this PR in the automated release notes v2.8.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@reta @ariamarble @Naarcha-AWS @kolchfa-aws @natebower @vagimeli