redis-om-dotnet icon indicating copy to clipboard operation
redis-om-dotnet copied to clipboard

Contains numeric not working

Open iozcelik opened this issue 2 years ago • 11 comments

I use the test model (Person) and search ages in my list but I get Could not retrieve value from Value, most likely, it is not properly decorated in the model defining the index. exception.

The first problem is that, why use nullable int here?

if (resolved is IEnumerable<int> ints)
{
    var sb = new StringBuilder();
    sb.Append('|');
    foreach (var i in ints)
    {
        sb.Append($"[{i} {i}]|");
    }

    sb.Remove(sb.Length - 1, 1);
    return sb.ToString();
}

Second problem is my property is nullable however my list is not. So I need to use .Value inside Contains.

And I think last problem in numeric may be query not like that FT.SEARCH person-idx "@Age:[23 23] | @Age:[25 25]"

image
var provider = new RedisConnectionProvider("redis://localhost:6379");
var connection = provider.Connection;
var people = provider.RedisCollection<Person>();
connection.CreateIndex(typeof(Person));

people.Insert(new Person {
    Name = "Beaker",
    Age = 23,
    Sales = 500000,
    SalesAdjustment = .6,
    Height = 15,
    DepartmentNumber = 3
});

var ages = new List<int> { 23, 25 };

var result = people.Where(w => ages.Contains(w.Age.Value)).ToList();

[Document(StorageType = StorageType.Json, IndexName = "person-idx")]
public class Person {
    [RedisIdField]
    [Indexed]
    public string Id { get; set; }
    [Searchable]
    public string Name { get; set; }

    [Indexed(Sortable = true)]
    public int? Age { get; set; }

    [Indexed(Sortable = true)]
    public double? Height { get; set; }

    [Indexed(Sortable = true)]
    public int? DepartmentNumber { get; set; }

    [Indexed(Sortable = true)]
    public double? Sales { get; set; }

    [Indexed(Sortable = true)]
    public double? SalesAdjustment { get; set; }
}

iozcelik avatar Sep 19 '22 17:09 iozcelik

Hi, @iozcelik while debugging the GetCustomAttributes() does not have any value pf below scrsht for ref. image

Jeevananthan-23 avatar Sep 20 '22 12:09 Jeevananthan-23

Hi @Jeevananthan-23 , your linq test at donetfiddle really has not any attributes. So what I do understand?

iozcelik avatar Sep 21 '22 05:09 iozcelik

Hi @iozcelik sorry pf the below code

using System;
using System.Linq;
using System.Collections.Generic;


public class Program
{
    public static void Main()
    {
        // Student collection
        IList<Student> studentList = new List<Student>() {
            new Student() { StudentID = 1, StudentName = "John", Age = 18, StandardID = 1 } ,
                new Student() { StudentID = 2, StudentName = "Steve",  Age = 25, StandardID = 1 } ,
                new Student() { StudentID = 3, StudentName = "Bill",  Age = 18, StandardID = 2 } ,
                new Student() { StudentID = 4, StudentName = "Ram" , Age = 20, StandardID = 2 } ,
                new Student() { StudentID = 5, StudentName = "Ron" , Age = 33 }
        };
        var ages = new List<int> { 33, 25 };
        var studentNames = studentList.Where(s => ages.Contains(s.Age.Value)).Select(s => s.StudentName);


        foreach (var name in studentNames)
        {
            Console.WriteLine(name);
        }
    }
}

public class Student
{

    public int StudentID { get; set; }
    public string StudentName { get; set; }
    public int? Age { get; set; }
    public int StandardID { get; set; }
}
op:
Steve
Ron

Jeevananthan-23 avatar Sep 21 '22 06:09 Jeevananthan-23

Ok, @Jeevananthan-23 however, I query redis not local list.

iozcelik avatar Sep 21 '22 09:09 iozcelik

@iozcelik Try out both tests


  [Fact]
        public void TestFirstOrDefaultWithMixedLocals() // here for multiple values using for loop working fine 
        {
            _mock.Setup(x => x.Execute(It.IsAny<string>(), It.IsAny<string[]>()))
                .Returns(_mockReply);
            var heightList = new List<double> {70.0, 68.0};
            var y = 33;
            foreach (var height in heightList)
            {

                var collection = new RedisCollection<Person>(_mock.Object);
                var res = collection.FirstOrDefault(x => x.Age == y && x.Height == height);
                _mock.Verify(x => x.Execute(
                    "FT.SEARCH",
                    "person-idx",
                    $"((@Age:[33 33]) (@Height:[{height} {height}]))",
                    "LIMIT",
                    "0",
                    "1"));

            }
        }

        [Fact]
        public void TestBasicQueryWithExactNumericMatch()
        {
            _mock.Setup(x => x.Execute(It.IsAny<string>(), It.IsAny<string[]>()))
                .Returns(_mockReply);
            var  ages = new List<int> { 33, 25 };
            var collection = new RedisCollection<Person>(_mock.Object);
            var res = collection.Where(x =>ages.Contains(x.Age.Value)).ToList(); //your test this fails and throw exception
            _mock.Verify(x => x.Execute(
                "FT.SEARCH",
                "person-idx",
                "(@Age:[33 33] | @Age:[25 25])",
                "LIMIT",
                "0",
                "100"));
        }

Jeevananthan-23 avatar Sep 21 '22 09:09 Jeevananthan-23

@Jeevananthan-23 Yes my test fails. image

iozcelik avatar Sep 21 '22 10:09 iozcelik

Ok yep that's a quirk I totally buy, will fix :)

slorello89 avatar Sep 23 '22 12:09 slorello89

@iozcelik - Looking at this issue again, I'm actually pretty certain that Redis OM .NET is doing the right thing here. In your case you are in fact accessing something that has not been indexed person.Age.Value, sure the Value is meant to be the abstract nullable's integral, however within the context of this collection, you're just trying to say is the person's age equal to, The access to Age should be sufficient for these purposes. Idk, feels like adding a 'fix' for this would be a bit of an unnecessary workaround. I've never seen this bit before, I actually suspect that if you tried interacting with Redis OM .NET in a variety of ways where you accessed a nullable value's Value property, you would encounter something like this.

slorello89 avatar Oct 03 '22 16:10 slorello89

@slorello89 Actually I add indexed attribute as you can see at first post. My aim is that; I have a list and this is not exist redis. This list created inside code with some logics and I need to search with this list. You do not thing about age only. In IoT, it will be status and Redis has support this kind of query a weird way but it has. If I query with @Age:[23 23] | @Age:[25 25], it works as I expected.

iozcelik avatar Oct 06 '22 05:10 iozcelik

Right @iozcelik - the key thing though is that you don't try to access the Nullable<T>.Value property, but just the property itself :)

slorello89 avatar Oct 06 '22 11:10 slorello89

@slorello89 I got your point, however it is not solve the problem. Because this is a scenario, I need. I could query my property with own type. Because in my real scenario, I have statuses and it could be null, however I still search it with contains and if exists, I expect to get result.

iozcelik avatar Oct 12 '22 05:10 iozcelik

@slorello89 I got your point, however it is not solve the problem. Because this is a scenario, I need. I could query my property with own type. Because in my real scenario, I have statuses and it could be null, however I still search it with contains and if exists, I expect to get result.

@iozcelik I'm not sure this relates to #258. Regarding this ticket, Redis doesn't search / store nulls. You would need to use logic before searching Redis to ensure the property is not NULL, otherwise exclude it from the query.

if ( request.From.HasValue ) query = query.Where(q => q.TimeSearch > request.From);
if ( request.To.HasValue ) query   = query.Where(q => q.TimeSearch < request.To);

VagyokC4 avatar Nov 22 '22 16:11 VagyokC4

@VagyokC4 I'm sure about relation. Null is not problem, I try to explain it. It is secondary problem in that issue.

iozcelik avatar Nov 22 '22 21:11 iozcelik

@VagyokC4 I'm sure about relation. Null is not problem, I try to explain it. It is secondary problem in that issue.

I'm not following... What is this 'secondary problem' you speak of?

VagyokC4 avatar Nov 23 '22 00:11 VagyokC4

My primary problem contains for list not working. Nullable or not nullable.

Second, when property is null but value is not null, then still I have problem. Normal query is working for Redis but client is not.

iozcelik avatar Nov 23 '22 06:11 iozcelik

My primary problem contains for list not working. Nullable or not nullable.

Yes, this is outlined in #258. Currently it only works for arrays.. not lists.

Second, when property is null but value is not null, then still I have problem. Normal query is working for Redis but client is not.

I know Redis does not understand NULL... NULL in redis gets deleted and does not exists. RedisJson will not store NULL properties, and you cannot search by a value being NULL. I'm not sure what exactly you are trying to achieve, and would need more context to your "property is null but value is not null".... Again because Redis does not support NULL... I'm not following.

