Skip to content

Commit

Permalink
Merge pull request #373 from MarkMpn/fixes
Browse files Browse the repository at this point in the history
Fixes
  • Loading branch information
MarkMpn authored Oct 16, 2023
2 parents 5cb3100 + 796cf45 commit 339cd16
Show file tree
Hide file tree
Showing 15 changed files with 593 additions and 452 deletions.
10 changes: 10 additions & 0 deletions AzureDataStudioExtension/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Change Log

## [v7.6.1)(https://github.com/MarkMpn/Sql4Cds/releases/tag/v7.6.1) - 2023-10-15

Fixed use of `IN` subqueries when data source cannot be converted to FetchXML
Fixed use of `LIKE` filters with data containing embedded returns
Fixed incorrect row count estimates with joins of huge tables
Fixed left outer join in nested loop when the first record has no matching records from the right source
Fixed use of partitioned aggregates within a loop
Avoid errors when using `DEBUG_BYPASS_OPTIMIZATION` hint
Avoid using custom paging for `IN` and `EXISTS` filters, and where a single child record is guaranteed by the filters

## [v7.6.0](https://github.com/MarkMpn/Sql4Cds/releases/tag/v7.6.0) - 2023-09-30

Allow updating many-to-many intersect tables
Expand Down
59 changes: 49 additions & 10 deletions MarkMpn.Sql4Cds.Engine.Tests/ExecutionPlanTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1896,20 +1896,17 @@ public void NotExistsFilterCorrelated()

var select = AssertNode<SelectNode>(plans[0]);
var fetch = AssertNode<FetchXmlScan>(select.Source);
Assert.IsTrue(fetch.UsingCustomPaging);
Assert.IsFalse(fetch.UsingCustomPaging);
AssertFetchXml(fetch, @"
<fetch>
<entity name='account'>
<attribute name='accountid' />
<attribute name='name' />
<link-entity name='contact' alias='Expr2' from='parentcustomerid' to='accountid' link-type='outer'>
<attribute name='contactid' />
<order attribute='contactid' />
</link-entity>
<filter>
<condition entityname='Expr2' attribute='contactid' operator='null' />
</filter>
<order attribute='accountid' />
</entity>
</fetch>");
}
Expand Down Expand Up @@ -2775,24 +2772,20 @@ public void FoldNotInToLeftOuterJoin()

var fetch = AssertNode<FetchXmlScan>(select.Source);

Assert.IsTrue(fetch.UsingCustomPaging);
Assert.IsFalse(fetch.UsingCustomPaging);
AssertFetchXml(fetch, @"
<fetch>
<entity name='account'>
<attribute name='name' />
<attribute name='accountid' />
<link-entity name='contact' alias='Expr1' from='createdon' to='createdon' link-type='outer'>
<attribute name='contactid' />
<filter>
<condition attribute='firstname' operator='eq' value='Mark' />
</filter>
<order attribute='contactid' />
</link-entity>
<filter>
<condition attribute='name' operator='like' value='Data8%' />
<condition entityname='Expr1' attribute='contactid' operator='null' />
</filter>
<order attribute='accountid' />
</entity>
</fetch>");
}
Expand Down Expand Up @@ -3672,7 +3665,7 @@ public void DistinctOrderByUsesScalarAggregate()
</entity>
</fetch>");
var sort = AssertNode<SortNode>(merge.RightSource);
Assert.AreEqual("entity.metadataid ASC", sort.Sorts[0].ToSql());
Assert.AreEqual("entity.metadataid", sort.Sorts[0].ToSql());
var meta = AssertNode<MetadataQueryNode>(sort.Source);
}

Expand Down Expand Up @@ -6385,5 +6378,51 @@ HAVING sum(1) > 1) AS dups

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

