feat(appset): Support various merge strategies for merge generator

[sonamkshenoy]

Support for various merge strategies in merge generator. Includes:
left-join, left-join-uniq, inner-join, inner-join-uniq, full-join, full-join-uniq

This PR also allows more than one parameter sets to have the same merge key in the base generator

Fixes #11952, #12837, #14560

[sonamkshenoy]

@crenshaw-dev Please review. Thank you!!

Also, do you think it's better to have a more intuitive name for the flag?
Something like allowNonUniqBaseMergeKeys

[crenshaw-dev]

Actually, I think we should probably have a string field called mergeMode to accommodate new modes as they come up.

[darkradish]

Hello !
This Pull request can make applicationSets very powerfull !
@sonamkshenoy do you work at it ? do you want some help ?

[sonamkshenoy]

Hey @darkradish
Thank you!
Yes, I'm actively working on it. Thanks for your offer to help; I'm actually waiting for a review from one of the maintainers. :)

[codecov]

Codecov Report

Attention: Patch coverage is 72.22222% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 49.50%. Comparing base (d595550) to head (c35ac69).
Report is 185 commits behind head on master.

❗ Current head c35ac69 differs from pull request most recent head 1548125. Consider uploading reports for the commit 1548125 to get more accurate results

Files	Patch %	Lines
applicationset/generators/merge.go	72.22%	13 Missing and 7 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #16080      +/-   ##
==========================================
+ Coverage   49.48%   49.50%   +0.01%     
==========================================
  Files         271      271              
  Lines       47728    47774      +46     
==========================================
+ Hits        23620    23651      +31     
- Misses      21773    21782       +9     
- Partials     2335     2341       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ptr1120]

Are there any plans to merge this awesome feature?

[Boeller666]

Same question here... will this feature be merged in the near future?
This would help a lot!

[alancaldelas]

I am also very interested in this

[agaudreault]

Code looks good. I think the user documentation needs to be clarified a bit more to explain the results expected by the different modes.

[agaudreault]

This will allow more than one parameter sets of (only) the base generator

I find it hard to understand the meaning of this sentence. Why is only in parenthesis? what is the base generator?

[wmcnamee-coreweave]

would it be helpful to provide the equivalent SQL to explain joins? It seems like the -uniq is kind of like using DISTINCT, but rather, instead of actually doing distinct, it just errors if there are merge keys that are identical.

[agaudreault]

We cannot expect users to be familiar with SQL. The documentation should be simple enough for a user not familiar with left/inner/full terminology to understand the behavior of these modes.

Something like

With the `uniq` merge modes (`left-join-uniq`, `inner-join-uniq`, `full-join-uniq`), all the parameter sets produced by any generators under a Merge generator need to have unique values for the configured merge keys. An error preventing the generation will be returned when duplicated keys are found.

However, I am still not sure what you are trying to explain with the last sentence of the second paragraph.

[agaudreault]

Maybe a short explanation on the result of inner and full join with the example above would help the users understand which mode to use better.

[wmcnamee-coreweave]

ya, what's the point of the -uniq variants? In this example, if server was added to the mergeKeys list, would left-join-uniq work?

[ProbstenHias]

Hi, what is the progress here, I would love to see this working as we have the need for exactly this feature.
I could help out with this if you want.

[ProbstenHias]

Suggested change

	However, this behaviour can be changed by using any of the merge modes that do not end with `-uniq`. This will allow more than one parameter sets of (only) the **base generator** to have the same values for the merge keys.
	However, this behaviour can be changed by using any of the merge modes that do not end with `-uniq`. Using those, the parameters from a child generator that are used as mergeKeys do no longer have to be unique.

Maybe this is easier to understand regarding the previous comment?

[Boeller666]

@sonamkshenoy If you would do the signoff for your commits we would be one step further towards getting this cool feature into prod. If you might have problems with signing old commits, you could follow this link

[reedjosh]

@sonamkshenoy If you would do the signoff for your commits we would be one step further towards getting this cool feature into prod. If you might have problems with signing old commits, you could follow this link

@sonamkshenoy we could really use this feature.

It seems the final commit is signed. Could this be squashed and rebased?

[joshfrench]

I've rebased and squashed this, I'm hoping we can get it into main: #21441