-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
flatten() function does not resolve attribute key conflicts when flattening #35793
Comments
Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
Would also be nice to keep the "original" attribute as is and only add the suffix to the new ones that arrived after flattening. This way it is clear which one was the original vs modified. |
created a first version of solution how this can potentially work #35831. Please consider it as a draft of an idea and not a PR ready for review. Happy to hear about your feedback! |
I agree we should have a conflict resolution option available for this function, thanks for opening a draft PR to look over. Can we offer more than just a suffixing strategy? I can think of a few others that we should offer:
|
Interesting idea, but doesn't this go against the functionality of
This might be valid, but how can you put an order on a map (unordered datatype)? Here we might be able to choose between the deeper/shallower value, but ordering might be problematic, since it will be non-deterministic. |
I see what you mean, I had forgotten slices are flattened as well. In that case, I think we just need to make sure that suffixing keeps slices in mind, for cases like this:
That's a fair point, but pdata maps are actually essentially just wrappers around slices to key-value objects, so there is technically a deterministic ordering. Maybe providing options for preferring deeper/shallower values is a better approach. If you don't provide first-wins/last-wins options, I think we should deprecate the current behavior. |
Ok let's get some conflict examples solved here, so I can proceed with the implementation.
results in
results in
results in
Would you expect a different behavior in any of the use-cases? Especially the third use-case would need some additional attention, since I am not sure, if the information that parameters were an array is important in the resulting outcome. If not, we can just bump the suffix counter without actually preserving the information in any way. Thanks! |
Ok, looked at a little bit different approach how to resolve conflicts between slices and maps, please have a look at the PR. If you agree, I can have a look at the potential first/last wins (only when using no conflicts resolution - here it makes no sense, since we do not loose any data) Thanks! |
Component(s)
pkg/ottl
Is your feature request related to a problem? Please describe.
The
flatten()
function is not able to flatten a map, which has conflicting keys after flattening. In the current state, the latest processed key in the map takes precedence. An example iswhich will result in
and therefore we have data loss
Describe the solution you'd like
I would like to introduce another optional parameter of type
boolean
, which will enable/disable the resolution of conflicts by adding a suffix for the conflicting keys, similar to howflatten()
handles slices. The definition might look like thisWhere the 3rd parameter is type
boolean
and if set tofalse
the behavior stays as it is. If set totrue
, it will keep all the conflicting keys and use a suffix to distinguish them. A solution for the mentioned example would beWe can use a prefix
0
to the suffix to distinguish the array conversions and map key conflict resolutionsDescribe alternatives you've considered
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: