diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/Utils.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/Utils.cs index 0e3189f57b..f4d8ae1273 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/Utils.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/Utils.cs @@ -2,10 +2,14 @@ // Licensed under the MIT license. using System.Globalization; +using System.Linq; using System.Numerics; +using Microsoft.PowerFx.Core.IR; +using Microsoft.PowerFx.Core.IR.Nodes; using Microsoft.PowerFx.Core.Localization; using Microsoft.PowerFx.Core.Types; using Microsoft.PowerFx.Core.Utils; +using Microsoft.PowerFx.Types; namespace Microsoft.PowerFx.Core.Functions { @@ -23,5 +27,18 @@ public static string GetLocalizedName(this FunctionCategories category, CultureI { return StringResources.Get(category.ToString(), culture.Name); } + + public static void FunctionSupportColumnNamesAsIdentifiersDependencyUtil(this CallNode node, DependencyVisitor visitor) + { + var aggregateType0 = node.Args[0].IRContext.ResultType as AggregateType; + + foreach (TextLiteralNode arg in node.Args.Skip(1).Where(a => a is TextLiteralNode)) + { + if (aggregateType0.TryGetFieldType(arg.LiteralValue, out _)) + { + visitor.AddDependency(aggregateType0.TableSymbolName, arg.LiteralValue); + } + } + } } } diff --git a/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs b/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs index f6619a7639..13d7117790 100644 --- a/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs +++ b/src/libraries/Microsoft.PowerFx.Core/IR/IRTranslator.cs @@ -29,8 +29,13 @@ namespace Microsoft.PowerFx.Core.IR { internal class IRResult - { - public IntermediateNode TopNode; + { + // IR top node after transformations. + public IntermediateNode TopNode; + + // Original IR node, without transformations. + public IntermediateNode TopOriginalNode; + public ScopeSymbol RuleScopeSymbol; } diff --git a/src/libraries/Microsoft.PowerFx.Core/IR/Visitors/DependencyVisitor.cs b/src/libraries/Microsoft.PowerFx.Core/IR/Visitors/DependencyVisitor.cs index b3f634f6cb..9a754f7bc6 100644 --- a/src/libraries/Microsoft.PowerFx.Core/IR/Visitors/DependencyVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Core/IR/Visitors/DependencyVisitor.cs @@ -6,6 +6,8 @@ using System.Text; using Microsoft.PowerFx.Core.IR.Nodes; using Microsoft.PowerFx.Core.IR.Symbols; +using Microsoft.PowerFx.Core.Types; +using Microsoft.PowerFx.Core.Utils; using Microsoft.PowerFx.Types; using static Microsoft.PowerFx.Syntax.PrettyPrintVisitor; @@ -14,7 +16,7 @@ namespace Microsoft.PowerFx.Core.IR // IR has already: // - resolved everything to logical names. // - resolved implicit ThisRecord - internal class DependencyVisitor : IRNodeVisitor + internal sealed class DependencyVisitor : IRNodeVisitor { // Track reults. public DependencyInfo Info { get; private set; } = new DependencyInfo(); @@ -50,13 +52,10 @@ public override RetVal Visit(ColorLiteralNode node, DependencyContext context) public override RetVal Visit(RecordNode node, DependencyContext context) { - // Read all the fields. The context will determine if the record is referencing a data source + // Visit all field values in case there are CallNodes. Field keys should be handled by the function caller. foreach (var kv in node.Fields) { - AddField(context, context.ScopeType?.TableSymbolName, kv.Key.Value); - - // Reset the context for field values in case we are operating in a write context. - kv.Value.Accept(this, new DependencyContext()); + kv.Value.Accept(this, context); } return null; @@ -109,20 +108,16 @@ public override RetVal Visit(ScopeAccessNode node, DependencyContext context) if (_scopeTypes.TryGetValue(sym.Parent.Id, out var type)) { // Ignore ThisRecord scopeaccess node. e.g. Summarize(table, f1, Sum(ThisGroup, f2)) where ThisGroup should be ignored. - if (type is TableType tableType && node.IRContext.ResultType is not AggregateType) + if (type is TableType tableType && tableType.TryGetFieldType(sym.Name.Value, out _)) { - var tableLogicalName = tableType.TableSymbolName; - var fieldLogicalName = sym.Name.Value; - - AddField(context, tableLogicalName, fieldLogicalName); + AddDependency(tableType.TableSymbolName, sym.Name.Value); return null; } } } - - // Any symbol access here is some temporary local, and not a field. - return null; + + return null; } // field // IR will implicity recognize as ThisRecod.field @@ -141,7 +136,7 @@ public override RetVal Visit(RecordFieldAccessNode node, DependencyContext conte if (tableLogicalName != null) { var fieldLogicalName = node.Field.Value; - AddField(context, tableLogicalName, fieldLogicalName); + AddDependency(tableLogicalName, fieldLogicalName); } } @@ -152,16 +147,7 @@ public override RetVal Visit(ResolvedObjectNode node, DependencyContext context) { if (node.IRContext.ResultType is AggregateType aggregateType) { - var tableLogicalName = aggregateType.TableSymbolName; - if (context.WriteState) - { - tableLogicalName = context.ScopeType?.TableSymbolName; - } - - if (tableLogicalName != null) - { - AddField(context, tableLogicalName, null); - } + AddDependency(aggregateType.TableSymbolName, null); } CheckResolvedObjectNodeValue(node, context); @@ -169,7 +155,7 @@ public override RetVal Visit(ResolvedObjectNode node, DependencyContext context) return null; } - protected virtual void CheckResolvedObjectNodeValue(ResolvedObjectNode node, DependencyContext context) + public void CheckResolvedObjectNodeValue(ResolvedObjectNode node, DependencyContext context) { if (node.Value is NameSymbol sym) { @@ -181,14 +167,14 @@ protected virtual void CheckResolvedObjectNodeValue(ResolvedObjectNode node, Dep if (symTable.IsThisRecord(sym)) { // "ThisRecord". Whole entity - AddField(context, tableLogicalName, null); + AddDependency(type.TableSymbolName, null); return; } // on current table var fieldLogicalName = sym.Name; - AddField(context, tableLogicalName, fieldLogicalName); + AddDependency(type.TableSymbolName, fieldLogicalName); } } } @@ -224,73 +210,27 @@ public class RetVal public class DependencyContext { - // Indicates the scanning is working on the args of a non self-contained function. - public bool WriteState { get; init; } - - // If WriteState is true, this will be set with the arg0 type. - public TableType ScopeType { get; init; } - public DependencyContext() { } } - // Translate relationship names to actual field references. - public virtual string Translate(string tableLogicalName, string fieldLogicalName) - { - return fieldLogicalName; - } - // if fieldLogicalName, then we're taking a dependency on entire record. - private void AddField(Dictionary> list, string tableLogicalName, string fieldLogicalName) + public void AddDependency(string tableLogicalName, string fieldLogicalName) { if (tableLogicalName == null) { return; } - if (!list.TryGetValue(tableLogicalName, out var fieldReads)) + if (!Info.Dependencies.ContainsKey(tableLogicalName)) { - fieldReads = new HashSet(); - list[tableLogicalName] = fieldReads; + Info.Dependencies[tableLogicalName] = new HashSet(); } - - if (fieldLogicalName != null) - { - var name = Translate(tableLogicalName, fieldLogicalName); - fieldReads.Add(name); - } - } - public void AddFieldRead(string tableLogicalName, string fieldLogicalName) - { - if (Info.FieldReads == null) - { - Info.FieldReads = new Dictionary>(); - } - - AddField(Info.FieldReads, tableLogicalName, fieldLogicalName); - } - - public void AddFieldWrite(string tableLogicalName, string fieldLogicalName) - { - if (Info.FieldWrites == null) - { - Info.FieldWrites = new Dictionary>(); - } - - AddField(Info.FieldWrites, tableLogicalName, fieldLogicalName); - } - - public void AddField(DependencyContext context, string tableLogicalName, string fieldLogicalName) - { - if (context.WriteState) - { - AddFieldWrite(tableLogicalName, fieldLogicalName); - } - else + if (fieldLogicalName != null) { - AddFieldRead(tableLogicalName, fieldLogicalName); + Info.Dependencies[tableLogicalName].Add(fieldLogicalName); } } } @@ -310,29 +250,28 @@ public class DependencyInfo /// The formula "Name & 'Primary Contact'.'Full Name' & Sum(Contacts, 'Number Of Childeren')" would return /// "contact" => { "fullname", "numberofchildren" }. /// - public Dictionary> FieldReads { get; set; } -#pragma warning restore CS1570 // XML comment has badly formed XML - - public Dictionary> FieldWrites { get; set; } + public Dictionary> Dependencies { get; set; } - public bool HasWrites => FieldWrites != null && FieldWrites.Count > 0; + public DependencyInfo() + { + Dependencies = new Dictionary>(); + } public override string ToString() { StringBuilder sb = new StringBuilder(); - DumpHelper(sb, "Read", FieldReads); - DumpHelper(sb, "Write", FieldWrites); + DumpHelper(sb, Dependencies); return sb.ToString(); } - private static void DumpHelper(StringBuilder sb, string kind, Dictionary> dict) + private static void DumpHelper(StringBuilder sb, Dictionary> dict) { if (dict != null) { foreach (var kv in dict) { - sb.Append(kind); + sb.Append("Entity"); sb.Append(" "); sb.Append(kv.Key); sb.Append(": "); diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/CheckResult.cs b/src/libraries/Microsoft.PowerFx.Core/Public/CheckResult.cs index f0cf13bbdd..1a374ca805 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/CheckResult.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/CheckResult.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis; using Microsoft.PowerFx.Core.Binding; using Microsoft.PowerFx.Core.IR; +using Microsoft.PowerFx.Core.IR.Nodes; using Microsoft.PowerFx.Core.Localization; using Microsoft.PowerFx.Core.Logging; using Microsoft.PowerFx.Core.Public; @@ -483,6 +484,7 @@ private void VerifyReturnTypeMatch() /// /// Compute the dependencies. Called after binding. /// + [Obsolete("Preview")] public DependencyInfo ApplyDependencyInfoScan() { var ir = ApplyIR(); //throws on errors @@ -490,7 +492,8 @@ public DependencyInfo ApplyDependencyInfoScan() var ctx = new DependencyVisitor.DependencyContext(); var visitor = new DependencyVisitor(); - ir.TopNode.Accept(visitor, ctx); + // Using the original node without transformations. This simplifies the dependency analysis for PFx.DV side. + ir.TopOriginalNode.Accept(visitor, ctx); return visitor.Info; } @@ -549,17 +552,19 @@ internal IRResult ApplyIR() { if (_irresult == null) { + IntermediateNode irnode = null; + // IR should not create any new errors. var binding = this.ApplyBindingInternal(); this.ThrowOnErrors(); - (var irnode, var ruleScopeSymbol) = IRTranslator.Translate(binding); + (var originalIRNode, var ruleScopeSymbol) = IRTranslator.Translate(binding); var list = _engine.IRTransformList; if (list != null) { foreach (var transform in list) { - irnode = transform.Transform(irnode, _errors); + irnode = transform.Transform(irnode ?? originalIRNode, _errors); // Additional errors from phases. // Stop any further processing if we have errors. @@ -569,7 +574,8 @@ internal IRResult ApplyIR() _irresult = new IRResult { - TopNode = irnode, + TopNode = irnode ?? originalIRNode, + TopOriginalNode = originalIRNode, RuleScopeSymbol = ruleScopeSymbol }; } @@ -606,7 +612,7 @@ public FormulaType GetNodeType(TexlNode node) /// /// /// Null if the node is not bound. - public FunctionInfo GetFunctionInfo(CallNode node) + public FunctionInfo GetFunctionInfo(Syntax.CallNode node) { if (node == null) { diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Collect.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Collect.cs index cf94ca8d9d..05fe602e7d 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Collect.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Collect.cs @@ -469,15 +469,27 @@ protected static List CreateIRCallNodeCollect(CallNode node, I public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor visitor, DependencyVisitor.DependencyContext context) { - var newContext = new DependencyVisitor.DependencyContext() - { - WriteState = true, - ScopeType = node.Args[0].IRContext.ResultType as TableType - }; + var tableType = (TableType)node.Args[0].IRContext.ResultType; foreach (var arg in node.Args.Skip(1)) { - arg.Accept(visitor, newContext); + var argType = arg.IRContext.ResultType; + string argTableSymbolName = null; + + if (argType is AggregateType aggregateType) + { + // If argN is a record/table and has a table symbol name, it means we are referencing another entity. + // We then need to add a dependency to the table symbol name as well. + argTableSymbolName = aggregateType.TableSymbolName; + } + + foreach (var name in argType._type.GetAllNames(DPath.Root)) + { + visitor.AddDependency(tableType.TableSymbolName, name.Name.Value); + visitor.AddDependency(argTableSymbolName, name.Name.Value); + } + + arg.Accept(visitor, context); } return true; @@ -511,7 +523,7 @@ internal override IntermediateNode CreateIRCallNode(PowerFx.Syntax.CallNode node public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor visitor, DependencyVisitor.DependencyContext context) { var tableType = (TableType)node.Args[0].IRContext.ResultType; - visitor.AddFieldWrite(tableType.TableSymbolName, "Value"); + visitor.AddDependency(tableType.TableSymbolName, "Value"); return true; } diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Join.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Join.cs index b2efb9747b..892cde5428 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Join.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Join.cs @@ -259,7 +259,7 @@ public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor vi foreach (var field in recordNode.Fields) { var tableType = (TableType)sourceArg.IRContext.ResultType; - visitor.AddField(context, tableType.TableSymbolName, field.Key.Value); + visitor.AddDependency(tableType.TableSymbolName, field.Key.Value); } break; diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Patch.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Patch.cs index d4a1edbe42..738f310bd1 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Patch.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Patch.cs @@ -451,18 +451,28 @@ public override void CheckSemantics(TexlBinding binding, TexlNode[] args, DType[ public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor visitor, DependencyVisitor.DependencyContext context) { - // Arg1 is the record to be found. All fields are readonly, so we don't need to add any writes here. - node.Args[1].Accept(visitor, new DependencyVisitor.DependencyContext() { ScopeType = node.Args[0].IRContext.ResultType as TableType }); + var tableType = (TableType)node.Args[0].IRContext.ResultType; + var recordType = (RecordType)node.Args[1].IRContext.ResultType; - var newContext = new DependencyVisitor.DependencyContext() + // arg1 might refere both to arg0 and arg1. + // Examples: + // Patch(t1, {...}, {...}) => arg1 is inmemory record and the fields refers to t1. + // Patch(t1, First(t2), {...}) => arg1 fields refers to t2 and t1. + foreach (var fieldName in recordType.FieldNames) { - WriteState = true, - ScopeType = node.Args[0].IRContext.ResultType as TableType - }; + visitor.AddDependency(recordType.TableSymbolName, fieldName); + } - foreach (var arg in node.Args.Skip(2).Select(arg => (IRRecordNode)arg)) + foreach (var arg in node.Args.Skip(1)) { - arg.Accept(visitor, newContext); + arg.Accept(visitor, context); + if (arg.IRContext.ResultType is AggregateType aggregateType) + { + foreach (var fieldName in aggregateType.FieldNames) + { + visitor.AddDependency(tableType.TableSymbolName, fieldName); + } + } } return true; @@ -496,7 +506,7 @@ public PatchSingleRecordFunction() } public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor visitor, DependencyVisitor.DependencyContext context) - { + { var tableType = (TableType)node.Args[0].IRContext.ResultType; var recordType = (RecordType)node.Args[1].IRContext.ResultType; @@ -504,14 +514,7 @@ public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor vi foreach (var fieldName in recordType.FieldNames) { - if (datasource != null && datasource.GetKeyColumns().Contains(fieldName)) - { - visitor.AddFieldRead(tableType.TableSymbolName, fieldName); - } - else - { - visitor.AddFieldWrite(tableType.TableSymbolName, fieldName); - } + visitor.AddDependency(tableType.TableSymbolName, fieldName); } return true; @@ -549,16 +552,14 @@ public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor vi var tableType1 = (TableType)node.Args[1].IRContext.ResultType; var tableType2 = (TableType)node.Args[2].IRContext.ResultType; - var datasource = tableType0._type.AssociatedDataSources.First(); - foreach (var fieldName in tableType1.FieldNames) { - visitor.AddFieldRead(tableType0.TableSymbolName, fieldName); + visitor.AddDependency(tableType0.TableSymbolName, fieldName); } foreach (var fieldName in tableType2.FieldNames) { - visitor.AddFieldWrite(tableType0.TableSymbolName, fieldName); + visitor.AddDependency(tableType0.TableSymbolName, fieldName); } return true; @@ -591,11 +592,11 @@ public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor vi { if (datasource != null && datasource.GetKeyColumns().Contains(fieldName)) { - visitor.AddFieldRead(tableType0.TableSymbolName, fieldName); + visitor.AddDependency(tableType0.TableSymbolName, fieldName); } else { - visitor.AddFieldWrite(tableType0.TableSymbolName, fieldName); + visitor.AddDependency(tableType0.TableSymbolName, fieldName); } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/RenameColumns.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/RenameColumns.cs index 7044d0d9a5..e264391b16 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/RenameColumns.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/RenameColumns.cs @@ -3,15 +3,21 @@ using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Linq; using Microsoft.PowerFx.Core.App.ErrorContainers; using Microsoft.PowerFx.Core.Binding; using Microsoft.PowerFx.Core.Entities.QueryOptions; using Microsoft.PowerFx.Core.Errors; using Microsoft.PowerFx.Core.Functions; +using Microsoft.PowerFx.Core.IR; +using Microsoft.PowerFx.Core.IR.Nodes; using Microsoft.PowerFx.Core.Localization; using Microsoft.PowerFx.Core.Types; using Microsoft.PowerFx.Core.Utils; using Microsoft.PowerFx.Syntax; +using Microsoft.PowerFx.Types; +using CallNode = Microsoft.PowerFx.Syntax.CallNode; +using IRCallNode = Microsoft.PowerFx.Core.IR.Nodes.CallNode; namespace Microsoft.PowerFx.Core.Texl.Builtins { @@ -265,5 +271,12 @@ public static ColumnReplacement Create(DName oldName, DName newName, DType type) }; } } + + public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor visitor, DependencyVisitor.DependencyContext context) + { + Utils2.FunctionSupportColumnNamesAsIdentifiersDependencyUtil(node, visitor); + + return true; + } } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/ShowDropColumnsBase.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/ShowDropColumnsBase.cs index c9b315149a..d976c478fb 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/ShowDropColumnsBase.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/ShowDropColumnsBase.cs @@ -2,9 +2,12 @@ // Licensed under the MIT license. using System.Collections.Generic; +using System.Linq; using Microsoft.PowerFx.Core.App.ErrorContainers; using Microsoft.PowerFx.Core.Errors; using Microsoft.PowerFx.Core.Functions; +using Microsoft.PowerFx.Core.IR; +using Microsoft.PowerFx.Core.IR.Nodes; using Microsoft.PowerFx.Core.Localization; using Microsoft.PowerFx.Core.Types; using Microsoft.PowerFx.Core.Utils; @@ -193,5 +196,12 @@ public override ParamIdentifierStatus GetIdentifierParamStatus(TexlNode node, Fe return index > 0 ? ParamIdentifierStatus.AlwaysIdentifier : ParamIdentifierStatus.NeverIdentifier; } + + public override bool ComposeDependencyInfo(IR.Nodes.CallNode node, DependencyVisitor visitor, DependencyVisitor.DependencyContext context) + { + Utils2.FunctionSupportColumnNamesAsIdentifiersDependencyUtil(node, visitor); + + return true; + } } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/SortByColumns.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/SortByColumns.cs index d2b9bff807..a1cf0af7bf 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/SortByColumns.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/SortByColumns.cs @@ -7,18 +7,22 @@ using System.Linq; using Microsoft.PowerFx.Core.App.ErrorContainers; using Microsoft.PowerFx.Core.Binding; -using Microsoft.PowerFx.Core.Entities; using Microsoft.PowerFx.Core.Entities.QueryOptions; using Microsoft.PowerFx.Core.Errors; using Microsoft.PowerFx.Core.Functions; using Microsoft.PowerFx.Core.Functions.Delegation; using Microsoft.PowerFx.Core.Functions.Delegation.DelegationMetadata; using Microsoft.PowerFx.Core.Functions.FunctionArgValidators; +using Microsoft.PowerFx.Core.IR; +using Microsoft.PowerFx.Core.IR.Nodes; using Microsoft.PowerFx.Core.Localization; using Microsoft.PowerFx.Core.Types; using Microsoft.PowerFx.Core.Types.Enums; using Microsoft.PowerFx.Core.Utils; using Microsoft.PowerFx.Syntax; +using Microsoft.PowerFx.Types; +using CallNode = Microsoft.PowerFx.Syntax.CallNode; +using IRCallNode = Microsoft.PowerFx.Core.IR.Nodes.CallNode; namespace Microsoft.PowerFx.Core.Texl.Builtins { @@ -543,5 +547,12 @@ public override ParamIdentifierStatus GetIdentifierParamStatus(TexlNode node, Fe return (index % 2) == 1 ? ParamIdentifierStatus.PossiblyIdentifier : ParamIdentifierStatus.NeverIdentifier; } + + public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor visitor, DependencyVisitor.DependencyContext context) + { + Utils2.FunctionSupportColumnNamesAsIdentifiersDependencyUtil(node, visitor); + + return true; + } } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Summarize.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Summarize.cs index e21d5052f9..4cb08064dc 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Summarize.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Summarize.cs @@ -270,7 +270,7 @@ public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor vi { if (arg is TextLiteralNode textLiteralNode) { - visitor.AddFieldRead(tableTypeName, textLiteralNode.LiteralValue); + visitor.AddDependency(tableTypeName, textLiteralNode.LiteralValue); } else if (arg is LazyEvalNode lazyEvalNode) { diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/RemoveFunction.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/RemoveFunction.cs index 09b8bdee9a..cda01cf15f 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/RemoveFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/RemoveFunction.cs @@ -241,15 +241,18 @@ public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, Cance public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor visitor, DependencyVisitor.DependencyContext context) { - var newContext = new DependencyVisitor.DependencyContext() - { - WriteState = true, - ScopeType = node.Args[0].IRContext.ResultType as TableType - }; + var tableType = (TableType)node.Args[0].IRContext.ResultType; - foreach (var arg in node.Args.Skip(1)) + foreach (var arg in node.Args.Skip(1).Where(a => a.IRContext.ResultType is AggregateType)) { - arg.Accept(visitor, newContext); + var argType = arg.IRContext.ResultType; + + foreach (var name in argType._type.GetAllNames(DPath.Root)) + { + visitor.AddDependency(tableType.TableSymbolName, name.Name.Value); + } + + arg.Accept(visitor, context); } return true; diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/SetFunction.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/SetFunction.cs index 724e67444a..bd7422d4e3 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/SetFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/SetFunction.cs @@ -150,12 +150,7 @@ private bool CheckMutability(TexlBinding binding, TexlNode[] args, DType[] argTy public override bool ComposeDependencyInfo(IRCallNode node, DependencyVisitor visitor, DependencyVisitor.DependencyContext context) { - var newContext = new DependencyVisitor.DependencyContext() - { - WriteState = true - }; - - node.Args[0].Accept(visitor, newContext); + node.Args[0].Accept(visitor, context); node.Args[1].Accept(visitor, context); return true; diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/DependencyTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/DependencyTests.cs index a8085b1847..e85fb38eb1 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/DependencyTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/DependencyTests.cs @@ -16,6 +16,7 @@ using Microsoft.PowerFx.Interpreter; using Microsoft.PowerFx.Types; using Xunit; +using Xunit.Sdk; namespace Microsoft.PowerFx.Core.Tests { @@ -23,69 +24,78 @@ public class DependencyTests : PowerFxTest { [Theory] [InlineData("1+2", "")] // none - [InlineData("ThisRecord.'Address 1: City' & 'Account Name'", "Read Accounts: address1_city, name;")] // basic read + [InlineData("ThisRecord.'Address 1: City' & 'Account Name'", "Entity Accounts: address1_city, name;")] // basic read - [InlineData("numberofemployees%", "Read Accounts: numberofemployees;")] // unary op - [InlineData("ThisRecord", "Read Accounts: ;")] // whole scope + [InlineData("numberofemployees%", "Entity Accounts: numberofemployees;")] // unary op + [InlineData("ThisRecord", "Entity Accounts: ;")] // whole scope [InlineData("{x:5}.x", "")] - [InlineData("With({x : ThisRecord}, x.'Address 1: City')", "Read Accounts: address1_city;")] // alias - [InlineData("With({'Address 1: City' : \"Seattle\"}, 'Address 1: City' & 'Account Name')", "Read Accounts: name;")] // 'Address 1: City' is shadowed + [InlineData("With({x : ThisRecord}, x.'Address 1: City')", "Entity Accounts: address1_city;")] // alias + [InlineData("With({'Address 1: City' : \"Seattle\"}, 'Address 1: City' & 'Account Name')", "Entity Accounts: name;")] // 'Address 1: City' is shadowed [InlineData("With({'Address 1: City' : 5}, ThisRecord.'Address 1: City')", "")] // shadowed - [InlineData("LookUp(local,'Address 1: City'=\"something\")", "Read Accounts: address1_city;")] // Lookup and RowScope - [InlineData("Filter(local,numberofemployees > 200)", "Read Accounts: numberofemployees;")] - [InlineData("First(local)", "Read Accounts: ;")] - [InlineData("First(local).'Address 1: City'", "Read Accounts: address1_city;")] - [InlineData("Last(local)", "Read Accounts: ;")] - [InlineData("local", "Read Accounts: ;")] // whole table + [InlineData("LookUp(local,'Address 1: City'=\"something\")", "Entity Accounts: address1_city;")] // Lookup and RowScope + [InlineData("Filter(local,numberofemployees > 200)", "Entity Accounts: numberofemployees;")] + [InlineData("First(local)", "Entity Accounts: ;")] + [InlineData("First(local).'Address 1: City'", "Entity Accounts: address1_city;")] + [InlineData("Last(local)", "Entity Accounts: ;")] + [InlineData("local", "Entity Accounts: ;")] // whole table [InlineData("12 & true & \"abc\" ", "")] // walker ignores literals - [InlineData("12;'Address 1: City';12", "Read Accounts: address1_city;")] // chaining - [InlineData("ParamLocal1.'Address 1: City'", "Read Accounts: address1_city;")] // basic read - [InlineData("{test:First(local).name}", "Read Accounts: name;")] + [InlineData("12;'Address 1: City';12", "Entity Accounts: address1_city;")] // chaining + [InlineData("ParamLocal1.'Address 1: City'", "Entity Accounts: address1_city;")] // basic read + [InlineData("{test:First(local).name}", "Entity Accounts: name;")] + [InlineData("AddColumns(simple1, z, 1)", "Entity Simple1: ;")] + [InlineData("RenameColumns(simple1, b, c)", "Entity Simple1: b;")] + [InlineData("SortByColumns(simple1, a, SortOrder.Descending)", "Entity Simple1: a;")] // Basic scoping - [InlineData("Min(local,numberofemployees)", "Read Accounts: numberofemployees;")] - [InlineData("Average(local,numberofemployees)", "Read Accounts: numberofemployees;")] + [InlineData("Min(local,numberofemployees)", "Entity Accounts: numberofemployees;")] + [InlineData("Average(local,numberofemployees)", "Entity Accounts: numberofemployees;")] // Patch - [InlineData("Patch(local, First(local), { 'Account Name' : \"some name\"})", "Read Accounts: ; Write Accounts: name;")] - [InlineData("Patch(local, {'Address 1: City':\"test\"}, { 'Account Name' : \"some name\"})", "Read Accounts: address1_city; Write Accounts: name;")] - [InlineData("Patch(local, {accountid:GUID(), 'Address 1: City':\"test\"})", "Read Accounts: accountid; Write Accounts: address1_city;")] - [InlineData("Patch(local, Table({accountid:GUID(), 'Address 1: City':\"test\"},{accountid:GUID(), 'Account Name':\"test\"}))", "Read Accounts: accountid; Write Accounts: address1_city, name;")] - [InlineData("Patch(local, Table({accountid:GUID(), 'Address 1: City':\"test\"},{accountid:GUID(), 'Account Name':\"test\"}),Table({'Address 1: City':\"test\"},{'Address 1: City':\"test\",'Account Name':\"test\"}))", "Read Accounts: accountid, address1_city, name; Write Accounts: address1_city, name;")] - [InlineData("Patch(local, First(local), { 'Address 1: City' : First(local).'Account Name' } )", "Read Accounts: name; Write Accounts: address1_city;")] + [InlineData("Patch(simple2, First(simple1), { a : 1 })", "Entity Simple1: a, b; Entity Simple2: a, b;")] + [InlineData("Patch(local, {'Address 1: City':\"test\"}, { 'Account Name' : \"some name\"})", "Entity Accounts: address1_city, name;")] + [InlineData("Patch(local, {accountid:GUID(), 'Address 1: City':\"test\"})", "Entity Accounts: accountid, address1_city;")] + [InlineData("Patch(local, Table({accountid:GUID(), 'Address 1: City':\"test\"},{accountid:GUID(), 'Account Name':\"test\"}))", "Entity Accounts: accountid, address1_city, name;")] + [InlineData("Patch(local, Table({accountid:GUID(), 'Address 1: City':\"test\"},{accountid:GUID(), 'Account Name':\"test\"}),Table({'Address 1: City':\"test\"},{'Address 1: City':\"test\",'Account Name':\"test\"}))", "Entity Accounts: accountid, address1_city, name;")] + [InlineData("Patch(simple2, First(simple1), { a : First(simple1).b } )", "Entity Simple1: a, b; Entity Simple2: a, b;")] + [InlineData("Patch(simple1, First(simple1), { a : First(simple1).b } )", "Entity Simple1: a, b;")] // Remove - [InlineData("Remove(local, {name: First(remote).name})", "Read Contacts: name; Write Accounts: name;")] + [InlineData("Remove(local, {name: First(remote).name})", "Entity Accounts: name; Entity Contacts: name;")] // Collect and ClearCollect. - [InlineData("Collect(local, Table({ 'Account Name' : \"some name\"}))", "Write Accounts: name;")] - [InlineData("Collect(local, local)", "Write Accounts: ;")] - [InlineData("Collect(local, { 'Address 1: City' : First(local).'Account Name' })", "Read Accounts: name; Write Accounts: address1_city;")] - [InlineData("ClearCollect(local, local)", "Write Accounts: ;")] - [InlineData("ClearCollect(local, Table({ 'Account Name' : \"some name\"}))", "Write Accounts: name;")] + [InlineData("Collect(local, Table({ 'Account Name' : \"some name\"}))", "Entity Accounts: name;")] + [InlineData("Collect(simple2, simple1)", "Entity Simple2: a, b; Entity Simple1: a, b;")] + [InlineData("Collect(simple2, { a : First(simple1).b })", "Entity Simple2: a; Entity Simple1: b;")] + [InlineData("Collect(local, { 'Address 1: City' : First(remote).'Contact Name' })", "Entity Accounts: address1_city; Entity Contacts: name;")] + [InlineData("ClearCollect(simple2, simple1)", "Entity Simple2: a, b; Entity Simple1: a, b;")] + [InlineData("ClearCollect(local, Table({ 'Account Name' : \"some name\"}))", "Entity Accounts: name;")] // Inside with. - [InlineData("With({r: local}, Filter(r, 'Number of employees' > 0))", "Read Accounts: numberofemployees;")] - [InlineData("With({r: local}, LookUp(r, 'Number of employees' > 0))", "Read Accounts: numberofemployees;")] + [InlineData("With({r: local}, Filter(r, 'Number of employees' > 0))", "Entity Accounts: numberofemployees;")] + [InlineData("With({r: local}, LookUp(r, 'Number of employees' > 0))", "Entity Accounts: numberofemployees;")] // Option set. - [InlineData("Filter(local, dayofweek = StartOfWeek.Monday)", "Read Accounts: dayofweek;")] + [InlineData("Filter(local, dayofweek = StartOfWeek.Monday)", "Entity Accounts: dayofweek;")] - [InlineData("Filter(ForAll(local, ThisRecord.numberofemployees), Value < 20)", "Read Accounts: numberofemployees;")] + [InlineData("Filter(ForAll(local, ThisRecord.numberofemployees), Value < 20)", "Entity Accounts: numberofemployees;")] // Summarize is special, becuase of ThisGroup. - [InlineData("Summarize(local, 'Account Name', Sum(ThisGroup, numberofemployees) As Employees)", "Read Accounts: name, numberofemployees;")] - [InlineData("Summarize(local, 'Account Name', Sum(ThisGroup, numberofemployees * 2) As TPrice)", "Read Accounts: name, numberofemployees;")] + [InlineData("Summarize(local, 'Account Name', Sum(ThisGroup, numberofemployees) As Employees)", "Entity Accounts: name, numberofemployees;")] + [InlineData("Summarize(local, 'Account Name', Sum(ThisGroup, numberofemployees * 2) As TPrice)", "Entity Accounts: name, numberofemployees;")] // Join - [InlineData("Join(remote As l, local As r, l.contactid = r.contactid, JoinType.Inner, r.name As AccountName)", "Read Contacts: contactid; Read Accounts: contactid, name;")] - [InlineData("Join(remote As l, local As r, l.contactid = r.contactid, JoinType.Inner, r.name As AccountName, l.contactnumber As NewContactNumber)", "Read Contacts: contactid, contactnumber; Read Accounts: contactid, name;")] + [InlineData("Join(remote As l, local As r, l.contactid = r.contactid, JoinType.Inner, r.name As AccountName)", "Entity Contacts: contactid; Entity Accounts: contactid, name;")] + [InlineData("Join(remote As l, local As r, l.contactid = r.contactid, JoinType.Inner, r.name As AccountName, l.contactnumber As NewContactNumber)", "Entity Contacts: contactid, contactnumber; Entity Accounts: contactid, name;")] + [InlineData("Join(remote, local, LeftRecord.contactid = RightRecord.contactid, JoinType.Inner, RightRecord.name As AccountName, LeftRecord.contactnumber As NewContactNumber)", "Entity Contacts: contactid, contactnumber; Entity Accounts: contactid, name;")] // Set - [InlineData("Set(numberofemployees, 200)", "Write Accounts: numberofemployees;")] - [InlineData("Set('Address 1: City', 'Account Name')", "Read Accounts: name; Write Accounts: address1_city;")] - [InlineData("Set('Address 1: City', 'Address 1: City' & \"test\")", "Read Accounts: address1_city; Write Accounts: address1_city;")] - [InlineData("Set(NewRecord.'Address 1: City', \"test\")", "Write Accounts: address1_city;")] + [InlineData("Set(numberofemployees, 200)", "Entity Accounts: numberofemployees;")] + [InlineData("Set('Address 1: City', 'Account Name')", "Entity Accounts: address1_city, name;")] + [InlineData("Set('Address 1: City', 'Address 1: City' & \"test\")", "Entity Accounts: address1_city;")] + [InlineData("Set(NewRecord.'Address 1: City', \"test\")", "Entity Accounts: address1_city;")] + + [InlineData("Filter(Distinct(ShowColumns(simple2, a, b), a), Value < 20)", "Entity Simple2: a, b;")] + [InlineData("Filter(Distinct(DropColumns(simple2, c), a), Value < 20)", "Entity Simple2: c, a;")] public void GetDependencies(string expr, string expected) { var opt = new ParserOptions() { AllowsSideEffects = true }; @@ -95,9 +105,11 @@ public void GetDependencies(string expr, string expected) .SetText(expr, opt) .SetBindingInfo(GetSymbols()); - check.ApplyBinding(); + check.ApplyBinding(); +#pragma warning disable CS0618 // Type or member is obsolete var info = check.ApplyDependencyInfoScan(); +#pragma warning restore CS0618 // Type or member is obsolete var actual = info.ToString().Replace("\r", string.Empty).Replace("\n", string.Empty).Trim(); Assert.Equal(expected, actual); } @@ -106,6 +118,8 @@ private ReadOnlySymbolTable GetSymbols() { var localType = Accounts(); var remoteType = Contacts(); + var simple1Type = Simple1(); + var simple2Type = Simple2(); var customSymbols = new SymbolTable { DebugName = "Custom symbols " }; var opt = new ParserOptions() { AllowsSideEffects = true }; @@ -123,8 +137,11 @@ private ReadOnlySymbolTable GetSymbols() customSymbols.AddFunction(new SummarizeFunction()); customSymbols.AddFunction(new RecalcEngineSetFunction()); customSymbols.AddFunction(new RemoveFunction()); + customSymbols.AddFunction(Library.DistinctInterpreterFunction); customSymbols.AddVariable("local", localType, mutable: true); customSymbols.AddVariable("remote", remoteType, mutable: true); + customSymbols.AddVariable("simple1", simple1Type, mutable: true); + customSymbols.AddVariable("simple2", simple2Type, mutable: true); // Simulate a parameter var parameterSymbols = new SymbolTable { DebugName = "Parameters " }; @@ -165,6 +182,38 @@ private TableType Contacts() return (TableType)FormulaType.Build(contactType); } + // Some test cases can produce a long list of dependencies. + // This method is used to simplify the schema for those cases. + private TableType Simple1() + { + var simplifiedchema = "*[a:w,b:w]"; + + DType type = TestUtils.DT2(simplifiedchema); + var dataSource = new TestDataSource( + "Simple1", + type, + keyColumns: new[] { "a" }); + + type = DType.AttachDataSourceInfo(type, dataSource); + + return (TableType)FormulaType.Build(type); + } + + private TableType Simple2() + { + var simplifiedchema = "*[a:w,b:w,c:s]"; + + DType type = TestUtils.DT2(simplifiedchema); + var dataSource = new TestDataSource( + "Simple2", + type, + keyColumns: new[] { "a" }); + + type = DType.AttachDataSourceInfo(type, dataSource); + + return (TableType)FormulaType.Build(type); + } + /// /// This test case is to ensure that all functions that are not self-contained or /// have a scope info have been assessed and either added to the exception list or overrides . @@ -179,11 +228,9 @@ public void DepedencyScanFunctionTests() // These methods use default implementation of ComposeDependencyInfo and do not neeed to override it. var exceptionList = new HashSet() { - "AddColumns", "Average", "Concat", "CountIf", - "DropColumns", "Filter", "ForAll", "IfError", @@ -191,11 +238,8 @@ public void DepedencyScanFunctionTests() "Max", "Min", "Refresh", - "RenameColumns", "Search", - "ShowColumns", "Sort", - "SortByColumns", "StdevP", "Sum", "Trace",