Linq2Couchbase generates invalid WHERE clause

.net
n1ql

#1

Hi Couchbase team,

I am facing the following issue:

I am having custom Id classes, for example TestDocumentId which ToString generates string in the form “testdocument-some-id”

The document class example:

public class TestDocument
{
    public TestDocumentId Id {get; set;}
    public string Name {get; set;}
}

Now, executing

bucketContext.Query<TestDocument>().Where( t => t.Id == TestDocumentIdConst.ToString() )

generates correct query:

SELECT RAW Extent1 FROM test as Extent1 WHERE (Extent1.type = ‘TestDocument’) AND (Extent1.id = ‘testdocument-f1514cc6-dc70-4e1d-9b53-3c506e58c881’)

But the query without .ToString() call generates invalid where clause (wthout ’ marks):

SELECT RAW Extent1 FROM test as Extent1 WHERE (Extent1.type = ‘TestDocument’) AND (Extent1.id = testdocument-f1514cc6-dc70-4e1d-9b53-3c506e58c881)

I think this is due to the fact that that VisitConstant function inside N1QlExpressionTreeVisitor class just calls ToString() method for namedParameters that are custom/unknown types.

I have found similar issue for Guids that has been solved:

Any workaround for this problem without actually changing Linq2Couchbase library?

Thanks


#2

Hi @dario.senic,

In the future, you may want to post .NET and Linq2Couchbase questions into the .NET SDK forums here: https://forums.couchbase.com/c/net-sdk

That being said, I think @btburnett3 should be able to help with this question, so I’m tagging him.


#3

@dario.senic

Can you provide the C# syntax that you’re using that’s generating the incorrect syntax? Also, are you using custom equality methods on the TestDocumentId class?


#4

Hi @btburnett3

Yes, Equals for TestDocumentId is overwritten.

The c# syntax causing the issue is:

bucketContext.Query<TestDocument>().Where( t => t.Id == SomeTestDocumentId )


#5

@dario.senic

In general, LINQ providers don’t support overrides of the equality operator. At least, I don’t know of one that does, but I could be wrong. Since the .Equals function isn’t an Expression tree, there’s no way to know what magic is happening inside it to replicate that in the query.

I would recommend looking at using the new support for custom serialization attributes. You could apply a [JsonConverter(…)] attribute to the TestDocumentId class, and combine that with a change to how the constant is then serialized in the query.

This is new functionality, just added in the last couple of weeks, you’d probably be the first one to use it outside of the internal usage inside Linq2Couchbase. If you do use it, I’d love to get feedback on how it goes. I’m also happy to help.


#6

@btburnett3

I created JsonIdConverter which does serialization of TestDocumentId.

TestDocumentId is decorated with:
[JsonConverter(typeof(JsonIdConverter))]

After that I created TestDocumentIdSerializationConverter inheriting SerializationConverterBase and ISerializationConverter

I registered serialization converter with:

TypeBasedSerializationConverterRegistry.Global.Add( typeof( JsonIdConverter ), typeof( TestDocumentIdSerializationConverter ) );

However, none of the methods inside TestDocumentIdSerializationConverter are being called when expression tree is being created. I was expecting RenderConvertedConstant method to be called.

What am I missing?


#7

@dario.senic

Can you provide me with the full code of TestDocumentIdSerializationConverter?


#8

@btburnett3

I created demo project demonstrating the issue -> https://github.com/dsenic/Linq2CoucbaseWhereQueryGeneration

TestDocumentIdSerializationConverter implementation:

using System;
using System.Collections.Generic;
using System.Linq.Expressions;
using System.Reflection;
using Couchbase.Linq.QueryGeneration;
using Couchbase.Linq.Serialization;
using Couchbase.Linq.Serialization.Converters;

namespace Linq2CoucbaseWhereQueryGeneration
{
    public class TestDocumentIdSerializationConverter :
        SerializationConverterBase,
        ISerializationConverter<TestDocumentId>
    {
        private static readonly IDictionary<Type, MethodInfo> ConvertFromMethodsStatic =
            GetConvertFromMethods<TestDocumentIdSerializationConverter>();

        private static readonly IDictionary<Type, MethodInfo> ConvertToMethodsStatic =
            GetConvertToMethods<TestDocumentIdSerializationConverter>();


        protected override IDictionary<Type, MethodInfo> ConvertFromMethods => ConvertFromMethodsStatic;
        protected override IDictionary<Type, MethodInfo> ConvertToMethods => ConvertToMethodsStatic;


        public TestDocumentId ConvertFrom( TestDocumentId value )
        {
            return value;
        }


        public TestDocumentId ConvertTo( TestDocumentId value )
        {
            return value;
        }

        protected override void RenderConvertedConstant( ConstantExpression constantExpression, IN1QlExpressionTreeVisitor expressionTreeVisitor )
        {
            expressionTreeVisitor.Expression.Append( "'" );
            expressionTreeVisitor.Visit( constantExpression );
            expressionTreeVisitor.Expression.Append( "'" );
        }

        protected override void RenderConvertFromMethod( Expression innerExpression, IN1QlExpressionTreeVisitor expressionTreeVisitor )
        {
            expressionTreeVisitor.Visit( innerExpression );
        }

        protected override void RenderConvertToMethod( Expression innerExpression, IN1QlExpressionTreeVisitor expressionTreeVisitor )
        {
            expressionTreeVisitor.Visit( innerExpression );
        }
    }
}

#9

@dario.senic

Apparently, the current logic is only finding the JsonConverter when applied to the property on the POCO, not when applied to the TestDocumentId class. Once I moved it to the ID property, it worked.

I’d also point out that the custom serializer inheriting from DefaultSerializer shouldn’t be necessary, and that the document type filter is probably better applied as a [DocumentTypeFilter(…)] attribute on the POCO.

I’ve filed a issue to track this limitation: https://github.com/couchbaselabs/Linq2Couchbase/issues/254


#10

@btburnett3 thanks a lot for helping.

I know that both custom serializer and attribute are not needed. It was more of trying every possible solution.

Would it be possible to have alternative approach for this situation other than applying attributes to POCO? I have situation where my POCOs are in external lib and it is not convenient to change then. Seems to me that having custom serializer (in my case inherited from DefaultSerializer) and custom serialization converter should be enough for Linq2Couchbase to understand how to generate underlying queries.


#11

@dario.senic

You are correct, using a custom serializer should be sufficient as well. The key is that Newtonsoft.Json tells us that the converter is in use on a specific property.

Of course, this logic clearly needs some tweaking. When I work on it, I’ll try to make sure it picks up converters added to the global serialization settings as well.


#12

@dario.senic

I have a WIP branch that I believe corrects your issue, both for global converters and attributes on classes, if you want to check it out.


#13

@btburnett3

Works perfectly. Looking forward to it being released officially :slight_smile: