Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linq expression on Nullable<T>.HasValue does not work when using camelCase serialization #2916

Closed
ssa3512 opened this issue Dec 1, 2021 · 4 comments · Fixed by #2917
Closed

Comments

@ssa3512
Copy link
Contributor

ssa3512 commented Dec 1, 2021

We are continuously addressing and improving the SDK, if possible, make sure the problem persist in the latest SDK version.

Describe the bug
Due to the logic in ExpressionToSQL, when camelCase serialization is enabled, Nullable<T>.HasValue does not translate to IS_DEFINED, and instead translates to ["hasValue"] in query text.

private static SqlScalarExpression VisitMemberAccess(MemberExpression inputExpression, TranslationContext context)
{
SqlScalarExpression memberExpression = ExpressionToSql.VisitScalarExpression(inputExpression.Expression, context);
string memberName = inputExpression.Member.GetMemberName(context?.serializationOptions);
// if expression is nullable
if (inputExpression.Expression.Type.IsNullable())
{
// ignore .Value
if (memberName == "Value")
{
return memberExpression;
}
// convert .HasValue to IS_DEFINED expression
if (memberName == "HasValue")
{
return SqlFunctionCallScalarExpression.CreateBuiltin("IS_DEFINED", memberExpression);
}
}

To Reproduce
Steps to reproduce the behavior. If you can include code snippets or links to repositories containing a repro of the issue that can helps us in detecting the scenario it would speed up the resolution.

void Main()
{
    var client = new CosmosClient("connectionString", new CosmosClientOptions()
    {
    	SerializerOptions = new CosmosSerializationOptions()
    	{
            PropertyNamingPolicy = CosmosPropertyNamingPolicy.CamelCase
    	}
    });
    var container = client.GetContainer("x", "y");
    var query = container.GetItemLinqQueryable<C>()
    	.Where(x => !x.NullableValue.HasValue || x.NullableValue == null);
    Console.WriteLine(query.ToQueryDefinition());	
}

public class C
{
    public DateTime? NullableValue { get; set; }
}

Expected behavior
Query is SELECT VALUE root FROM root WHERE ((NOT IS_DEFINED(root["NullableValue"])) OR (root["NullableValue"] = null))

Actual behavior
Query is SELECT VALUE root FROM root WHERE ((NOT root["nullableValue"]["hasValue"]) OR (root["nullableValue"] = null))

Environment summary
SDK Version: 3.23
OS Version (e.g. Windows, Linux, MacOSX) Windows

Additional context
N/A

@ssa3512
Copy link
Contributor Author

ssa3512 commented Dec 6, 2021

@jackbond there has been some discussion about this behavior over at #2248 but for now this is the behavior of the SDK, and since the IsDefined LINQ extension is not unit testable this is the only mechanism to use LINQ/IQueryable to check for an undefined value and be able to have test coverage. That being said, it seems to make sense to fix the issue with the existing behavior independently from any proposals of behavior changes.

@ssa3512
Copy link
Contributor Author

ssa3512 commented Dec 6, 2021

what isn't testable about IsDefined?

The implementation is throw new NotImplementedException() so while it works if you use it with the SDK IQueryable provider, it will throw in unit tests. Since it's an extension method there is no way to mock it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants
@ssa3512 @kirankumarkolli @j82w @khdang and others