StackExchange / NRediSearch

Other
31 stars 9 forks source link

NRediSearch QueryBuilder does not correctly add parenthesis to unions of different tag fields #16

Open dcoopertd opened 3 years ago

dcoopertd commented 3 years ago

Using NRediSearch Version 2.2.62

Background: I have an example query like this, where the search works successfully:

FT.SEARCH idx "(@IntersectField1:{foo} ((@UnionField1:{bar})|(@UnionField2:{bar})))"

When creating a union on "UnionField1 and UnionField2" they both have to be wrapped in parenthesis, otherwise a syntax error will occur.

In NRedisSearch, consider the following example:

using System;
using NRediSearch;
using static NRediSearch.QueryBuilder.QueryBuilder;
using static NRediSearch.QueryBuilder.Values;

namespace QueryExampleConsoleApp
{
    public static class QueryExample
    {
        public static string GetExampleQuery()
        {
            var query = Intersect().Add("IntersectField1", Tags("foo")).Add(
                    Union().Add("UnionField1", Tags("bar")).Add("UnionField2", Tags("bar")));

            return query.ToString();
        }
    }

    internal class Program
    {
        private static void Main(string[] args)
        {
            Console.WriteLine(QueryExample.GetExampleQuery());
        }
    }
}

In this example the query is returned as:

(@IntersectField1:{foo} (@UnionField1:{bar}|@UnionField2:{bar}))

Which causes a syntax error because there is no parenthesis wrapping @UnionField1:{bar} and @UnionField2:{bar}

It should be: (@IntersectField1:{foo} ((@UnionField1:{bar})|(@UnionField2:{bar})))

I did find a workaround however, by changing the query.ToString() to query.ToString(NRediSearch.QueryBuilder.ParenMode.Always)

While this works, it adds unnecessary parenthesis: ((@IntersectField1:{foo}) ((@UnionField1:{bar})|(@UnionField2:{bar})))

I consider this a "Workaround" because if additional intersections or unions are added and they are empty, you can get cases where the query contains () which will also leads to syntax errors.

Please let me know if this is by design.