Skip to content

Conversation

@DanielBertocci
Copy link

Fix unexpected behavior when HandleNullPropagationOption is configured in ODataQuerySettings

Context

Although a developer can set the value of context.QuerySettings.HandleNullPropagationOption using

[EnableQuery(HandleNullPropagation = HandleNullPropagationOption.False)]

or using DI with ODataQuerySettings

services.AddSingleton(new ODataQuerySettings {
    HandleNullPropagation = HandleNullPropagationOption.False
});

In the current implementation, when execution enters SelectExpandBinder the setting is sometimes forced to true, regardless of the developer's configuration.

This is my first time contributing to this library, although I have used it extensively. I tried to trace the history of this behavior but couldn't find a clear reason for forcing the option. The oldest commit I found that introduced this change is Commit: Merge routing, formatter, query into one from five years ago, and the behavior appears to have been retained through subsequent refactorings.

@xuzhg — you authored that commit and have recently worked on this file, so perhaps you can help if I've missed something.

All the tests are green, and I don't think if it is valuable to introduce a new one to validate this behavior.

Current workaround (project side)

I register a custom FilterBinder that forces the HandleNullPropagation setting:

class CustomODataFilterBinder : FilterBinder
{
    public override Expression Bind(QueryNode node, QueryBinderContext context)
    {
        context.QuerySettings.HandleNullPropagation = HandleNullPropagationOption.False;
        return base.Bind(node, context);
    }
}

How I encountered this issue

I have a project that uses Entity Framework with PostgreSQL. I needed to build OData queries involving many-to-many relationships, for example:

  1. nodes?$expand=children($filter=children/$count gt 0)
  2. customers?$expand=orders($filter=products/$count eq 0)
  3. customers?$expand=orders($filter=products/any(c: c/name eq 'Sample'))

In case 1, even though HandleNullPropagationOption was forced to true, the expression that joined the same table did not cause issues. However, when joining different tables (cases 2 and 3), the generated expression included null checks that Entity Framework could not translate.

@DanielBertocci
Copy link
Author

@microsoft-github-policy-service agree

@WanjohiSammy
Copy link
Member

/AzurePipelines run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.


private QueryBinderContext CreateSubContext(QueryBinderContext context, ComputeClause computeClause, Type clrElementType,
HandleNullPropagationOption option = HandleNullPropagationOption.True)
private QueryBinderContext CreateSubContext(QueryBinderContext context, ComputeClause computeClause, Type clrElementType)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not pass the HandleNullPropagation in the caller. For example:

  • In SelectExpandBinder, you can pass as follows:
    // rest of code ..
    QueryBinderContext subContext = CreateSubContext(context, computeClause, clrElementType, settings.HandleNullPropagation);
    // rest of code ...

Copy link
Author

@DanielBertocci DanielBertocci Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @WanjohiSammy. The reason why I didn't pass HandleNullPropagation in the caller is because CreateSubContext originally clones the settings:

ODataQuerySettings newSettings = new ODataQuerySettings();
newSettings.CopyFrom(context.QuerySettings);

The CopyFrom method does copy the HandleNullPropagation value:

internal void CopyFrom(ODataQuerySettings settings)
{
    // ...
    HandleNullPropagation = settings.HandleNullPropagation;
    // ...
}

I kept this approach because the original code also cloned context.QuerySettings, and I wanted to preserve that behavior unless there's a specific reason to move to the overload that takes HandleNullPropagation directly. If using the explicit overload is preferred for clarity, I can switch to that as well.

Copy link
Member

@WanjohiSammy WanjohiSammy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DanielBertocci Could you add some tests for this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants