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

lib: Extract keys and values from map #46

Merged
merged 7 commits into from
Feb 12, 2024
Merged

Conversation

kcreddy
Copy link
Contributor

@kcreddy kcreddy commented Dec 12, 2023

This adds a 2 functions to collections, namely keys and values that can be applied to a map and extract keys and values respectively into a list.

@kcreddy kcreddy self-assigned this Dec 12, 2023
@kcreddy kcreddy added enhancement New feature or request Team:Security-External Integrations Label for the Security External Integrations team labels Dec 12, 2023
@kcreddy kcreddy requested a review from efd6 December 12, 2023 12:28
Comment on lines +16 to +29
"good_instance": [
[
"aa",
"ab"
],
[
2,
3
],
{
"cc": 4,
"cd": 5
}
]
Copy link
Contributor Author

@kcreddy kcreddy Dec 12, 2023

Choose a reason for hiding this comment

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

CI is failing because map doesn't maintain order when iterating using range. Any workarounds to make the tests pass?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think a nice property to have would be to make zip(map_keys(m), map_values(m)) return the original map (well, a copy of it). This currently does not happen, but if it did, this test would pass. It adds an allocation cost to the map_values call and a sort CPU cost to both, but I think it is worth it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added new test into zip to test zip(map_keys(m), map_values(m))

lib/collections.go Outdated Show resolved Hide resolved
lib/collections.go Outdated Show resolved Hide resolved
lib/collections.go Outdated Show resolved Hide resolved
lib/collections.go Show resolved Hide resolved
lib/collections.go Outdated Show resolved Hide resolved
@kcreddy kcreddy requested a review from efd6 February 11, 2024 15:28
lib/collections.go Outdated Show resolved Hide resolved
lib/collections.go Outdated Show resolved Hide resolved
lib/collections.go Outdated Show resolved Hide resolved
testdata/values.txt Outdated Show resolved Hide resolved
@kcreddy kcreddy requested a review from efd6 February 12, 2024 07:37
Comment on lines 1065 to 1086
type kv struct {
Key ref.Val
Value ref.Val
}
if mapK.Size() != types.IntZero {
canSort := true
it := mapK.Iterator()
ss := make([]kv, 0, n)
for it.HasNext() == types.True {
k := it.Next()
v := mapK.Get(k)
ss = append(ss, kv{k, v})
_, ok := k.(traits.Comparer)
if !ok {
canSort = false
}
}
if canSort {
sort.Slice(ss, func(i, j int) bool {
return ss[i].Key.(traits.Comparer).Compare(ss[j].Key) == types.Int(-1)
})
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
type kv struct {
Key ref.Val
Value ref.Val
}
if mapK.Size() != types.IntZero {
canSort := true
it := mapK.Iterator()
ss := make([]kv, 0, n)
for it.HasNext() == types.True {
k := it.Next()
v := mapK.Get(k)
ss = append(ss, kv{k, v})
_, ok := k.(traits.Comparer)
if !ok {
canSort = false
}
}
if canSort {
sort.Slice(ss, func(i, j int) bool {
return ss[i].Key.(traits.Comparer).Compare(ss[j].Key) == types.Int(-1)
})
}
type valComparer interface {
ref.Val
traits.Comparer
}
type kv struct {
Key valComparer
Value ref.Val
}
if mapK.Size() != types.IntZero {
canSort := true
it := mapK.Iterator()
ss := make([]kv, 0, n)
for it.HasNext() == types.True {
k := it.Next()
v := mapK.Get(k)
ck, ok := k.(valComparer)
if !ok {
canSort = false
}
ss = append(ss, kv{ck, v})
}
if canSort {
sort.Slice(ss, func(i, j int) bool {
return ss[i].Key.Compare(ss[j].Key) == types.Int(-1)
})
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, thanks for the recommendation here. Updated

@kcreddy kcreddy requested a review from efd6 February 12, 2024 10:45
@kcreddy kcreddy merged commit 9d975c7 into elastic:dev Feb 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Team:Security-External Integrations Label for the Security External Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants