Query: Consistency issues

src/EFCore.Relational/Query/ISqlExpressionFactory.cs

@@ -565,6 +565,7 @@ SqlFunctionExpression Function(
         /// <param name="sql"> A custom SQL for the table source. </param>
         /// <param name="sqlArguments"> An expression representing parameters passed to the custom SQL. </param>
         /// <returns> An expression representing a SELECT in a SQL tree. </returns>
+        [Obsolete("Use overload which takes TableExpressionBase by passing FromSqlExpression directly.")]
         SelectExpression Select([NotNull] IEntityType entityType, [NotNull] string sql, [NotNull] Expression sqlArguments);
     }
 }

src/EFCore.Relational/Query/NullabilityBasedSqlProcessingExpressionVisitor.cs

@@ -152,22 +152,17 @@ protected override Expression VisitCase(CaseExpression caseExpression)
             // - if there is no Else block, return null
             if (whenClauses.Count == 0)
             {
-                return elseResult == null
-                    ? SqlExpressionFactory.Constant(null, caseExpression.TypeMapping)
-                    : elseResult;
+                return elseResult ?? SqlExpressionFactory.Constant(null, caseExpression.TypeMapping);
             }
 
             // if there is only one When clause and it's test evaluates to 'true' AND there is no else block, simply return the result
-            if (elseResult == null
+            return elseResult == null
                 && whenClauses.Count == 1
                 && whenClauses[0].Test is SqlConstantExpression singleTestConstant
                 && singleTestConstant.Value is bool boolConstant
-                && boolConstant)
-            {
-                return whenClauses[0].Result;
-            }
-
-            return caseExpression.Update(operand, whenClauses, elseResult);
+                && boolConstant
+                ? whenClauses[0].Result
+                : caseExpression.Update(operand, whenClauses, elseResult);
         }
 
         protected override Expression VisitCollate(CollateExpression collateExpresion)
@@ -805,14 +800,11 @@ private SqlExpression OptimizeComparison(
 
             return sqlBinaryExpression.Update(left, right);
 
-            bool? IsTrueOrFalse(SqlExpression sqlExpression)
+            static bool? IsTrueOrFalse(SqlExpression sqlExpression)
             {
-                if (sqlExpression is SqlConstantExpression sqlConstantExpression && sqlConstantExpression.Value is bool boolConstant)
-                {
-                    return boolConstant;
-                }
-
-                return null;
+                return sqlExpression is SqlConstantExpression sqlConstantExpression && sqlConstantExpression.Value is bool boolConstant
+                    ? boolConstant
+                    : (bool?)null;
             }
         }
 
@@ -941,10 +933,8 @@ private SqlExpression RewriteNullSemantics(
 
         private SqlExpression SimplifyLogicalSqlBinaryExpression(SqlBinaryExpression sqlBinaryExpression)
         {
-            var leftUnary = sqlBinaryExpression.Left as SqlUnaryExpression;
-            var rightUnary = sqlBinaryExpression.Right as SqlUnaryExpression;
-            if (leftUnary != null
-                && rightUnary != null
+            if (sqlBinaryExpression.Left is SqlUnaryExpression leftUnary
+                && sqlBinaryExpression.Right is SqlUnaryExpression rightUnary
                 && (leftUnary.OperatorType == ExpressionType.Equal || leftUnary.OperatorType == ExpressionType.NotEqual)
                 && (rightUnary.OperatorType == ExpressionType.Equal || rightUnary.OperatorType == ExpressionType.NotEqual)
                 && leftUnary.Operand.Equals(rightUnary.Operand))

src/EFCore.Relational/Query/QuerySqlGenerator.cs

@@ -449,7 +449,7 @@ protected override Expression VisitSqlBinary(SqlBinaryExpression sqlBinaryExpres
                 _relationalCommandBuilder.Append(")");
             }
 
-            _relationalCommandBuilder.Append(GenerateOperator(sqlBinaryExpression));
+            _relationalCommandBuilder.Append(GetOperator(sqlBinaryExpression));
 
             requiresBrackets = RequiresBrackets(sqlBinaryExpression.Right);
 
@@ -730,13 +730,26 @@ protected override Expression VisitIn(InExpression inExpression)
         /// </summary>
         /// <param name="binaryExpression"> A SQL binary operation. </param>
         /// <returns> A string representation of the binary operator. </returns>
+        [Obsolete("Use GetOperator instead.")]
         protected virtual string GenerateOperator([NotNull] SqlBinaryExpression binaryExpression)
         {
             Check.NotNull(binaryExpression, nameof(binaryExpression));
 
             return _operatorMap[binaryExpression.OperatorType];
         }
 
+        /// <summary>
+        ///     Gets a SQL operator for a SQL binary operation.
+        /// </summary>
+        /// <param name="binaryExpression"> A SQL binary operation. </param>
+        /// <returns> A string representation of the binary operator. </returns>
+        protected virtual string GetOperator([NotNull] SqlBinaryExpression binaryExpression)
+        {
+            Check.NotNull(binaryExpression, nameof(binaryExpression));
+
+            return _operatorMap[binaryExpression.OperatorType];
+        }
+
         /// <summary>
         ///     Generates a TOP construct in the relational command
         /// </summary>

src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

@@ -87,8 +87,11 @@ protected override Expression VisitExtension(Expression extensionExpression)
                         fromSqlQueryRootExpression.EntityType,
                         _sqlExpressionFactory.Select(
                             fromSqlQueryRootExpression.EntityType,
-                            fromSqlQueryRootExpression.Sql,
-                            fromSqlQueryRootExpression.Argument));
+                            new FromSqlExpression(
+                                (fromSqlQueryRootExpression.EntityType.GetViewOrTableMappings().SingleOrDefault()?.Table.Name
+                                ?? fromSqlQueryRootExpression.EntityType.ShortName()).Substring(0, 1).ToLower(),
+                                fromSqlQueryRootExpression.Sql,
+                                fromSqlQueryRootExpression.Argument)));
 
                 case TableValuedFunctionQueryRootExpression tableValuedFunctionQueryRootExpression:
                     var function = tableValuedFunctionQueryRootExpression.Function;

