Skip to content

Commit

Permalink
UNIONing projection with a numeric column to another projection that …
Browse files Browse the repository at this point in the history
…specifically projects NULL the the same name as a numeric field, the JOINING that to another table can leave that numeric field as another data type (NULL is not a numeric data type). Hence when using that supposedly numeric field in the join predicat against another numeric field we get a data type mismatch.

We try to detect this and when visiting the column (but only if we are within a binary expression and not a projection) we quickly wrap it in a convert function to make it properly numeric

The top/outer projection of this field can still be in a non numeric format, but JetDataReader is able to handle that. Would be better to produce the correctly converted data output anyway
  • Loading branch information
ChrisJollyAU committed Oct 2, 2023
1 parent cae7f83 commit 5b591b7
Showing 1 changed file with 29 additions and 4 deletions.
33 changes: 29 additions & 4 deletions src/EFCore.Jet/Query/Sql/Internal/JetQuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public class JetQuerySqlGenerator : QuerySqlGenerator, IJetExpressionVisitor

private readonly ISqlGenerationHelper _sqlGenerationHelper;
//private readonly JetSqlExpressionFactory _sqlExpressionFactory;
private List<string> _nullNumerics = new List<string>();
private Stack<Expression> parent = new Stack<Expression>();
private CoreTypeMapping? _boolTypeMapping;
/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
Expand Down Expand Up @@ -315,9 +317,30 @@ private List<ColumnExpression> ExtractColumnExpressions(SqlBinaryExpression bina
return result;
}

protected override Expression VisitInnerJoin(InnerJoinExpression innerJoinExpression)
protected override Expression VisitProjection(ProjectionExpression projectionExpression)
{
return base.VisitInnerJoin(innerJoinExpression);
if (projectionExpression.Expression is SqlConstantExpression { Value: null } constantExpression && (constantExpression.Type == typeof(int) || constantExpression.Type == typeof(double) || constantExpression.Type == typeof(float) || constantExpression.Type == typeof(decimal) || constantExpression.Type == typeof(short)))
{
_nullNumerics.Add(projectionExpression.Alias);
}
return base.VisitProjection(projectionExpression);
}

protected override Expression VisitColumn(ColumnExpression columnExpression)
{
if (columnExpression.IsNullable && _nullNumerics.Contains(columnExpression.Name) && _convertMappings.TryGetValue(columnExpression.Type.Name, out var function))
{

if (parent.TryPeek(out var exp) && exp is SqlBinaryExpression)
{
Sql.Append(function);
Sql.Append("(");
base.VisitColumn(columnExpression);
Sql.Append(")");
return columnExpression;
}
}
return base.VisitColumn(columnExpression);
}

private bool IsNonComposedSetOperation(SelectExpression selectExpression)
Expand Down Expand Up @@ -411,8 +434,10 @@ protected override Expression VisitSqlBinary(SqlBinaryExpression sqlBinaryExpres
Visit(caseexp);
return sqlBinaryExpression;
}

return base.VisitSqlBinary(sqlBinaryExpression);
parent.Push(sqlBinaryExpression);
var res = base.VisitSqlBinary(sqlBinaryExpression);
parent.Pop();
return res;
}

protected override void GenerateIn(InExpression inExpression, bool negated)
Expand Down

0 comments on commit 5b591b7

Please sign in to comment.