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

[Change Proposal] Properly support or remove the array field type #408

Closed
Tracked by #399
jsoriano opened this issue Sep 7, 2022 · 3 comments · Fixed by #412
Closed
Tracked by #399

[Change Proposal] Properly support or remove the array field type #408

jsoriano opened this issue Sep 7, 2022 · 3 comments · Fixed by #412
Assignees
Labels
discuss Issue needs discussion Team:Ecosystem Label for the Packages Ecosystem team

Comments

@jsoriano
Copy link
Member

jsoriano commented Sep 7, 2022

The package-spec allows the use of the array field type, but this filed type doesn't exist. Elasticsearch doesn't have any type for arrays. Instead, any field can contain single or multiple values.

This type seems to come from Beats, where it seems to be some kind of convention when multiple values are expected, but it is also not heavily used there.

On integrations, where it is used, it doesn't generate any mapping, what is probably unexpected.

This type should be properly supported or removed.

Option 1: Removing it

We could just remove it on package-spec 2.0.0 (#399), and force package developers to make a decision on these fields when upgrading to this format.
In most of the cases it should be just replaced by the actual type expected there.

Option 2: Add support for it

Fleet should need to implement the generation of mappings for these fields. Following Beats implementation I think that we could translate fields with type: array to type: keyword, and fields with type: array and object_type: something to type: something.

In elastic-package we could add some validation equivalent to the one for array normalization (elastic/elastic-package#765).

@jsoriano jsoriano added the discuss Issue needs discussion label Sep 7, 2022
@jlind23 jlind23 added the Team:Ecosystem Label for the Packages Ecosystem team label Sep 7, 2022
@jsoriano jsoriano mentioned this issue Sep 8, 2022
16 tasks
@tetianakravchenko
Copy link
Contributor

I think that we could translate fields with type: array to type: keyword

I think this translation might be confusing and contradict with official documentation: according to the elasticsearch doc on arrays https://www.elastic.co/guide/en/elasticsearch/reference/8.4/array.html:

When adding a field dynamically, the first value in the array determines the field type

I would expect to follow the same behavior.

@jsoriano
Copy link
Member Author

jsoriano commented Sep 8, 2022

In Elasticsearch adding fields dynamically refers to adding fields that don't have a mapping before, then Elasticsearch tries to guess the type. In the case of arrays, it uses the first value.
With the fields definitions we are not adding fields dynamically, but providing the mappings beforehand, we have to decide on a type there.

I agree though that doing a silent automatic translation from array to keyword may look unexpected, we could go on the direction of explicitly using both type: array and object_type for the type of the elements, or with the option of removing support for arrays.

Removing support is probably better, in coherence with Elasticsearch data types.

@jsoriano
Copy link
Member Author

I have finally opted by the option of removing support for array. I don't see any case that cannot be properly covered with existing field types, and array leads to confusion as what to expect from this type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issue needs discussion Team:Ecosystem Label for the Packages Ecosystem team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants