Query: Allow providers to replace and extend SqlTranslatingExpressionVisitor

[roji]

While working on array support for PostgreSQL, I ran into an issue: array indexing expressions, which are BinaryExpressions with NodeType ExpressionType.ArrayIndex, aren't recognized in SqlTranslatingExpressionVisitor.VisitBinary, which returns null for them. This prevents Npgsql from being able to map array indexing to SQL.

I could easily extend SqlTranslatingExpressionVisitor, but this doesn't seem to be something providers are supposed to do (i.e. I can't see it in RelationalDatabaseProviderServices). I realize I'm pushing things here, but it seems like a good idea to open this up to provider customizations.

[roji]

Out of curiosity - what exactly would be the impact of extending SqlTranslatingExpressionVisitor right now, before it officially becomes a "provider-overridable" service? Is it purely an issue of contract (where this service may change at any time so I risk breakage), or is there something else?

[ajcvickers]

@roji EF Core allows services for multiple providers to be registered within in the same D.I. container. The set of services to use is then determined for a given context instance with a UseSqlServer or similar. This works because provider service resolution goes through the indirection of IDatabaseProviderServices--it ensures that the context instance gets the correct services for the provider in use.

If a provider replaces a non-provider service when multiple providers have been registered, then the replacement service is going to be resolved even if a different provider is currently being used. This will cause problems if the replaced service does not work with the other provider, or if both providers have attempted to replace the same service.

This bug will not manifest in common cases--where there is only one provider registered. So it could be said that its not a super-important bug. Nevertheless, EF does support multiple provider registration, so providers should play nice and only replace provider services.

[roji]

@ajcvickers thanks for the explanation, understood. And thanks for making the change.

[roji]

Super, thanks for the quick turnaround!