Query: Fix Set Operation design

Remove SetOperationType enum
Add support for Intersect/Except Distinct

Resolves #16709

src/EFCore.Relational/Query/Internal/CollectionJoinApplyingExpressionVisitor.cs

@@ -21,7 +21,6 @@ protected override Expression VisitExtension(Expression extensionExpression)
  if (selectExpression.IsDistinct
  || selectExpression.Limit != null
  || selectExpression.Offset != null
- || selectExpression.IsSetOperation
  || selectExpression.GroupBy.Count > 1)
  {
  selectExpression.PushdownIntoSubquery();

src/EFCore.Relational/Query/Internal/NullSemanticsRewritingExpressionVisitor.cs

@@ -73,9 +73,10 @@ private SqlConstantExpression VisitSqlConstantExpression(SqlConstantExpression s
 
  private ColumnExpression VisitColumnExpression(ColumnExpression columnExpression)
  {
+ var newTable = (TableExpressionBase)Visit(columnExpression.Table);
  _isNullable = columnExpression.IsNullable;
 
- return columnExpression;
+ return columnExpression.Update(newTable);
  }
 
  private SqlParameterExpression VisitSqlParameterExpression(SqlParameterExpression sqlParameterExpression)

src/EFCore.Relational/Query/QuerySqlGenerator.cs

@@ -92,38 +92,40 @@ protected override Expression VisitSqlFragment(SqlFragmentExpression sqlFragment
  return sqlFragmentExpression;
  }
 
+ private bool IsNonComposedSetOperation(SelectExpression selectExpression)
+ => selectExpression.Offset == null
+ && selectExpression.Limit == null
+ && !selectExpression.IsDistinct
+ && selectExpression.Predicate == null
+ && selectExpression.Having == null
+ && selectExpression.Orderings.Count == 0
+ && selectExpression.GroupBy.Count == 0
+ && selectExpression.Tables.Count == 1
+ && selectExpression.Tables[0] is SetOperationBase setOperation
+ && selectExpression.Projection.Count == setOperation.Source1.Projection.Count
+ && selectExpression.Projection.Select((pe, index) => pe.Expression is ColumnExpression column
+ && column.Table.Equals(setOperation)
+ && string.Equals(column.Name, setOperation.Source1.Projection[index].Alias, StringComparison.OrdinalIgnoreCase))
+ .All(e => e);
+
  protected override Expression VisitSelect(SelectExpression selectExpression)
  {
- IDisposable subQueryIndent = null;
-
- if (selectExpression.Alias != null)
+ if (IsNonComposedSetOperation(selectExpression))
  {
- _relationalCommandBuilder.AppendLine("(");
- subQueryIndent = _relationalCommandBuilder.Indent();
- }
+ // Naked set operation
+ GenerateSetOperation((SetOperationBase)selectExpression.Tables[0]);
 
- if (selectExpression.IsSetOperation)
- {
- GenerateSetOperation(selectExpression);
- }
- else
- {
- GenerateSelect(selectExpression);
+ return selectExpression;
  }
 
+ IDisposable subQueryIndent = null;
+
  if (selectExpression.Alias != null)
  {
- subQueryIndent.Dispose();
-
- _relationalCommandBuilder.AppendLine()
- .Append(")" + AliasSeparator + _sqlGenerationHelper.DelimitIdentifier(selectExpression.Alias));
+ _relationalCommandBuilder.AppendLine("(");
+ subQueryIndent = _relationalCommandBuilder.Indent();
  }
 
- return selectExpression;
- }
-
- protected virtual void GenerateSelect(SelectExpression selectExpression)
- {
  _relationalCommandBuilder.Append("SELECT ");
 
  if (selectExpression.IsDistinct)
@@ -172,54 +174,16 @@ protected virtual void GenerateSelect(SelectExpression selectExpression)
 
  GenerateOrderings(selectExpression);
  GenerateLimitOffset(selectExpression);
- }
-
- protected virtual void GenerateSetOperation(SelectExpression setOperationExpression)
- {
- Debug.Assert(setOperationExpression.Tables.Count == 2,
- $"{nameof(SelectExpression)} with {setOperationExpression.Tables.Count} tables, must be 2");
-
- static string GenerateSetOperationType(SetOperationType setOperationType)
- => setOperationType switch
- {
- SetOperationType.Union => "UNION",
- SetOperationType.UnionAll => "UNION ALL",
- SetOperationType.Intersect => "INTERSECT",
- SetOperationType.Except => "EXCEPT",
- _ => throw new InvalidOperationException($"Invalid {nameof(SetOperationType)}: {setOperationType}")
- };
-
- GenerateSetOperationOperand(setOperationExpression, (SelectExpression)setOperationExpression.Tables[0]);
-
- _relationalCommandBuilder
- .AppendLine()
- .AppendLine(GenerateSetOperationType(setOperationExpression.SetOperationType));
-
- GenerateSetOperationOperand(setOperationExpression, (SelectExpression)setOperationExpression.Tables[1]);
-
- GenerateOrderings(setOperationExpression);
- GenerateLimitOffset(setOperationExpression);
- }
 
- protected virtual void GenerateSetOperationOperand(
- SelectExpression setOperationExpression,
- SelectExpression operandExpression)
- {
- // INTERSECT has higher precedence over UNION and EXCEPT, but otherwise evaluation is left-to-right.
- // To preserve meaning, add parentheses whenever a set operation is nested within a different set operation.
- if (operandExpression.IsSetOperation
- && operandExpression.SetOperationType != setOperationExpression.SetOperationType)
+ if (selectExpression.Alias != null)
  {
- _relationalCommandBuilder.AppendLine("(");
- using (_relationalCommandBuilder.Indent())
- {
- Visit(operandExpression);
- }
- _relationalCommandBuilder.AppendLine().Append(")");
- return;
+ subQueryIndent.Dispose();
+
+ _relationalCommandBuilder.AppendLine()
+ .Append(")" + AliasSeparator + _sqlGenerationHelper.DelimitIdentifier(selectExpression.Alias));
  }
 
- Visit(operandExpression);
+ return selectExpression;
  }
 
  protected override Expression VisitProjection(ProjectionExpression projectionExpression)
@@ -621,9 +585,7 @@ protected override Expression VisitIn(InExpression inExpression)
  }
 
  protected virtual string GenerateOperator(SqlBinaryExpression binaryExpression)
- {
- return _operatorMap[binaryExpression.OperatorType];
- }
+ => _operatorMap[binaryExpression.OperatorType];
 
  protected virtual void GenerateTop(SelectExpression selectExpression)
  {
@@ -657,8 +619,6 @@ protected virtual void GenerateOrderings(SelectExpression selectExpression)
 
  protected virtual void GenerateLimitOffset(SelectExpression selectExpression)
  {
- // The below implements ISO SQL:2008
-
  if (selectExpression.Offset != null)
  {
  _relationalCommandBuilder.AppendLine()
@@ -779,5 +739,75 @@ protected override Expression VisitRowNumber(RowNumberExpression rowNumberExpres
 
  return rowNumberExpression;
  }
+
+ protected virtual void GenerateSetOperation(SetOperationBase setOperation)
+ {
+ string getSetOperation() => setOperation switch
+ {
+ ExceptExpression _ => "EXCEPT",
+ IntersectExpression _ => "INTERSECT",
+ UnionExpression _ => "UNION",
+ _ => throw new InvalidOperationException("Unknown SetOperationType."),
+ };
+
+ GenerateSetOperationOperand(setOperation, setOperation.Source1);
+ _relationalCommandBuilder.AppendLine();
+ _relationalCommandBuilder.AppendLine($"{getSetOperation()}{(setOperation.IsDistinct ? "" : " ALL")}");
+ GenerateSetOperationOperand(setOperation, setOperation.Source2);
+ }
+
+ protected virtual void GenerateSetOperationOperand(SetOperationBase setOperation, SelectExpression operand)
+ {
+ // INTERSECT has higher precedence over UNION and EXCEPT, but otherwise evaluation is left-to-right.
+ // To preserve meaning, add parentheses whenever a set operation is nested within a different set operation.
+ if (IsNonComposedSetOperation(operand)
+ && operand.Tables[0].GetType() != setOperation.GetType())
+ {
+ _relationalCommandBuilder.AppendLine("(");
+ using (_relationalCommandBuilder.Indent())
+ {
+ Visit(operand);
+ }
+ _relationalCommandBuilder.AppendLine().Append(")");
+ }
+ else
+ {
+ Visit(operand);
+ }
+ }
+
+ private void GenerateSetOperationHelper(SetOperationBase setOperation)
+ {
+ _relationalCommandBuilder.AppendLine("(");
+ using (_relationalCommandBuilder.Indent())
+ {
+ GenerateSetOperation(setOperation);
+ }
+ _relationalCommandBuilder.AppendLine()
+ .Append(")")
+ .Append(AliasSeparator)
+ .Append(_sqlGenerationHelper.DelimitIdentifier(setOperation.Alias));
+ }
+
+ protected override Expression VisitExcept(ExceptExpression exceptExpression)
+ {
+ GenerateSetOperationHelper(exceptExpression);
+
+ return exceptExpression;
+ }
+
+ protected override Expression VisitIntersect(IntersectExpression intersectExpression)
+ {
+ GenerateSetOperationHelper(intersectExpression);
+
+ return intersectExpression;
+ }
+
+ protected override Expression VisitUnion(UnionExpression unionExpression)
+ {
+ GenerateSetOperationHelper(unionExpression);
+
+ return unionExpression;
+ }
  }
 }

src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

@@ -183,9 +183,8 @@ protected override ShapedQueryExpression TranslateCast(ShapedQueryExpression sou
 
  protected override ShapedQueryExpression TranslateConcat(ShapedQueryExpression source1, ShapedQueryExpression source2)
  {
- var operand1 = (SelectExpression)source1.QueryExpression;
- var operand2 = (SelectExpression)source2.QueryExpression;
- source1.ShaperExpression = operand1.ApplySetOperation(SetOperationType.UnionAll, operand2, source1.ShaperExpression);
+ ((SelectExpression)source1.QueryExpression).ApplyUnion((SelectExpression)source2.QueryExpression, distinct: false);
+
  return source1;
  }
 
@@ -262,9 +261,7 @@ protected override ShapedQueryExpression TranslateElementAtOrDefault(ShapedQuery
 
  protected override ShapedQueryExpression TranslateExcept(ShapedQueryExpression source1, ShapedQueryExpression source2)
  {
- var operand1 = (SelectExpression)source1.QueryExpression;
- var operand2 = (SelectExpression)source2.QueryExpression;
- source1.ShaperExpression = operand1.ApplySetOperation(SetOperationType.Except, operand2, source1.ShaperExpression);
+ ((SelectExpression)source1.QueryExpression).ApplyExcept((SelectExpression)source2.QueryExpression, distinct: true);
  return source1;
  }
 
@@ -444,9 +441,7 @@ protected override ShapedQueryExpression TranslateGroupJoin(ShapedQueryExpressio
 
  protected override ShapedQueryExpression TranslateIntersect(ShapedQueryExpression source1, ShapedQueryExpression source2)
  {
- var operand1 = (SelectExpression)source1.QueryExpression;
- var operand2 = (SelectExpression)source2.QueryExpression;
- source1.ShaperExpression = operand1.ApplySetOperation(SetOperationType.Intersect, operand2, source1.ShaperExpression);
+ ((SelectExpression)source1.QueryExpression).ApplyIntersect((SelectExpression)source2.QueryExpression, distinct: true);
  return source1;
  }
 
@@ -937,9 +932,7 @@ protected override ShapedQueryExpression TranslateThenBy(ShapedQueryExpression s
 
  protected override ShapedQueryExpression TranslateUnion(ShapedQueryExpression source1, ShapedQueryExpression source2)
  {
- var operand1 = (SelectExpression)source1.QueryExpression;
- var operand2 = (SelectExpression)source2.QueryExpression;
- source1.ShaperExpression = operand1.ApplySetOperation(SetOperationType.Union, operand2, source1.ShaperExpression);
+ ((SelectExpression)source1.QueryExpression).ApplyUnion((SelectExpression)source2.QueryExpression, distinct: true);
  return source1;
  }
 

src/EFCore.Relational/Query/SqlExpressionFactory.cs

@@ -500,7 +500,7 @@ private void AddConditions(
  var otherSelectExpression = new SelectExpression(entityType);
 
  AddInnerJoin(otherSelectExpression, otherFk, sharingTypes, skipInnerJoins: false);
- selectExpression.ApplySetOperation(SetOperationType.Union, otherSelectExpression, null);
+ selectExpression.ApplyUnion(otherSelectExpression, distinct: true);
  }
  }
  }
@@ -629,7 +629,7 @@ private void AddOptionalDependentConditions(
  AddInnerJoin(otherSelectExpression, referencingFk,
  sameTable ? sharingTypes : null,
  skipInnerJoins: sameTable);
- selectExpression.ApplySetOperation(SetOperationType.Union, otherSelectExpression, null);
+ selectExpression.ApplyUnion(otherSelectExpression, distinct: true);
  }
  }
  }

src/EFCore.Relational/Query/SqlExpressionVisitor.cs

@@ -80,12 +80,24 @@ protected override Expression VisitExtension(Expression extensionExpression)
 
  case TableExpression tableExpression:
  return VisitTable(tableExpression);
+
+ case ExceptExpression exceptExpression:
+ return VisitExcept(exceptExpression);
+
+ case IntersectExpression intersectExpression:
+ return VisitIntersect(intersectExpression);
+
+ case UnionExpression unionExpression:
+ return VisitUnion(unionExpression);
  }
 
  return base.VisitExtension(extensionExpression);
  }
 
  protected abstract Expression VisitRowNumber(RowNumberExpression rowNumberExpression);