I have been able to query using a nullable property (assuming the .HasValue == true). If I try to query when .HasValue == false I get an error as expected.

VagyokC4 avatar Nov 23 '22 17:11 VagyokC4

@VagyokC4 I explain in first post. As you can see the picture shown how I search. https://user-images.githubusercontent.com/10682780/191081489-d1002899-a0e6-4f0c-afbe-70d22cf6568a.png

This is not redis problem. This is client problem. Because some of item has that property but some of not, because of nullability. So when property is null, the redis om net should use .Value.

iozcelik avatar Nov 23 '22 18:11 iozcelik

https://user-images.githubusercontent.com/10682780/191081489-d1002899-a0e6-4f0c-afbe-70d22cf6568a.png

@iozcelik So, this qurey failed @Age:|[ and this succeeded @Age:[ I'm no redis-search expert (I tend to leave the abstraction to Redis-OM) but the test shows it should be @Age:[ and not @Age:|[ so I'm still not following you exact issue there.

image

Furthermore, the following code works just fine for me:

var ages = new int?[] { 23, 25 }; // Array works List does not
await collection.Where(w => ages.Contains(w.Age)).ToListAsync().Dump("Found");

Am I missing something?

VagyokC4 avatar Nov 23 '22 19:11 VagyokC4

@VagyokC4 the first post, I add all code and the code is not working at me.

https://github.com/redis/redis-om-dotnet/issues/203#issuecomment-1253507917

In this comment, I send what happend, when I try to debug. If you need deep detail, of course I try to figure out what happend and why my code is not working as expected. Unit test may not cover my case.

iozcelik avatar Nov 23 '22 19:11 iozcelik

@VagyokC4 the first post, I add all code and the code is not working at me.

Yeah, I'm still not following. Apologies I couldn't help more.

FWIW: Your first post has bad code, and should fail as @slorello89 already pointed out.

var ages = new List<int> { 23, 25 };
var result = people.Where(w => ages.Contains(w.Age.Value)).ToList();

However, functionally when you make it look like this code here, it works, as I achieve your desired results no problem:

var ages = new int?[] { 23, 25 }; 
var result = people.Where(w => ages.Contains(w.Age)).ToList();

VagyokC4 avatar Nov 24 '22 03:11 VagyokC4

I should add my point here so the result of both the Array and List types are below.

        [Fact]
        public void SearchNumericFieldContains()
        {
            var potentialTagFieldValues = new int?[]{35, 50, 60};
            _mock.Setup(x => x.Execute(It.IsAny<string>(), It.IsAny<string[]>()))
                .Returns(_mockReply);
            var collection = new RedisCollection<Person>(_mock.Object).Where(x => potentialTagFieldValues.Contains(x.Age));
            collection.ToList();
            _mock.Verify(x=>x.Execute(
                "FT.SEARCH",
                "person-idx",
                "@Age:[35 35]|@Age:[50 50]|@Age:[60 60]",
                "LIMIT",
                "0",
                "100"));
        }

        [Fact]
        public void SearchNumericFieldContainsList()
        {
            var potentialTagFieldValues = new List<int?> { 35, 50, 60 };
            _mock.Setup(x => x.Execute(It.IsAny<string>(), It.IsAny<string[]>()))
                .Returns(_mockReply);
            var collection = new RedisCollection<Person>(_mock.Object).Where(x => potentialTagFieldValues.Contains(x.Age));
            collection.ToList();
            _mock.Verify(x => x.Execute(
                "FT.SEARCH",
                "person-idx",
                "(|[35 35]|[50 50]|[60 60]:{\\@Age})", // here the expression parsing fails.
                "LIMIT",
                "0",
                "100"));
        }

Jeevananthan-23 avatar Nov 24 '22 06:11 Jeevananthan-23

Yeah @iozcelik, the fundamental issue here is with the .NET type system, not with Redis OM, in your initial post:

var ages = new List<int> { 23, 25 };

var result = people.Where(w => ages.Contains(w.Age.Value)).ToList();

You needed to use the w.Age.Value because there is a type mismatch between the ages generic type (int) and the and the type of age Nullable<int>, consequentially if you try to remove the Value property, you get an ambiguous reference error, however if you change the generic type of ages to Nullable<int>, you can query it normally:

var ages = new List<int?> { 23, 25 };

var result = people.Where(w => ages.Contains(w.Age)).ToList();

slorello89 avatar Dec 08 '22 20:12 slorello89