Confusion with Reducer.Count(), memoization vs mutablity

[mgravell]

When using reducers, you might use (for example) Reducer.Count() to fetch the count. If you want to alias that to an explicit field, you might use reducer.As("my_alias"). Unfortunately:

• Reducer.Count() is memoized and singleton, with a private .ctor
• the aliasing via .As() treats it as mutable
• there is also a .Alias with a setter

This means that competing uses smash into each-other, and randomly change the alias of unrelated queries, for example:

// query one
search.GroupBy(..., Reducers.Count().As("qty"))...

// unrelated, query two, could be on a different thread
unrelatedSearch.GroupBy(..., Reducers.Count().As("count"))...

Who gets qty and who gets count is effectively random at this point.

Related code:

NRedisStack/src/NRedisStack/Search/Reducers.cs

Line 8 in 4708e2a

internal static readonly Reducer Instance = new CountReducer();

This could even break arg composition in niche race cases when changing between aliased and non-aliased.

Options:

1. remove the memoization, so each .Count() returns a different instance
2. remove the assumption of safe mutability, so that:

• .As(...) returns a different instance when the value changes
• the .Alias setter throws an exception (we can also mask the setter via property overloading)

Option 1 is simpler and risk free, but has the negative side-effect is performance (more allocations)
Option 2 is more complex and risks runtime breaks

However, if people are using aliasing (and they probably should be), option 2 would end up with the same allocations anyway, so: my vote goes "keep it simple, option 1".

Any thoughts @uglide @atakavci ?

[mgravell]

in the HybridSearch work I am proposing option 1

[mgravell]

Fixed in #454