src/EFCore.Relational/Query/SqlExpressionFactory.cs

@@ -37,26 +37,22 @@ public SqlExpressionFactory([NotNull] SqlExpressionFactoryDependencies dependenc
         /// <inheritdoc />
         public virtual SqlExpression ApplyDefaultTypeMapping(SqlExpression sqlExpression)
         {
-            if (sqlExpression == null
-                || sqlExpression.TypeMapping != null)
-            {
-                return sqlExpression;
-            }
-
-            if (sqlExpression is SqlUnaryExpression sqlUnaryExpression
-                && sqlUnaryExpression.OperatorType == ExpressionType.Convert
-                && sqlUnaryExpression.Type == typeof(object))
-            {
-                return sqlUnaryExpression.Operand;
-            }
-
-            return ApplyTypeMapping(sqlExpression, _typeMappingSource.FindMapping(sqlExpression.Type));
+            return sqlExpression == null
+                || sqlExpression.TypeMapping != null
+                ? sqlExpression
+                : sqlExpression is SqlUnaryExpression sqlUnaryExpression
+                    && sqlUnaryExpression.OperatorType == ExpressionType.Convert
+                    && sqlUnaryExpression.Type == typeof(object)
+                    ? sqlUnaryExpression.Operand
+                    : ApplyTypeMapping(sqlExpression, _typeMappingSource.FindMapping(sqlExpression.Type));
         }
 
         /// <inheritdoc />
         public virtual SqlExpression ApplyTypeMapping(SqlExpression sqlExpression, RelationalTypeMapping typeMapping)
         {
+#pragma warning disable IDE0046 // Convert to conditional expression
             if (sqlExpression == null
+#pragma warning restore IDE0046 // Convert to conditional expression
                 || sqlExpression.TypeMapping != null)
             {
                 return sqlExpression;
@@ -404,7 +400,7 @@ public virtual SqlFunctionExpression Coalesce(SqlExpression left, SqlExpression
                 typeMappedArguments,
                 nullable: true,
                 // COALESCE is handled separately since it's only nullable if *both* arguments are null
-                argumentsPropagateNullability: new[] { false, false},
+                argumentsPropagateNullability: new[] { false, false },
                 resultType,
                 inferredTypeMapping);
         }
@@ -762,14 +758,15 @@ public virtual SelectExpression Select(IEntityType entityType, TableExpressionBa
         }
 
         /// <inheritdoc />
+        [Obsolete("Use overload which takes TableExpressionBase by passing FromSqlExpression directly.")]
         public virtual SelectExpression Select(IEntityType entityType, string sql, Expression sqlArguments)
         {
             Check.NotNull(entityType, nameof(entityType));
             Check.NotNull(sql, nameof(sql));
 
-            var tableExpression = new FromSqlExpression(sql, sqlArguments,
-                (entityType.GetViewOrTableMappings().SingleOrDefault()?.Table.Name
-                ?? entityType.ShortName()).Substring(0, 1).ToLower());
+            var tableExpression = new FromSqlExpression(
+                (entityType.GetViewOrTableMappings().SingleOrDefault()?.Table.Name ?? entityType.ShortName()).Substring(0, 1).ToLower(),
+                sql, sqlArguments);
             var selectExpression = new SelectExpression(entityType, tableExpression);
             AddConditions(selectExpression, entityType);
 

src/EFCore.Relational/Query/SqlExpressions/FromSqlExpression.cs

@@ -17,10 +17,31 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions
     ///         not used in application code.
     ///     </para>
     /// </summary>
-    // Class is sealed because there are no public/protected constructors. Can be unsealed if this is changed.
-    public sealed class FromSqlExpression : TableExpressionBase
+    public class FromSqlExpression : TableExpressionBase
     {
-        internal FromSqlExpression([NotNull] string sql, [NotNull] Expression arguments, [NotNull] string alias)
+        /// <summary>
+        ///     Creates a new instance of the <see cref="FromSqlExpression" /> class.
+        /// </summary>
+        /// <param name="sql"> A user-provided custom SQL for the table source. </param>
+        /// <param name="arguments"> A user-provided parameters to pass to the custom SQL. </param>
+        /// <param name="alias"> A string alias for the table source. </param>
+        [Obsolete("Use the constructor which takes alias as first argument.")]
+        public FromSqlExpression([NotNull] string sql, [NotNull] Expression arguments, [NotNull] string alias)
+            : base(alias)
+        {
+            Check.NotEmpty(sql, nameof(sql));
+            Check.NotNull(arguments, nameof(arguments));
+
+            Sql = sql;
+            Arguments = arguments;
+        }
+        /// <summary>
+        ///     Creates a new instance of the <see cref="FromSqlExpression" /> class.
+        /// </summary>
+        /// <param name="alias"> A string alias for the table source. </param>
+        /// <param name="sql"> A user-provided custom SQL for the table source. </param>
+        /// <param name="arguments"> A user-provided parameters to pass to the custom SQL. </param>
+        public FromSqlExpression([NotNull] string alias, [NotNull] string sql, [NotNull] Expression arguments)
             : base(alias)
         {
             Check.NotEmpty(sql, nameof(sql));
@@ -33,24 +54,24 @@ internal FromSqlExpression([NotNull] string sql, [NotNull] Expression arguments,
         /// <summary>
         ///     The user-provided custom SQL for the table source.
         /// </summary>
-        public string Sql { get; }
+        public virtual string Sql { get; }
         /// <summary>
         ///     The user-provided parameters passed to the custom SQL.
         /// </summary>
-        public Expression Arguments { get; }
+        public virtual Expression Arguments { get; }
 
         /// <summary>
         ///     Creates a new expression that is like this one, but using the supplied children. If all of the children are the same, it will
         ///     return this expression.
         /// </summary>
         /// <param name="arguments"> The <see cref="Arguments"/> property of the result. </param>
         /// <returns> This expression if no children changed, or an expression with the updated children. </returns>
-        public FromSqlExpression Update([NotNull] Expression arguments)
+        public virtual FromSqlExpression Update([NotNull] Expression arguments)
         {
             Check.NotNull(arguments, nameof(arguments));
 
             return arguments != Arguments
-                ? new FromSqlExpression(Sql, arguments, Alias)
+                ? new FromSqlExpression(Alias, Sql, arguments)
                 : this;
         }
 

src/EFCore.Relational/Query/SqlExpressions/TableValuedFunctionExpression.cs

@@ -19,9 +19,6 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions
     ///         not used in application code.
     ///     </para>
     /// </summary>
-    /// <summary>
-    ///     Represents a SQL Table Valued Fuction in the sql generation tree.
-    /// </summary>
     public class TableValuedFunctionExpression : TableExpressionBase
     {
         /// <summary>

src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs

@@ -34,14 +34,14 @@ public SqliteQuerySqlGenerator([NotNull] QuerySqlGeneratorDependencies dependenc
         ///     any release. You should only use it directly in your code with extreme caution and knowing that
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
-        protected override string GenerateOperator(SqlBinaryExpression binaryExpression)
+        protected override string GetOperator(SqlBinaryExpression binaryExpression)
         {
             Check.NotNull(binaryExpression, nameof(binaryExpression));
 
             return binaryExpression.OperatorType == ExpressionType.Add
                 && binaryExpression.Type == typeof(string)
                     ? " || "
-                    : base.GenerateOperator(binaryExpression);
+                    : base.GetOperator(binaryExpression);
         }
 
         /// <summary>