Skip to content

Commit

Permalink
Query: Fix Set Operation design
Browse files Browse the repository at this point in the history 
Remove SetOperationType enum
Add support for Intersect/Except Distinct

Resolves #16709
  • Loading branch information
@smitpatel
smitpatel committed Aug 21, 2019
1 parent a6986d6 commit 42876b2
Show file tree
Hide file tree
Showing 19 changed files with 569 additions and 303 deletions.
1 change: 0 additions & 1 deletion src/EFCore.Relational/Query/Internal/CollectionJoinApplyingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore.Relational/Query/Internal/NullSemanticsRewritingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
172 changes: 101 additions & 71 deletions src/EFCore.Relational/Query/QuerySqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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;
}
}
}
17 changes: 5 additions & 12 deletions src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Query/SqlExpressionFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions src/EFCore.Relational/Query/SqlExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 4 additions & 5 deletions src/EFCore.Relational/Query/SqlExpressions/ColumnExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 42876b2

Please sign in to comment.