Query: SqlExpression.Type is non-nullable even when it could be null

[smitpatel]

When we make ColumnExpression nullable, we mark it's type as nullable. But when we do Average over it, we generate SqlUnary (for int/long), which ends up taking Type from typeMapping, which makes it non-null.
Design I am thinking is

• SqlConstantExpression.Type would be pretty obvious to be null/non-null.
• SqlParameterExpression.Type should be coming from Parameter directly. If parameter is non-null type then parameter will never take null value in query.
• ColumnExpression.Type is maintained with nullable flag.
• Any other SqlExpression.Type should be combination of above components.

SqlExpression.Type would be a nullable type if can take null values. In a way it would be superset of null semantics. e.g. Certain things won't be null in tree due to other conditions but still have nullable type.

Another alternative is to make all Types non-null so SQL tree becomes null-agnostic, any SqlExpression can take null value in database. This may require some changes when materializing to know if we need to make null check. (I believe we make null check for everything other than basic SqlConstant/SqlParameter/Column already).

Identified during #20635
The issue arises because a nullable type SqlUnaryExpression got converted to non-null SqlUnary since the TypeMapping's clrType is non-null.

[roji]

Am not a big query honcho, but definitely seems like Type should be nullable if it can take null values - but not in other cases (so your first suggestion, not your second). Unless I'm mistaken a lot of our logic already works in this way, right? If so, is it accurate to say this is a bug specifically in the handling of SqlUnary (or even certain types of SqlUnary)? Or is this a larger issue in our pipeline?

In a way it would be superset of null semantics

That makes sense to me. It's the same as the situation in pure C# LINQ - an expression tree can have nullable types even where the tree structure doesn't allow actual null values to appear (e.g. because of a previous null check). Type information is independent of actual possible values in a given node.

[smitpatel]

C# LINQ null types are less about if there would be actual null data and more about how generic methods interact.
e.g.
source.Select(e => e.Property).Min() throws error if source is empty and Property is of non-null type.
source.Select(e => (int?)e.Property).Min() returns null if source is empty.
Neither of the result actually care about the fact that if there is an element in source, property may never be null.

The larger issue in pipeline is this,

• How do we interpret if column is non-nullable but user casted it to nullable type. It does not change anything in SQL, just C# semantics in above way. Currently we ignore this during translation phase. So question becomes, does type corresponds to what C# did or what is expected nullability in database?
• If we decide that it is expected nullability in SQL then the maintenance of type becomes complex
  • if you do int + int?, now you need to check both types and use nullable type for binary result.
  • But then TypeMapping also specify CLR types when there are typeof(object). That's where I hit issue that RelationalTypeMapping for double? has clr type of double.
    So what we end up with is, multiple factors just to determine the type of resulting expression. It is not just for SqlUnary but also any expression exception for conditions which are of type bool.

Our logic which is partially correct only worked because we never cared about types. In rare case it can add duplicate columns in projection if types don't match. #20633 would also make it important that we have accurate type.

2nd proposal is about changing semantics of type somewhat. sqlExpression.Type being non-null does not imply anything if we will receive null value or not. We always assume nullable except for ColumnExpression anyway. So Sql tree becomes, every node can take null value even if type is non-null. The type is there because Expression requires it. Otherwise for a properly typeMapped sql tree, the SqlExpression.Type should be same as RelationalTypeMapping.ClrType.

[smitpatel]

For relational providers,
SqlExpression.Type is either reference type or non-nullable value type. So any provider specific expression or a custom expression which is type dependent (e.g. SqlFunctionExpression) should pass in right type.
If any custom expression requires information about if it is can be nullable or not then instead of relying on Type's nullability, it should have a bool property to store the information.