redis / redis-om-dotnet

Object mapping, and more, for Redis and .NET
MIT License
442 stars 74 forks source link

Using Load in AggregationSet not working #415

Closed giabos closed 8 months ago

giabos commented 8 months ago

This example:

var cai = _providerTest.AggregationSet<RedisCachedAuctionItem>();
var counts = cai.Load(a => a.RecordShell!.OptionsSearch).Apply(a => a.RecordShell!.OptionsSearch.Split(',', StringSplitOptions.TrimEntries), "val").GroupBy(a => a["val"]).CountGroupMembers().ToDictionary("val");

Generates this redisearch query:

FT.AGGREGATE idxCachedAuctionItem * LOAD 1 OptionsSearch APPLY split(@OptionsSearch,",") AS val GROUPBY 1 @val REDUCE COUNT 0 AS COUNT

where the OptionsSearch field is missing the @ prefix.

This change seems to fix the problem:

diff --git a/src/Redis.OM/Aggregation/AggregationPredicates/Load.cs b/src/Redis.OM/Aggregation/AggregationPredicates/Load.cs
index ef3fdd0..ccb7812 100644
--- a/src/Redis.OM/Aggregation/AggregationPredicates/Load.cs
+++ b/src/Redis.OM/Aggregation/AggregationPredicates/Load.cs
@@ -27,7 +27,7 @@ namespace Redis.OM.Aggregation.AggregationPredicates
             yield return _properties.Count().ToString();
             foreach (var property in _properties)
             {
-                yield return property;
+                yield return $"@{property}";
             }
         }
     }
slorello89 commented 8 months ago

Hi @giabos - LOAD doesn't require the @ prefix.

127.0.0.1:6379> FT.CREATE test on hash prefix 1 test: SCHEMA name tag
OK
127.0.0.1:6379> HSET test:1 name steve age 34
(integer) 2
127.0.0.1:6379> FT.AGGREGATE test * LOAD 2 name age
1) (integer) 1
2) 1) "name"
   2) "steve"
   3) "age"
   4) "34"

Are you getting an error? If so what? What does your model look like?

giabos commented 8 months ago

Hi @slorello89 thanks for quick answer!

Yes I'm getting this error: image

And then it's working fine when adding the @

Perhaps because we are using a too old redisearch module ? Here is our version:

> module list
name
ft
ver
10425
slorello89 commented 8 months ago

Yeah I think that's got to be it - they probably pushed that change into 2.0 (which was before my time!) For reference, Redis OM was released with RediSearch 2.2 (which is when JSON indexing was introduced).