Skip to content

Commit

Permalink
Reuse same alias logic for FetchXmlScan and Select column mappings.
Browse files Browse the repository at this point in the history 
Remove unnecessary column mappings
Fixes #523
  • Loading branch information
@MarkMpn
MarkMpn committed Aug 19, 2024
1 parent 1acc7fc commit 36d83ba
Show file tree
Hide file tree
Showing 3 changed files with 230 additions and 117 deletions.
96 changes: 80 additions & 16 deletions MarkMpn.Sql4Cds.Engine.Tests/ExecutionPlanTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2681,10 +2681,6 @@ FROM contact
Assert.AreEqual("a.fname", select.ColumnSet[1].SourceColumn);
Assert.AreEqual("a.lname", select.ColumnSet[2].SourceColumn);
var fetch = AssertNode<FetchXmlScan>(select.Source);
Assert.AreEqual("a.fname", fetch.ColumnMappings[0].SourceColumn);
Assert.AreEqual("a.fname", fetch.ColumnMappings[0].OutputColumn);
Assert.AreEqual("a.lname", fetch.ColumnMappings[1].SourceColumn);
Assert.AreEqual("a.lname", fetch.ColumnMappings[1].OutputColumn);
AssertFetchXml(fetch, @"
<fetch>
<entity name='account'>
Expand Down Expand Up @@ -2727,12 +2723,8 @@ FROM contact
Assert.AreEqual("a.fname", select.ColumnSet[1].SourceColumn);
Assert.AreEqual("a.lname", select.ColumnSet[2].SourceColumn);
var fetch = AssertNode<FetchXmlScan>(select.Source);
Assert.AreEqual("a.fname", fetch.ColumnMappings[0].SourceColumn);
Assert.AreEqual("a.fname", fetch.ColumnMappings[0].OutputColumn);
Assert.AreEqual("a.lname", fetch.ColumnMappings[1].SourceColumn);
Assert.AreEqual("a.lname", fetch.ColumnMappings[1].OutputColumn);
Assert.AreEqual("systemuser.uname", fetch.ColumnMappings[2].SourceColumn);
Assert.AreEqual("a.uname", fetch.ColumnMappings[2].OutputColumn);
Assert.AreEqual("systemuser.uname", fetch.ColumnMappings[0].SourceColumn);
Assert.AreEqual("a.uname", fetch.ColumnMappings[0].OutputColumn);
AssertFetchXml(fetch, @"
<fetch>
<entity name='account'>
Expand Down Expand Up @@ -6974,12 +6966,10 @@ systemuser AS s
Assert.AreEqual("b_s", fetch2.Alias);
CollectionAssert.Contains(fetch2.HiddenAliases, "b_s");
CollectionAssert.Contains(fetch2.HiddenAliases, "s");
Assert.AreEqual("b_s.name", fetch2.ColumnMappings[0].SourceColumn);
Assert.AreEqual("b_s.name", fetch2.ColumnMappings[0].OutputColumn);
Assert.AreEqual("s.systemuserid", fetch2.ColumnMappings[1].SourceColumn);
Assert.AreEqual("b_s.systemuserid", fetch2.ColumnMappings[1].OutputColumn);
Assert.AreEqual("s.msdyn_agentType", fetch2.ColumnMappings[2].SourceColumn);
Assert.AreEqual("b_s.msdyn_agentType", fetch2.ColumnMappings[2].OutputColumn);
Assert.AreEqual("s.systemuserid", fetch2.ColumnMappings[0].SourceColumn);
Assert.AreEqual("b_s.systemuserid", fetch2.ColumnMappings[0].OutputColumn);
Assert.AreEqual("s.msdyn_agentType", fetch2.ColumnMappings[1].SourceColumn);
Assert.AreEqual("b_s.msdyn_agentType", fetch2.ColumnMappings[1].OutputColumn);
AssertFetchXml(fetch2, @"
<fetch xmlns:generator='MarkMpn.SQL4CDS'>
<entity name='account'>
Expand Down Expand Up @@ -8288,5 +8278,79 @@ FROM account
var streamAggregate = AssertNode<StreamAggregateNode>(tryCatch1.CatchSource);
var fetch3 = AssertNode<FetchXmlScan>(streamAggregate.Source);
}

[TestMethod]
public void OptionSetNameFromQueryDefinedTable()
{
var query = @"
SELECT a.new_customentityid,
a.new_optionsetvaluename AS [a_new_optionsetvaluename],
a.new_optionsetvalue AS [a_new_optionsetvalue]
FROM (SELECT new_customentityid,
new_optionsetvaluename,
new_optionsetvalue
FROM new_customentity
WHERE new_name IN ('test')) AS a
ORDER BY a.new_customentityid";

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

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

Assert.AreEqual(1, plans.Length);

var select = AssertNode<SelectNode>(plans[0]);
var fetch = AssertNode<FetchXmlScan>(select.Source);
AssertFetchXml(fetch, @"
<fetch>
<entity name='new_customentity'>
<attribute name='new_customentityid' />
<attribute name='new_optionsetvalue' />
<filter>
<condition attribute='new_name' operator='in'>
<value>test</value>
</condition>
</filter>
<order attribute='new_customentityid' />
</entity>
</fetch>");
}

[TestMethod]
public void OptionSetNameFromQueryDefinedTableWithAlias()
{
var query = @"
SELECT a.new_customentityid,
a.new_optionsetvaluename AS [a_new_optionsetvaluename],
a.x AS [a_new_optionsetvalue]
FROM (SELECT new_customentityid,
new_optionsetvaluename,
new_optionsetvalue AS x
FROM new_customentity
WHERE new_name IN ('test')) AS a
ORDER BY a.new_customentityid";

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

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

Assert.AreEqual(1, plans.Length);

var select = AssertNode<SelectNode>(plans[0]);
var fetch = AssertNode<FetchXmlScan>(select.Source);
AssertFetchXml(fetch, @"
<fetch>
<entity name='new_customentity'>
<attribute name='new_customentityid' />
<attribute name='new_optionsetvalue' />
<filter>
<condition attribute='new_name' operator='in'>
<value>test</value>
</condition>
</filter>
<order attribute='new_customentityid' />
</entity>
</fetch>");
}
}
}
155 changes: 149 additions & 6 deletions MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1999,17 +1999,52 @@ public override void AddRequiredColumns(NodeCompilationContext context, IList<st
else if (HiddenAliases.Contains(parts[0], StringComparer.OrdinalIgnoreCase))
continue;

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

if (mapping != null && !mapping.AllColumns && attr != null)
{
if (attr.name != parts[1] && IsValidAlias(parts[1]))
attr.alias = parts[1];
// Apply any aliases where possible. Remove each mapping first so they are not ignored by
// the AddAliases method. Alter the output column to the second part of the alias, e.g.
// if mapping a.firstname to a.fname, the requested alias should be simply "fname" for AddAliases
// to work.
var mappings = new Dictionary<SelectColumn,int>();

for (var i = ColumnMappings.Count - 1; i >= 0; i--)
{
var mapping = ColumnMappings[i];

if (mapping.SourceColumn == null || mapping.OutputColumn == null)
continue;

var sourceParts = mapping.SourceColumn.SplitMultiPartIdentifier();
var outputParts = mapping.OutputColumn.SplitMultiPartIdentifier();

mapping.SourceColumn = (linkEntity?.alias ?? Alias).EscapeIdentifier() + "." + (attr.alias ?? attr.name).EscapeIdentifier();
if (sourceParts.Length == 2 &&
outputParts.Length == 2 &&
sourceParts[0].Equals(outputParts[0], StringComparison.OrdinalIgnoreCase))
{
ColumnMappings.RemoveAt(i);
mappings.Add(new SelectColumn
{
SourceColumn = mapping.SourceColumn,
OutputColumn = outputParts[1],
SourceExpression = mapping.SourceExpression
}, i);
}
}

AddAliases(mappings.Keys.ToList(), schema, dataSource.Metadata);

foreach (var mapping in mappings.OrderBy(kvp => kvp.Value))
{
var sourceParts = mapping.Key.SourceColumn.SplitMultiPartIdentifier();
var outputParts = mapping.Key.OutputColumn.SplitMultiPartIdentifier();

if (sourceParts.Length == 2 && outputParts.Length == 1)
mapping.Key.OutputColumn = sourceParts[0] + "." + outputParts[0];

ColumnMappings.Insert(mapping.Value, mapping.Key);
}

// If there is no attribute requested the server will return everything instead of nothing, so
// add the primary key in to limit it
if ((!FetchXml.aggregate || !FetchXml.aggregateSpecified) && !HasAttribute(Entity.Items) && !Entity.GetLinkEntities().Any(link => HasAttribute(link.Items)))
Expand Down Expand Up @@ -2047,6 +2082,107 @@ public override void AddRequiredColumns(NodeCompilationContext context, IList<st
SetDefaultPageSize(context);
}

public void AddAliases(List<SelectColumn> columnSet, INodeSchema schema, IAttributeMetadataCache metadata)
{
var aliasStars = new HashSet<string>(Entity.GetLinkEntities().Where(le => le.Items != null && le.Items.OfType<allattributes>().Any()).Select(le => le.alias), StringComparer.OrdinalIgnoreCase);
if (Entity.Items != null && Entity.Items.OfType<allattributes>().Any())
aliasStars.Add(Alias);

// Check what aliases we can fold down to the FetchXML.
// Ignore:
// 1. columns that have more than 1 alias
// 2. aliases that are invalid for FetchXML
// 3. attributes that are included via an <all-attributes/>
// 4. virtual ___name or ___type attributes
var aliasedColumns = columnSet
.Where(c => !c.AllColumns)
.Select(c =>
{
var sourceCol = c.SourceColumn;
schema.ContainsColumn(sourceCol, out sourceCol);

return new { Mapping = c, SourceColumn = sourceCol, Alias = c.OutputColumn };
})
.Select(c =>
{
// Check which underlying attribute the data is coming from, handling virtual attributes
var parts = c.SourceColumn.SplitMultiPartIdentifier();
var entityName = Entity.name;
var attrName = parts.Last();

if (parts.Length > 1 && !parts[0].Equals(Alias))
entityName = Entity.FindLinkEntity(parts[0])?.name;

if (entityName == null)
return null;

var meta = metadata[entityName].Attributes.SingleOrDefault(a => a.LogicalName.Equals(attrName, StringComparison.OrdinalIgnoreCase) && a.AttributeOf == null);
var isVirtual = false;
if (meta == null)
{
meta = metadata[entityName].FindBaseAttributeFromVirtualAttribute(attrName, out _);
if (meta != null)
isVirtual = true;
}

return new { c.Mapping, c.SourceColumn, c.Alias, meta?.LogicalName, IsVirtual = isVirtual };
})
.Where(c => c?.LogicalName != null) // Ignore attributes we can't find in the metadata
.GroupBy(c => c.LogicalName, StringComparer.OrdinalIgnoreCase)
.Where(g => g.Count() == 1) // Ignore attributes that appear multiple times, either as physical or virtual attributes
.Select(g => g.Single())
.Where(c => c.IsVirtual == false) // Ignore virtual attributes
.GroupBy(c => c.Alias, StringComparer.OrdinalIgnoreCase)
.Where(g => g.Count() == 1) // Don't fold aliases if there are multiple columns using the same alias
.Select(g => g.Single())
.Where(c =>
{
var parts = c.SourceColumn.SplitMultiPartIdentifier();

if (parts.Length > 1 && aliasStars.Contains(parts[0]))
return false; // Don't fold aliases if we're using an <all-attributes/>

if (c.Alias.Equals(parts.Last(), StringComparison.OrdinalIgnoreCase))
return false; // Don't fold aliases if we're using the original source name

if (!FetchXmlScan.IsValidAlias(c.Alias))
return false; // Don't fold aliases if they contain invalid characters

if (ColumnMappings.Any(m => m.OutputColumn == c.SourceColumn))
return false; // Don't fold aliases if they're already aliased in the FetchXmlScan node

return true;
})
.Select(c =>
{
var attr = AddAttribute(c.SourceColumn, null, metadata, out _, out var linkEntity, out _);
return new { c.Mapping, c.SourceColumn, c.Alias, Attr = attr, LinkEntity = linkEntity };
})
.Where(c =>
{
var items = c.LinkEntity?.Items ?? Entity.Items;

// Don't fold the alias if there's also a sort on the same attribute, as it breaks paging
// https://markcarrington.dev/2019/12/10/inside-fetchxml-pt-4-order/#sorting_&_aliases
if (items != null && items.OfType<FetchOrderType>().Any(order => order.attribute == c.Attr.name) && AllPages)
return false;

// Don't fold the alias if it's on the audit table, it seems to break the provider
if (c.LinkEntity != null && c.LinkEntity.name == "audit" ||
c.LinkEntity == null && Entity.name == "audit")
return false;

return true;
})
.ToList();

foreach (var aliasedColumn in aliasedColumns)
{
aliasedColumn.Attr.alias = aliasedColumn.Alias;
aliasedColumn.Mapping.SourceColumn = aliasedColumn.SourceColumn.SplitMultiPartIdentifier()[0] + "." + aliasedColumn.Alias;
}
}

private void AddPrimaryIdAttribute(FetchEntityType entity, DataSource dataSource)
{
entity.Items = AddPrimaryIdAttribute(entity.Items, Alias, dataSource.Metadata[entity.name]);
Expand Down Expand Up @@ -2189,6 +2325,13 @@ private void SetDefaultPageSize(NodeCompilationContext context)
public override void FinishedFolding()
{
ReturnFullSchema = false;

// Remove any mappings that have no effect
for (var i = ColumnMappings.Count - 1; i >= 0; i--)
{
if (ColumnMappings[i].OutputColumn == ColumnMappings[i].SourceColumn)
ColumnMappings.RemoveAt(i);
}
}

protected override RowCountEstimate EstimateRowsOutInternal(NodeCompilationContext context)
Expand Down
Loading

0 comments on commit 36d83ba

Please sign in to comment.