FullGroupJoin with null keys

[fhucko]

I expected that FullGroupJoin will return all items, even those with null keys, but somehow FullGroupJoin does not return null keys.
I tried implementing custom comparer, but it did not help.

So to fix this, I am currently using "clever hack" for key functions using tuple: p => (p.Id, true).
This treats null keys as non-null. A record would work too I guess.

Is there any more clear way to make FullGroupJoin not ignore null keys?

Null keys do not work: https://dotnetfiddle.net/iefSBj
Null keys work: https://dotnetfiddle.net/QS7ZNK

[viceroypenguin]

Technically, this matches the behavior of Enumerable.Join. There is no mention of ignoring null in the documentation, but if you test it, you'll notice that nulls are excluded from analysis.

Example:

new int?[] { 1, 2, null, 4, 5, 1, 2, 4, null, null, 3, 6, }
	.Join(
		new int?[] { 2, 3, 4, null, 1, 2, 7, null, 4, 2, 1, },
		x => x,
		x => x,
		(a, b) => (a, b))

A ValueTuple<> (like you did) is the best way to get around it if you have null values that should be matched.

There's room for debate over whether nulls should be: a) silently ignored (as current, and like Join), b) explicitly ignored (via notnull and in docs), or c) handled properly. I can make decent arguments for any of the above, though I lean towards b) for semantic interpretation of null.

[fowl2]

Whoah what a foot gun! This took me a few hours to track down deep in a codebase.

1. Would you accept a documentation patch calling this out?

2. would you accept a PR to add a parameter to control the null key behaviour?

I quickly hacked around it by removing the check, and it seems to work but I don't quite understand the code enough to be sure:

MoreLINQ/MoreLinq/Lookup.cs

Line 98 in 03a160c

if (keySelector(item) is { } key)

3. If this is confirmed undocumented behavior of Enumerable.Join, is there a doc bug about it?

[atifaziz]

@fowl2 Lookup.cs in this project comes from .NET, which has the same guard and behaviour.

1. Would you accept a documentation patch calling this out?

Sure.

2. would you accept a PR to add a parameter to control the null key behaviour?

Not really.