+ protected abstract Expression VisitExcept(ExceptExpression exceptExpression);
+ protected abstract Expression VisitIntersect(IntersectExpression intersectExpression);
+ protected abstract Expression VisitUnion(UnionExpression unionExpression);
  protected abstract Expression VisitExists(ExistsExpression existsExpression);
  protected abstract Expression VisitIn(InExpression inExpression);
  protected abstract Expression VisitCrossJoin(CrossJoinExpression crossJoinExpression);

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

@@ -51,13 +51,12 @@ private ColumnExpression(string name, TableExpressionBase table, Type type, Rela
  public bool IsNullable { get; }
 
  protected override Expression VisitChildren(ExpressionVisitor visitor)
- {
- var newTable = (TableExpressionBase)visitor.Visit(Table);
+ => Update((TableExpressionBase)visitor.Visit(Table));
 
- return newTable != Table
- ? new ColumnExpression(Name, newTable, Type, TypeMapping, IsNullable)
+ public virtual ColumnExpression Update(TableExpressionBase table)
+ => table != Table
+ ? new ColumnExpression(Name, table, Type, TypeMapping, IsNullable)
  : this;
- }
 
  public ColumnExpression MakeNullable()
  => new ColumnExpression(Name, Table, Type.MakeNullable(), TypeMapping, true);