-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
N:1 left join for Merge generator #11952
Comments
I think other merge "modes" would be really useful. How would you feel about something like this? generators:
- merge:
mode: left-join |
I like that idea. Would be great to add more modes in the future (e.g. Wondering what the current merge mode should be called. It works like a left-join that strictly only allows unique keys. |
Good question. left-unique? When I wrote the original merge generator code, I was writing for a specific internal use case with the expectation that other modes would eventually be added. I tried to use a SQL analogy for our use case, but it quickly broke down. :-) Whatever name makes it most clear for the user is I think best. |
While this is a really a very unique case but this is exactly what I am stuck with right now . |
Maybe merge should be left as-is and have a join plugin ? I mean merge allows to override param of previous generator while join would not. |
I was expecting the merge generator to work like a SQL left join. But it does not allow a N:1 mapping, where the base has a non unique key.
In the following example there are multiple clusters with dev, stg and prod
env
label. I would expect that all clusters would merge with the list generator, that has only one matching entry for each env. Instead I get the errorthe parameters from a generator were not unique by the given mergeKeys, Merge requires all param sets to be unique.
A work around is to combine a matrix and merge generator.
I dont see a reason to forbid non unique keys on the base generator. Allowing non unique key for the base generator would make the Merge generator more useful and would simplify the configuration for usecases like in the example.
The text was updated successfully, but these errors were encountered: