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

Preserve JSON types in flatten_json function #13947

Merged
merged 9 commits into from Nov 15, 2022
Merged

Conversation

patrickmann
Copy link
Contributor

Fixes #13888

Pipeline function flatten_json returns all of the extracted values as strings, regardless of original JSON type.
We now honor the type.

See the associated issue for details and examples.

@patrickmann patrickmann marked this pull request as ready for review November 11, 2022 16:28
@patrickmann patrickmann self-assigned this Nov 11, 2022
# Entry type according to https://keepachangelog.com/en/1.0.0/
# One of: a(dded), c(hanged), d(eprecated), r(emoved), f(ixed), s(ecurity)
type = "fixed"
message = "Pipeline function flatten_json respects the original JSON types. An optional parameter is provided for back-compat."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
message = "Pipeline function flatten_json respects the original JSON types. An optional parameter is provided for back-compat."
message = "The pipeline function flatten_json now respects the original JSON types. An optional parameter is provided for backwards compatibility"

Copy link
Member

Choose a reason for hiding this comment

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

I guess we should note that also in UPGRADING.md because it's a breaking change.
I do agree that we should change the default though, it's better to do it now, rather than to have the wrong default forever in our product.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

Choose a reason for hiding this comment

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

@patrickmann did you see my note about UPGRADING.md?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, missed that - fixed it

@mpfz0r mpfz0r self-assigned this Nov 14, 2022
Copy link
Member

@mpfz0r mpfz0r left a comment

Choose a reason for hiding this comment

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

see inline

UPGRADING.md Outdated
@@ -43,6 +43,11 @@ It is now possible to change this behavior. When configuration property `stream_
If all of your streams go to dedicated, separate index sets, it is advised to keep the default value of `stream_aware_field_types` property (`false`). It will decrease the load on ES/OS and stream separation across index sets already helps with showing proper fields for a query.
On the other hand, if multiple streams go to the same index sets, and you want precise field types and suggestions, you should set it to `true`. Consider monitoring your ES/OS load after that change, especially when using huge numbers of fields and streams.

## Breaking changes to pipeline functions
Copy link
Member

Choose a reason for hiding this comment

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

We already have a Breaking changes section in line 8
If I was to skim that document, I might have missed it down here.

UPGRADING.md Outdated Show resolved Hide resolved
@patrickmann patrickmann merged commit e09a07c into master Nov 15, 2022
@patrickmann patrickmann deleted the preserveTypes branch November 15, 2022 12:44
bernd pushed a commit that referenced this pull request Dec 9, 2022
* preserve JSON types when flattening

* cl

* add stringify optional param

* make stringify parameter optional

* update cl

* incorporate review feedback

* add to upgrading.md

* put upgrade message in proper section

* minor wording change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

flatten_json converts all values in that nested blob to string
2 participants