efcore: NullReferenceException hides actual exception

MS drives us to the idea of using more stable libraries…

The change from EF Core 2.2 to 3.0 was huge and pretty much the last chance for the EF Core team gain full control over the query pipeline, which they did. But the other side of the coin is, that this change had a high impact on existing apps, potentially destabilizing them with bad performance in cases where existing queries would cause cartesian explosions.

Since 3.0, EF Core in general and the query pipeline in particular have been very stable when it comes to upgrading, at least that is the case for the applications that I am maintaining, where I did not have to change a thing in regards to query performance. In fact, while I believe that none of my queries got slower, some got faster due to more man hours spent on query optimization by the EF Core team and provider maintainers since 3.0.

So in my experience, once you have made the major jump from 2.2 to 3.0/3.1/5.0/6.0 (which can be daunting), you are unlikely to encounter significant upgrade issues in the future.

But we need interceptors, so now we’re examining all the produced queries, maybe it’s better to write them in sql from the beginning…

That is a legitimate question. In most projects, I automatically measure at least the time of all major operations (e.g. Web API calls) and the time of all query executions, as part of my application infrastructure (I would do this with any technology stack really, not just when it comes to EF Core).

Then its just a matter of establishing a value of what can be considered as “fast enough” and checking the actual execution time against it. You then only spend developer time on those few queries that really benefit from some manual optimization (either by adding missing indices or rewriting the query).

---

However, it is also a popular approach to use Dapper for all the read only querying (so your SQL gets executed exactly how you wrote it) and then EF Core for insert/update/delete operations (so you don’t have to track those yourself).

@Obi-Dann Thanks… Are you now using 3.1? What performance issues did you have? As far as I understand EF 3.1 uses single queries and you can use it in EF5, why switch to split queries?

We’re on EF 2 still because we have a lot of split queries (that was the default behaviour) and it isn’t really a priority atm to rewrite them. But because of https://github.com/dotnet/efcore/issues/24808 we can’t keep using split queries in the same way as they were in ef 2

I think it would double the logs.

Could be the case, depending on how many internal exceptions are being thrown in your app (in EF Core and providers, it is not common to throw internal exceptions, except when checking for database existence). Consider using a special log or log level just for this case, so you can easily turn it off again once you found the issue.

Maybe I write a new issue, that EF 5.* cannot be used in production with such problems?

If you feel strongly about it, it might be a good idea. After all, being able to correctly diagnose an issue is an important thing. If the risk is low and the benefit high, it might make it into a servicing release. However, the hurdle to jump for servicing releases is pretty high, so the counter argument could be, that users might already have implemented exception handlers targeting the NRE for 5.0.

But the EF Core team will be able to give you a better estimated, what the chances for a 5.0 backport are.

Would it be possible to backport that fix for 5.0.x? The change looks quite significant, perhaps it’s possible to backport just the part that affects the Dispose method of SplitQueryingEnumerable.Enumerator in, say, 5.0.4. It will be causing quite a lot of pain for us once we finally upgrade to version 5. Happy to submit a PR, if it’s ok

@smitpatel Thanks to bringing my attention back to the issue. As it turns out, we just had a bug in our MySqlCompatibilityExpressionVisitor code:

    public class MySqlCompatibilityExpressionVisitor : ExpressionVisitor
    {
        protected override Expression VisitExtension(Expression extensionExpression)
            => extensionExpression switch
            {
                RowNumberExpression rowNumberExpression => VisitRowNumber(rowNumberExpression),
                CrossApplyExpression crossApplyExpression => VisitCrossApply(crossApplyExpression),
                OuterApplyExpression outerApplyExpression => VisitOuterApply(outerApplyExpression),
                ExceptExpression exceptExpression => VisitExcept(exceptExpression),
                IntersectExpression intersectExpression => VisitIntercept(intersectExpression),
                ShapedQueryExpression shapedQueryExpression => shapedQueryExpression.Update(
                    Visit(shapedQueryExpression.QueryExpression),
                    shapedQueryExpression.ShaperExpression), // <-- missing the Visit() call
                _ => base.VisitExtension(extensionExpression)
            };

        protected virtual Expression VisitCrossApply(CrossApplyExpression crossApplyExpression)
            => CheckSupport(crossApplyExpression, _options.ServerVersion.SupportsCrossApply);
}

This was not an issue when processing the expression visitor at the very end of the pipeline, because ShapedQueryExpressions would not exist anymore.

CollectionJoinApplyingExpressionVisitor should be the last one changing tables. So if base is called beforehand by providers, it should work.

Fixed in #23185

Thanks!

Also afaik EF wouldn’t change a join type after TranslationPostProcessor. Is this being violated at some point?

I believe so. This seems to be the case for some OUTER APPLY or CROSS APPLY statements. I can investigate later today to find out again, which tests failed for us with a MySqlException instead of the expected translation exception when using MariaDB (that does not have LATERAL support, our translation for those expressions).