QueryRewrite: materializer doesn't intercept reader exceptions

[roji]

In the old pipeline we had TypedRelationalValueBufferFactoryFactory.ThrowReadValueException() intercepting exceptions and throwing threw InvalidOperationException with more detailed messages (including entity name, property name, etc.). We should bring this back in the new pipeline.

Note: the old implementation always threw InvalidOperationException, including for InvalidCastExceptions coming out of the database layer. We should probably change this to throw InvalidCastException instead.

Tests blocked by this (there are probably more):

• FromSqlQueryTestBase.Bad_data_*
• FromSqlQueryTestBase.FromSqlRaw_queryable_simple_columns_out_of_order_and_not_enough_columns_throws
• FromSqlQueryTestBase.From_sql_queryable_simple_columns_out_of_order_and_not_enough_columns_throws

[smitpatel]

Throwing any exception in new pipeline is not implemented yet.

https://github.com/aspnet/EntityFrameworkCore/blob/4b2e969d4b131ad992e270d8f65510e091e8eb4d/src/EFCore.Relational/Query/Pipeline/RelationalShapedQueryCompilingExpressionVisitor.cs#L373-L400

[roji]

Throwing any exception in new pipeline is not implemented yet.

OK. Right now SqlException bubbles up from SqlClient which seems fine. Am not sure this is totally critical for 3.0, we can discuss - I'll update with tests that depend on this as I see them.

[smitpatel]

Disable those tests with TaskList#24

[roji]

OK, as I've already opened this issue we can use it to track? Added reference from task list.

[ajcvickers]

@smitpatel @roji This should only happen when "rich data logging" is enabled, right? Also, I'm not sure I agree with throwing different exception types. It makes it more difficult if people are trying to catch these exceptions in one place. (The inner exception should vary, of course, but I think the one we throw should be the same.)

[ajcvickers]

Note so that I find this next time I search: this is EnableDetailedErrors.