[TestMethod]
public void DoNotUseCustomPagingForInJoin()
{
// https://github.com/MarkMpn/Sql4Cds/issues/366

_supportedJoins.Add(JoinOperator.Any);

try
{
var planBuilder = new ExecutionPlanBuilder(_dataSources.Values, new OptionsWrapper(this) { PrimaryDataSource = "uat" });

var query = @"
SELECT contactid
FROM contact
WHERE contactid IN (SELECT DISTINCT primarycontactid FROM account WHERE name = 'Data8')
AND contactid NOT IN (SELECT DISTINCT primarycontactid FROM account WHERE employees IS NULL)";

var plans = planBuilder.Build(query, null, out _);
var select = AssertNode<SelectNode>(plans[0]);
var fetch = AssertNode<FetchXmlScan>(select.Source);
AssertFetchXml(fetch, @"
<fetch xmlns:generator='MarkMpn.SQL4CDS'>
<entity name='contact'>
<attribute name='contactid' />
<link-entity name='account' from='primarycontactid' to='contactid' link-type='in' alias='Expr1'>
<filter>
<condition attribute='name' operator='eq' value='Data8' />
</filter>
</link-entity>
<link-entity name='account' from='primarycontactid' to='contactid' link-type='outer' alias='Expr3'>
<filter>
<condition attribute='employees' operator='null' />
</filter>
</link-entity>
<filter>
<condition entityname='Expr3' attribute='accountid' operator='null' />
</filter>
</entity>
</fetch>");
}
finally
{
_supportedJoins.Remove(JoinOperator.Any);
}
}
}
}
30 changes: 30 additions & 0 deletions MarkMpn.Sql4Cds.Engine.Tests/ExpressionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,36 @@ public void FormatDateTime()
Assert.AreEqual(compilationContext.PrimaryDataSource.DefaultCollation.ToSqlString("2022-01-02"), value);
}

[TestMethod]
public void LikeWithEmbeddedReturns()
{
var expr = new LikePredicate
{
FirstExpression = Col("content"),
SecondExpression = new StringLiteral { Value = "%func%" }
};
var schema = new NodeSchema(new Dictionary<string, IColumnDefinition>
{
["content"] = new ExecutionPlan.ColumnDefinition(DataTypeHelpers.VarChar(100, Collation.USEnglish, CollationLabel.Implicit), true, false)
}, new Dictionary<string, IReadOnlyList<string>>(), null, Array.Empty<string>());
var parameterTypes = new Dictionary<string, DataTypeReference>();
var options = new StubOptions();
var compilationContext = new ExpressionCompilationContext(_localDataSource, options, parameterTypes, schema, null);
var func = expr.Compile(compilationContext);

var record = new Entity
{
["content"] = Collation.USEnglish.ToSqlString(@"function func() {
console.log(""hello"");
};")
};
var executionContext = new ExpressionExecutionContext(compilationContext);
executionContext.Entity = record;

var value = func(executionContext);
Assert.AreEqual(true, value);
}

