Skip to content

Commit

Permalink
Avoid folding filters to tables in subqueries if the same alias exist…
Browse files Browse the repository at this point in the history 
…s in the outer query
  • Loading branch information
@MarkMpn
MarkMpn committed Nov 2, 2023
1 parent 604586c commit 57af6c5
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 14 deletions.
47 changes: 47 additions & 0 deletions MarkMpn.Sql4Cds.Engine.Tests/ExecutionPlanTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6426,5 +6426,52 @@ WHERE contactid IN (SELECT DISTINCT primarycontactid FROM account WHERE name =
_supportedJoins.Remove(JoinOperator.Any);
}
}

[TestMethod]
public void FoldFilterToCorrectTableAlias()
{
// The same table alias can be used in the main query and in a query-derived table. Ensure filters are
// folded to the correct one.

var planBuilder = new ExecutionPlanBuilder(_localDataSource.Values, this);

var query = @"
SELECT *
FROM (SELECT name FROM account AS a INNER JOIN contact AS c ON a.primarycontactid = c.contactid) AS q
INNER JOIN contact AS c ON q.name = c.fullname
WHERE c.firstname = 'Mark'";

var plans = planBuilder.Build(query, null, out _);

Assert.AreEqual(1, plans.Length);

var select = AssertNode<SelectNode>(plans[0]);
var join = AssertNode<HashJoinNode>(select.Source);
var leftFetch = AssertNode<FetchXmlScan>(join.LeftSource);
var rightFetch = AssertNode<FetchXmlScan>(join.RightSource);

AssertFetchXml(leftFetch, @"
<fetch xmlns:generator='MarkMpn.SQL4CDS'>
<entity name='contact'>
<all-attributes />
<filter>
<condition attribute='fullname' operator='not-null' />
<condition attribute='firstname' operator='eq' value='Mark' />
</filter>
</entity>
</fetch>");

AssertFetchXml(rightFetch, @"
<fetch xmlns:generator='MarkMpn.SQL4CDS'>
<entity name='account'>
<attribute name='name' />
<link-entity name='contact' from='contactid' to='primarycontactid' link-type='inner' alias='c'>
</link-entity>
<filter>
<condition attribute='name' operator='not-null' />
</filter>
</entity>
</fetch>");
}
}
}
17 changes: 11 additions & 6 deletions MarkMpn.Sql4Cds.Engine/ExecutionPlan/BaseDataNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,15 @@ public void MergeStatsFrom(BaseDataNode other)
/// <param name="criteria">The SQL criteria to attempt to translate to FetchXML</param>
/// <param name="schema">The schema of the node that the criteria apply to</param>
/// <param name="allowedPrefix">The prefix of the table that the <paramref name="criteria"/> can be translated for, or <c>null</c> if any tables can be referenced</param>
/// <param name="barredPrefixes">The prefixes of any tables that the <paramref name="criteria"/> cannot be translated for</param>
/// <param name="targetEntityName">The logical name of the root entity that the FetchXML query is targetting</param>
/// <param name="targetEntityAlias">The alias of the root entity that the FetchXML query is targetting</param>
/// <param name="items">The child items of the root entity in the FetchXML query</param>
/// <param name="filter">The FetchXML version of the <paramref name="criteria"/> that is generated by this method</param>
/// <returns><c>true</c> if the <paramref name="criteria"/> can be translated to FetchXML, or <c>false</c> otherwise</returns>
protected bool TranslateFetchXMLCriteria(NodeCompilationContext context, IAttributeMetadataCache metadata, BooleanExpression criteria, INodeSchema schema, string allowedPrefix, string targetEntityName, string targetEntityAlias, object[] items, out filter filter)
protected bool TranslateFetchXMLCriteria(NodeCompilationContext context, IAttributeMetadataCache metadata, BooleanExpression criteria, INodeSchema schema, string allowedPrefix, HashSet<string> barredPrefixes, string targetEntityName, string targetEntityAlias, object[] items, out filter filter)
{
if (!TranslateFetchXMLCriteria(context, metadata, criteria, schema, allowedPrefix, targetEntityName, targetEntityAlias, items, out var condition, out filter))
if (!TranslateFetchXMLCriteria(context, metadata, criteria, schema, allowedPrefix, barredPrefixes, targetEntityName, targetEntityAlias, items, out var condition, out filter))
return false;

if (condition != null)
Expand All @@ -214,22 +215,23 @@ protected bool TranslateFetchXMLCriteria(NodeCompilationContext context, IAttrib
/// <param name="criteria">The SQL criteria to attempt to translate to FetchXML</param>
/// <param name="schema">The schema of the node that the criteria apply to</param>
/// <param name="allowedPrefix">The prefix of the table that the <paramref name="criteria"/> can be translated for, or <c>null</c> if any tables can be referenced</param>
/// <param name="barredPrefixes">The prefixes of any tables that the <paramref name="criteria"/> cannot be translated for</param>
/// <param name="targetEntityName">The logical name of the root entity that the FetchXML query is targetting</param>
/// <param name="targetEntityAlias">The alias of the root entity that the FetchXML query is targetting</param>
/// <param name="items">The child items of the root entity in the FetchXML query</param>
/// <param name="filter">The FetchXML version of the <paramref name="criteria"/> that is generated by this method when it covers multiple conditions</param>
/// <param name="condition">The FetchXML version of the <paramref name="criteria"/> that is generated by this method when it is for a single condition only</param>
/// <returns><c>true</c> if the <paramref name="criteria"/> can be translated to FetchXML, or <c>false</c> otherwise</returns>
private bool TranslateFetchXMLCriteria(NodeCompilationContext context, IAttributeMetadataCache metadata, BooleanExpression criteria, INodeSchema schema, string allowedPrefix, string targetEntityName, string targetEntityAlias, object[] items, out condition condition, out filter filter)
private bool TranslateFetchXMLCriteria(NodeCompilationContext context, IAttributeMetadataCache metadata, BooleanExpression criteria, INodeSchema schema, string allowedPrefix, HashSet<string> barredPrefixes, string targetEntityName, string targetEntityAlias, object[] items, out condition condition, out filter filter)
{
condition = null;
filter = null;

if (criteria is BooleanBinaryExpression binary)
{
if (!TranslateFetchXMLCriteria(context, metadata, binary.FirstExpression, schema, allowedPrefix, targetEntityName, targetEntityAlias, items, out var lhsCondition, out var lhsFilter))
if (!TranslateFetchXMLCriteria(context, metadata, binary.FirstExpression, schema, allowedPrefix, barredPrefixes, targetEntityName, targetEntityAlias, items, out var lhsCondition, out var lhsFilter))
return false;
if (!TranslateFetchXMLCriteria(context, metadata, binary.SecondExpression, schema, allowedPrefix, targetEntityName, targetEntityAlias, items, out var rhsCondition, out var rhsFilter))
if (!TranslateFetchXMLCriteria(context, metadata, binary.SecondExpression, schema, allowedPrefix, barredPrefixes, targetEntityName, targetEntityAlias, items, out var rhsCondition, out var rhsFilter))
return false;

filter = new filter
Expand All @@ -246,7 +248,7 @@ private bool TranslateFetchXMLCriteria(NodeCompilationContext context, IAttribut

if (criteria is BooleanParenthesisExpression paren)
{
return TranslateFetchXMLCriteria(context, metadata, paren.Expression, schema, allowedPrefix, targetEntityName, targetEntityAlias, items, out condition, out filter);
return TranslateFetchXMLCriteria(context, metadata, paren.Expression, schema, allowedPrefix, barredPrefixes, targetEntityName, targetEntityAlias, items, out condition, out filter);
}

if (criteria is DistinctPredicate distinct)
Expand Down Expand Up @@ -469,6 +471,9 @@ private bool TranslateFetchXMLCriteria(NodeCompilationContext context, IAttribut
if (allowedPrefix != null && !allowedPrefix.Equals(entityAlias))
return false;

if (barredPrefixes != null && barredPrefixes.Contains(entityAlias))
return false;

var entityName = AliasToEntityName(targetEntityAlias, targetEntityName, items, entityAlias);

if (IsInvalidAuditFilter(targetEntityName, entityName, items))
Expand Down
10 changes: 7 additions & 3 deletions MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1704,14 +1704,18 @@ public override void AddRequiredColumns(NodeCompilationContext context, IList<st
if (parts.Length != 2)
continue;

var mapping = ColumnMappings.SingleOrDefault(map => map.OutputColumn == normalizedCol);
var mapping = ColumnMappings.SingleOrDefault(map => map.OutputColumn == normalizedCol || map.OutputColumn == parts[0] && map.AllColumns);

if (mapping != null)
if (mapping != null && mapping.AllColumns)
normalizedCol = (mapping.SourceColumn ?? parts[0]).EscapeIdentifier() + "." + parts[1].EscapeIdentifier();
else if (mapping != null)
normalizedCol = mapping.SourceColumn;
else if (HiddenAliases.Contains(parts[0], StringComparer.OrdinalIgnoreCase))
continue;

var attr = AddAttribute(normalizedCol, null, dataSource.Metadata, out _, out var linkEntity);

if (mapping != null && attr != null)
if (mapping != null && !mapping.AllColumns && attr != null)
{
if (attr.name != parts[1] && IsValidAlias(parts[1]))
attr.alias = parts[1];
Expand Down
72 changes: 67 additions & 5 deletions MarkMpn.Sql4Cds.Engine/ExecutionPlan/FilterNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,10 @@ private bool FoldFiltersToDataSources(NodeCompilationContext context, IList<Opti
var foldedFilters = false;

// Find all the data source nodes we could fold this into. Include direct data sources, those from either side of an inner join, or the main side of an outer join
// CTEs or other query-derived tables can contain a table with the same alias as a table in the main query. Identify these and prefer using the table
// from the outer query.
var ignoreAliasesByNode = GetIgnoreAliasesByNode();

foreach (var source in GetFoldableSources(Source))
{
var schema = source.GetSchema(context);
Expand All @@ -1099,7 +1103,7 @@ private bool FoldFiltersToDataSources(NodeCompilationContext context, IList<Opti
throw new NotSupportedQueryFragmentException("Missing datasource " + fetchXml.DataSource);

// If the criteria are ANDed, see if any of the individual conditions can be translated to FetchXML
Filter = ExtractFetchXMLFilters(context, dataSource.Metadata, Filter, schema, null, fetchXml.Entity.name, fetchXml.Alias, fetchXml.Entity.Items, out var fetchFilter);
Filter = ExtractFetchXMLFilters(context, dataSource.Metadata, Filter, schema, null, ignoreAliasesByNode[fetchXml], fetchXml.Entity.name, fetchXml.Alias, fetchXml.Entity.Items, out var fetchFilter);

if (fetchFilter != null)
{
Expand Down Expand Up @@ -1192,6 +1196,64 @@ private bool FoldFiltersToDataSources(NodeCompilationContext context, IList<Opti
return foldedFilters;
}

private Dictionary<FetchXmlScan,HashSet<string>> GetIgnoreAliasesByNode()
{
var fetchXmlSources = GetFoldableSources(Source)
.OfType<FetchXmlScan>()
.ToList();

// Build a full list of where we see each alias
var nodesByAlias = new Dictionary<string, List<FetchXmlScan>>(StringComparer.OrdinalIgnoreCase);

foreach (var fetchXml in fetchXmlSources)
{
foreach (var alias in GetAliases(fetchXml))
{
if (!nodesByAlias.TryGetValue(alias, out var nodes))
{
nodes = new List<FetchXmlScan>();
nodesByAlias.Add(alias, nodes);
}

nodes.Add(fetchXml);
}
}

var ignoreAliases = fetchXmlSources
.ToDictionary(f => f, f => new HashSet<string>(StringComparer.OrdinalIgnoreCase));

// Aliases should be ignored if they appear as visible in another node, or they are hidden in all nodes
foreach (var alias in nodesByAlias)
{
if (alias.Value.Count < 2)
continue;

var hiddenNodes = alias.Value
.Where(n => n.HiddenAliases.Contains(alias.Key, StringComparer.OrdinalIgnoreCase))
.ToList();

var visibleNodes = alias.Value
.Except(hiddenNodes)
.ToList();

foreach (var node in alias.Value)
{
if (visibleNodes.Count == 0 || visibleNodes.Any(n => n != node))
ignoreAliases[node].Add(alias.Key);
}
}

return ignoreAliases;
}

private IEnumerable<string> GetAliases(FetchXmlScan fetchXml)
{
yield return fetchXml.Alias;

foreach (var linkEntity in fetchXml.Entity.GetLinkEntities())
yield return linkEntity.alias;
}

private BooleanExpression ReplaceColumnNames(BooleanExpression filter, Dictionary<ScalarExpression, string> replacements)
{
filter = filter.Clone();
Expand Down Expand Up @@ -1350,9 +1412,9 @@ public override void AddRequiredColumns(NodeCompilationContext context, IList<st
Source.AddRequiredColumns(context, requiredColumns);
}

private BooleanExpression ExtractFetchXMLFilters(NodeCompilationContext context, IAttributeMetadataCache metadata, BooleanExpression criteria, INodeSchema schema, string allowedPrefix, string targetEntityName, string targetEntityAlias, object[] items, out filter filter)
private BooleanExpression ExtractFetchXMLFilters(NodeCompilationContext context, IAttributeMetadataCache metadata, BooleanExpression criteria, INodeSchema schema, string allowedPrefix, HashSet<string> barredPrefixes, string targetEntityName, string targetEntityAlias, object[] items, out filter filter)
{
if (TranslateFetchXMLCriteria(context, metadata, criteria, schema, allowedPrefix, targetEntityName, targetEntityAlias, items, out filter))
if (TranslateFetchXMLCriteria(context, metadata, criteria, schema, allowedPrefix, barredPrefixes, targetEntityName, targetEntityAlias, items, out filter))
return null;

if (!(criteria is BooleanBinaryExpression bin))
Expand All @@ -1361,8 +1423,8 @@ private BooleanExpression ExtractFetchXMLFilters(NodeCompilationContext context,
if (bin.BinaryExpressionType != BooleanBinaryExpressionType.And)
return criteria;

bin.FirstExpression = ExtractFetchXMLFilters(context, metadata, bin.FirstExpression, schema, allowedPrefix, targetEntityName, targetEntityAlias, items, out var lhsFilter);
bin.SecondExpression = ExtractFetchXMLFilters(context, metadata, bin.SecondExpression, schema, allowedPrefix, targetEntityName, targetEntityAlias, items, out var rhsFilter);
bin.FirstExpression = ExtractFetchXMLFilters(context, metadata, bin.FirstExpression, schema, allowedPrefix, barredPrefixes, targetEntityName, targetEntityAlias, items, out var lhsFilter);
bin.SecondExpression = ExtractFetchXMLFilters(context, metadata, bin.SecondExpression, schema, allowedPrefix, barredPrefixes, targetEntityName, targetEntityAlias, items, out var rhsFilter);

filter = (lhsFilter != null && rhsFilter != null) ? new filter { Items = new object[] { lhsFilter, rhsFilter } } : lhsFilter ?? rhsFilter;

Expand Down

0 comments on commit 57af6c5

Please sign in to comment.