private ColumnReferenceExpression Col(string name)
{
return new ColumnReferenceExpression
Expand Down
7 changes: 6 additions & 1 deletion MarkMpn.Sql4Cds.Engine/ExecutionPlan/ExpressionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1257,7 +1257,12 @@ internal static Regex LikeToRegex(SqlString pattern, SqlString escape, bool patI
if (!patIndex || !endWildcard)
regexBuilder.Append("$");

return new Regex(regexBuilder.ToString(), pattern.SqlCompareOptions.HasFlag(SqlCompareOptions.IgnoreCase) ? RegexOptions.IgnoreCase : RegexOptions.None);
var regexOptions = RegexOptions.Multiline;

if (pattern.SqlCompareOptions.HasFlag(SqlCompareOptions.IgnoreCase))
regexOptions |= RegexOptions.IgnoreCase;

return new Regex(regexBuilder.ToString(), regexOptions);
}

private static SqlBoolean Like(SqlString value, SqlString pattern, SqlString escape, bool not)
Expand Down
56 changes: 54 additions & 2 deletions MarkMpn.Sql4Cds.Engine/ExecutionPlan/FetchXmlScan.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,11 @@ public FetchXmlScan()

public bool RequiresCustomPaging(IDictionary<string, DataSource> dataSources)
{
// Custom paging is required if we have links to child entities, as standard Dataverse paging is applied at
// the top-level entity only.
// Custom paging can't be used with distinct queries, as the primary key fields required to implement the paging
// may not be included.
// It also can't be used with aggregate queries as doing so would affect the aggregae behaviour.
if (FetchXml.distinct)
return false;

Expand All @@ -225,10 +230,57 @@ public bool RequiresCustomPaging(IDictionary<string, DataSource> dataSources)

foreach (var linkEntity in Entity.GetLinkEntities())
{
// Link entities used for filtering do not require paging
if (linkEntity.linktype == "exists" || linkEntity.linktype == "in")
continue;

if (linkEntity.from != dataSource.Metadata[linkEntity.name].PrimaryIdAttribute)
if (HasSingleRecordFilter(linkEntity, dataSource.Metadata[linkEntity.name].PrimaryIdAttribute))
continue;

// Parental lookups do not require paging
if (linkEntity.from == dataSource.Metadata[linkEntity.name].PrimaryIdAttribute)
continue;

return true;
}

return false;
}

private bool HasSingleRecordFilter(FetchLinkEntityType linkEntity, string primaryIdAttribute)
{
// Look for outer joins where we are filtering on a null primary key, i.e. there are no child records,
// or any join where we are filtering on the primary key equals a fixed guid.
return HasSingleRecordFilter(linkEntity, primaryIdAttribute, Entity.Items, linkEntity.alias) ||
HasSingleRecordFilter(linkEntity, primaryIdAttribute, linkEntity.Items, null);
}

private bool HasSingleRecordFilter(FetchLinkEntityType linkEntity, string primaryIdAttribute, object[] items, string alias)
{
if (items == null)
return false;

foreach (var filter in items.OfType<filter>().Where(f => f.type == filterType.and))
{
if (HasSingleRecordFilter(linkEntity, primaryIdAttribute, filter.Items, alias))
return true;
}

foreach (var condition in items.OfType<condition>())
{
if (condition.entityname != alias)
continue;

if (condition.attribute != primaryIdAttribute)
continue;

if (condition.@operator == @operator.@null)
return true;

if (condition.@operator == @operator.eq)
return true;

if (condition.@operator == @operator.@in && condition.Items != null && condition.Items.Length == 1)
return true;
}

Expand Down Expand Up @@ -1664,7 +1716,7 @@ public override void AddRequiredColumns(NodeCompilationContext context, IList<st
// Ensure the primary key of each entity is included
AddPrimaryIdAttribute(Entity, dataSource);

foreach (var linkEntity in Entity.GetLinkEntities())
foreach (var linkEntity in Entity.GetLinkEntities().Where(le => le.linktype != "exists" && le.linktype != "in" && !HasSingleRecordFilter(le, dataSource.Metadata[le.name].PrimaryIdAttribute)))
AddPrimaryIdAttribute(linkEntity, dataSource);
}

Expand Down
4 changes: 4 additions & 0 deletions MarkMpn.Sql4Cds.Engine/ExecutionPlan/FoldableJoinNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,10 @@ protected override RowCountEstimate EstimateRowsOutInternal(NodeCompilationConte
else
{
estimate = leftMax * rightMax;

// Check for overflow
if (estimate < leftMax || estimate < rightMax)
estimate = Int32.MaxValue;
}

if (leftIsRange && rightIsRange)
Expand Down
10 changes: 10 additions & 0 deletions MarkMpn.Sql4Cds.Engine/ExecutionPlan/NestedLoopNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,17 @@ protected override IEnumerable<Entity> ExecuteInternal(NodeExecutionContext cont
}

if (!hasRight && JoinType == QualifiedJoinType.LeftOuter)
{
if (rightSchema == null)
{
rightSchema = RightSource.GetSchema(rightCompilationContext);
mergedSchema = GetSchema(context, true);
joinCondition = JoinCondition?.Compile(new ExpressionCompilationContext(context, mergedSchema, null));
joinConditionContext = joinCondition == null ? null : new ExpressionExecutionContext(context);
}

yield return Merge(left, leftSchema, null, rightSchema);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ protected override IEnumerable<Entity> ExecuteInternal(NodeExecutionContext cont
InitializePartitionedAggregates(expressionCompilationContext);
var aggregates = CreateAggregateFunctions(expressionExecutionContext, true);

var fetchXmlNode = (FetchXmlScan)Source;
// Clone the source FetchXmlScan node so we can safely modify it later by adding the partitioning filter
// We might be called in a loop and need to execute again, so don't make any changes to the original
// source query.
var fetchXmlNode = (FetchXmlScan)Source.Clone();

var name = fetchXmlNode.Entity.name;
var meta = context.DataSources[fetchXmlNode.DataSource].Metadata[name];
Expand Down
9 changes: 4 additions & 5 deletions MarkMpn.Sql4Cds.Engine/ExecutionPlanOptimizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,8 @@ public IRootExecutionPlanNodeInternal[] Optimize(IRootExecutionPlanNodeInternal
var context = new NodeCompilationContext(DataSources, Options, ParameterTypes);

// Move any additional operators down to the FetchXml
var nodes =
hints != null && hints.OfType<UseHintList>().Any(list => list.Hints.Any(h => h.Value.Equals("DEBUG_BYPASS_OPTIMIZATION", StringComparison.OrdinalIgnoreCase)))
? new[] { node }
: node.FoldQuery(context, hints);
var bypassOptimization = hints != null && hints.OfType<UseHintList>().Any(list => list.Hints.Any(h => h.Value.Equals("DEBUG_BYPASS_OPTIMIZATION", StringComparison.OrdinalIgnoreCase)));
var nodes = bypassOptimization ? new[] { node } : node.FoldQuery(context, hints);

foreach (var n in nodes)
{
Expand All @@ -66,7 +64,8 @@ public IRootExecutionPlanNodeInternal[] Optimize(IRootExecutionPlanNodeInternal
SortFetchXmlElements(n);

// Let the nodes know that folding is now finished so they can do any internal tidy-up
MarkComplete(n);
if (!bypassOptimization)
MarkComplete(n);
}

return nodes;
Expand Down
14 changes: 7 additions & 7 deletions MarkMpn.Sql4Cds.Engine/MarkMpn.Sql4Cds.Engine.nuspec
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
<iconUrl>https://markcarrington.dev/sql4cds-icon/</iconUrl>
<description>Convert SQL queries to FetchXml and execute them against Dataverse / D365</description>
<summary>Convert SQL queries to FetchXml and execute them against Dataverse / D365</summary>
<releaseNotes>Allow updating many-to-many intersect tables
Allow folding `NOT EXISTS` predicates to FetchXML for improved performance
Improved sorting reliability on audit and elastic tables
Fixed KeyNotFoundException from certain joins
Fixed NullReferenceException when applying filters to certain joins
Preserve additional join criteria on `metadata` schema tables
Fixed use of `SELECT *` in subqueries
<releaseNotes>Fixed use of IN subqueries when data source cannot be converted to FetchXML
Fixed use of LIKE filters with data containing embedded returns
Fixed incorrect row count estimates with joins of huge tables
Fixed left outer join in nested loop when the first record has no matching records from the right source
Fixed use of partitioned aggregates within a loop
Avoid errors when using DEBUG_BYPASS_OPTIMIZATION hint
Avoid using custom paging for IN and EXISTS filters, and where a single child record is guaranteed by the filters
</releaseNotes>
<copyright>Copyright © 2020 Mark Carrington</copyright>
<language>en-GB</language>
Expand Down
Loading

0 comments on commit 339cd16

Please sign in to comment.