From 4e89b28e54eccd6bd22165286aac543f1b171e9b Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Mon, 9 Dec 2024 12:25:00 -0600 Subject: [PATCH 01/15] initial --- .../Localization/Strings.cs | 11 +- .../Public/Config/Features.cs | 5 + .../Public/Values/CollectionTableValue.cs | 2 +- .../Microsoft.PowerFx.Core/Texl/Remove.cs | 417 ++++++++++++++++++ .../Types/Enums/BuiltInEnums.cs | 9 + .../Types/Enums/EnumStoreBuilder.cs | 5 + .../Utils/LanguageConstants.cs | 5 + .../Environment/PowerFxConfigExtensions.cs | 3 +- .../Functions/LibraryMutation.cs | 18 + .../Functions/Mutation/MutationUtils.cs | 94 +++- .../Functions/Mutation/RemoveFunction.cs | 239 ---------- .../ExpressionTestCases/Remove.txt | 48 +- .../MutationScripts/AndOr_V1Compat.txt | 8 +- .../MutationScripts/Remove.txt | 18 + .../MutationScripts/Remove_V1Compat.txt | 22 - .../Remove_V1CompatDisabled.txt | 22 - .../PADIntegrationTests.cs | 2 +- 17 files changed, 609 insertions(+), 319 deletions(-) create mode 100644 src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs delete mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/RemoveFunction.cs create mode 100644 src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt delete mode 100644 src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove_V1Compat.txt delete mode 100644 src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove_V1CompatDisabled.txt diff --git a/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs b/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs index 0bb73cce3a..b5ee24186a 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs @@ -523,8 +523,10 @@ internal static class TexlStrings public static StringGetter AboutClearCollect = (b) => StringResources.Get("AboutClearCollect", b); public static StringGetter AboutRemove = (b) => StringResources.Get("AboutRemove", b); - public static StringGetter RemoveDataSourceArg = (b) => StringResources.Get("RemoveDataSourceArg", b); - public static StringGetter RemoveRecordsArg = (b) => StringResources.Get("RemoveRecordsArg", b); + public static StringGetter RemoveArg1 = (b) => StringResources.Get("RemoveArg1", b); + public static StringGetter RemoveArg2 = (b) => StringResources.Get("RemoveArg2", b); + public static StringGetter RemoveArg3 = (b) => StringResources.Get("RemoveArg3", b); + public static StringGetter RemoveAllArg2 = (b) => StringResources.Get("RemoveAllArg2", b); public static StringGetter AboutDec2Hex = (b) => StringResources.Get("AboutDec2Hex", b); public static StringGetter Dec2HexArg1 = (b) => StringResources.Get("Dec2HexArg1", b); @@ -655,6 +657,7 @@ internal static class TexlStrings public static ErrorResourceKey ErrBadType_ExpectedType_ProvidedType = new ErrorResourceKey("ErrBadType_ExpectedType_ProvidedType"); public static ErrorResourceKey ErrBadType_VoidExpression = new ErrorResourceKey("ErrBadType_VoidExpression"); public static ErrorResourceKey ErrBadSchema_ExpectedType = new ErrorResourceKey("ErrBadSchema_ExpectedType"); + public static ErrorResourceKey ErrNeedTable_Arg = new ErrorResourceKey("ErrNeedTable_Arg"); public static ErrorResourceKey ErrInvalidArgs_Func = new ErrorResourceKey("ErrInvalidArgs_Func"); public static ErrorResourceKey ErrNeedTable_Func = new ErrorResourceKey("ErrNeedTable_Func"); public static ErrorResourceKey ErrNeedTableCol_Func = new ErrorResourceKey("ErrNeedTableCol_Func"); @@ -868,5 +871,9 @@ internal static class TexlStrings public static ErrorResourceKey ErrJoinArgIsNotAsNode = new ErrorResourceKey("ErrJoinArgIsNotAsNode"); public static ErrorResourceKey ErrJoinAtLeastOneRigthRecordField = new ErrorResourceKey("ErrJoinAtLeastOneRigthRecordField"); public static ErrorResourceKey ErrJoinDottedNameleft = new ErrorResourceKey("ErrJoinDottedNameleft"); + + public static ErrorResourceKey ErrCollectionDoesNotAcceptThisType = new ErrorResourceKey("ErrCollectionDoesNotAcceptThisType"); + public static ErrorResourceKey ErrNeedAll = new ErrorResourceKey("ErrNeedAll"); + public static ErrorResourceKey ErrNeedCollection_Func = new ErrorResourceKey("ErrNeedCollection_Func"); } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs b/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs index abf908f462..8a4a109439 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs @@ -82,6 +82,11 @@ public sealed class Features /// internal bool IsUserDefinedTypesEnabled { get; init; } = false; + /// + /// Enables Remove All delegation. + /// + internal bool IsRemoveAllDelegationEnabled { get; set; } + internal static readonly Features None = new Features(); /// diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs b/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs index fee7d280c7..5b967c555e 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs @@ -220,7 +220,7 @@ public override async Task> RemoveAsync(IEnumerable true; + + public override bool ModifiesValues => true; + + public override bool CanSuggestInputColumns => true; + + public override bool IsSelfContained => false; + + public override bool RequiresDataSourceScope => true; + + public override bool SupportsParamCoercion => false; + + public override RequiredDataSourcePermissions FunctionPermission => RequiredDataSourcePermissions.Delete; + + // Return true if this function affects datasource query options. + public override bool AffectsDataSourceQueryOptions => true; + + public override bool MutatesArg(int argIndex, TexlNode arg) => argIndex == 0; + + public override bool RequireAllParamColumns => true; + + public RemoveFunctionBase(int arityMax, params DType[] paramTypes) + : base("Remove", TexlStrings.AboutRemove, FunctionCategories.Behavior, DType.EmptyTable, 0, 2, arityMax, paramTypes) + { + } + + public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DType[] argTypes, IErrorContainer errors, out DType returnType, out Dictionary nodeToCoercedTypeMap) + { + Contracts.AssertValue(args); + Contracts.AssertAllValues(args); + Contracts.AssertValue(argTypes); + Contracts.Assert(args.Length == argTypes.Length); + Contracts.AssertValue(errors); + Contracts.Assert(MinArity <= args.Length && args.Length <= MaxArity); + + bool fValid = base.CheckTypes(context, args, argTypes, errors, out returnType, out nodeToCoercedTypeMap); + Contracts.Assert(returnType.IsTable); + + DType collectionType = argTypes[0]; + if (!collectionType.IsTable) + { + fValid = false; + errors.EnsureError(args[0], TexlStrings.ErrNeedCollection_Func, Name); + } + + // !!!TODO New stuff. In case this doesnt work, delete from here to the end of the function + //var argCount = argTypes.Count(); + + //for (int i = 1; i < argCount; i++) + //{ + // var argType = argTypes[i]; + // var arg = args[i]; + + // if (!argType.IsAggregate) + // { + // if (argCount >= 3 && i == argCount - 1) + // { + // fValid = (context.Features.StronglyTypedBuiltinEnums && BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) || + // (!context.Features.StronglyTypedBuiltinEnums && DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)); + + // if (!fValid) + // { + // errors.EnsureError(DocumentErrorSeverity.Severe, arg, TexlStrings.ErrRemoveAllArg); + // } + // } + // else + // { + // fValid = false; + // errors.EnsureError(args[i], TexlStrings.ErrNeedRecord_Arg, args[i]); + // } + + // continue; + // } + + // var collectionAcceptsRecord = collectionType.Accepts(argType.ToTable(), exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules); + // var recordAcceptsCollection = argType.ToTable().Accepts(collectionType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules); + + // // PFxV1 is more restrictive than PA in terms of column matching. If the collection does not accept the record or vice versa, it is an error. + // if ((context.Features.PowerFxV1CompatibilityRules && (!collectionAcceptsRecord || !recordAcceptsCollection)) || + // (!context.Features.PowerFxV1CompatibilityRules && (!collectionAcceptsRecord && !recordAcceptsCollection))) + // { + // fValid = false; + // SetErrorForMismatchedColumns(collectionType, argType, arg, errors, context.Features); + // } + + // // Only warn about no-op record inputs if there are no data sources that would use reference identity for comparison. + // else if (!collectionType.AssociatedDataSources.Any() && !recordAcceptsCollection) + // { + // errors.EnsureError(DocumentErrorSeverity.Warning, args[i], TexlStrings.ErrCollectionDoesNotAcceptThisType); + // } + //} + + returnType = context.Features.PowerFxV1CompatibilityRules ? DType.Void : collectionType; + + return fValid; + } + + public override void CheckSemantics(TexlBinding binding, TexlNode[] args, DType[] argTypes, IErrorContainer errors) + { + base.CheckSemantics(binding, args, argTypes, errors); + base.ValidateArgumentIsMutable(binding, args[0], errors); + MutationUtils.CheckSemantics(binding, this, args, argTypes, errors); + } + + public override IEnumerable GetRequiredEnumNames() + { + return new List() { LanguageConstants.RemoveFlagsEnumString }; + } + + public override bool IsLazyEvalParam(TexlNode node, int index, Features features) + { + // First argument to mutation functions is Lazy for datasources that are copy-on-write. + // If there are any side effects in the arguments, we want those to have taken place before we make the copy. + return index == 0; + } + + public override IEnumerable GetIdentifierOfModifiedValue(TexlNode[] args, out TexlNode identifierNode) + { + Contracts.AssertValue(args); + + identifierNode = null; + if (args.Length == 0) + { + return null; + } + + var firstNameNode = args[0]?.AsFirstName(); + identifierNode = firstNameNode; + if (firstNameNode == null) + { + return null; + } + + var identifiers = new List + { + firstNameNode.Ident + }; + + return identifiers; + } + + public override bool IsAsyncInvocation(CallNode callNode, TexlBinding binding) + { + Contracts.AssertValue(callNode); + Contracts.AssertValue(binding); + + return Arg0RequiresAsync(callNode, binding); + } + } + + // Remove(collection:*[], item1:![], item2:![], ..., ["All"]) + internal class RemoveFunction : RemoveFunctionBase, ISuggestionAwareFunction + { + public bool CanSuggestThisItem => true; + + public override bool ArgMatchesDatasourceType(int argNum) + { + return argNum >= 1; + } + + public override bool TryGetTypeForArgSuggestionAt(int argIndex, out DType type) + { + if (argIndex > 0) + { + type = default; + return false; + } + + return base.TryGetTypeForArgSuggestionAt(argIndex, out type); + } + + public RemoveFunction() + : base(int.MaxValue, DType.EmptyTable) + { + } + + public override IEnumerable GetSignatures() + { + yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveArg2 }; + yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveArg2, TexlStrings.RemoveArg2 }; + yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveArg2, TexlStrings.RemoveArg2, TexlStrings.RemoveArg2 }; + } + + public override IEnumerable GetSignatures(int arity) + { + if (arity > 2) + { + return GetGenericSignatures(arity, TexlStrings.RemoveArg1, TexlStrings.RemoveArg2); + } + + return base.GetSignatures(arity); + } + + // This method returns true if there are special suggestions for a particular parameter of the function. + public override bool HasSuggestionsForParam(int argumentIndex) + { + Contracts.Assert(argumentIndex >= 0); + + return argumentIndex != 1; + } + + /// + /// As Remove uses the source record in it's entirety to find the entry in table, uses deepcompare at runtime, we need all fields from source. + /// So update the selects for all columns in the source in this case except when datasource is pageable. + /// In that case, we can get the info at runtime. + /// + public override bool UpdateDataQuerySelects(CallNode callNode, TexlBinding binding, DataSourceToQueryOptionsMap dataSourceToQueryOptionsMap) + { + Contracts.AssertValue(callNode); + Contracts.AssertValue(binding); + + if (!CheckArgsCount(callNode, binding)) + { + return false; + } + + var args = Contracts.VerifyValue(callNode.Args.Children); + + DType dsType = binding.GetType(args[0]); + if (dsType.AssociatedDataSources == null + || dsType == DType.EmptyTable) + { + return false; + } + + var sourceRecordType = binding.GetType(args[1]); + + // This might be the case where Remove(CDS, Gallery.Selected) + if (sourceRecordType == DType.EmptyRecord) + { + return false; + } + + var firstTypeName = sourceRecordType.GetNames(DPath.Root).FirstOrDefault(); + + if (!firstTypeName.IsValid) + { + return false; + } + + DType type = firstTypeName.Type; + DName columnName = firstTypeName.Name; + + // This might be the case where Remove(CDS, Gallery.Selected) + if (!dsType.Contains(columnName)) + { + return false; + } + + dsType.AssociateDataSourcesToSelect( + dataSourceToQueryOptionsMap, + columnName, + type, + false /*skipIfNotInSchema*/, + true); /*skipExpands*/ + + return true; + } + + // This method filters for a table as the first parameter, records as intermediate parameters + // and a string (First/All) as the final parameter. + public override bool IsSuggestionTypeValid(int paramIndex, DType type) + { + Contracts.Assert(paramIndex >= 0); + Contracts.AssertValid(type); + + if (paramIndex >= MaxArity) + { + return false; + } + + if (paramIndex == 0) + { + return type.IsTable; + } + + // String suggestions for column names may occur within the context of a record + return type.IsRecord || type.Kind == DKind.String; + } + + public override bool IsAsyncInvocation(CallNode callNode, TexlBinding binding) + { + Contracts.AssertValue(callNode); + Contracts.AssertValue(binding); + + return Arg0RequiresAsync(callNode, binding); + } + + protected override bool RequiresPagedDataForParamCore(TexlNode[] args, int paramIndex, TexlBinding binding) + { + Contracts.AssertValue(args); + Contracts.AssertAllValues(args); + Contracts.Assert(paramIndex >= 0 && paramIndex < args.Length); + Contracts.AssertValue(binding); + Contracts.Assert(binding.IsPageable(Contracts.VerifyValue(args[paramIndex]))); + + // For the first argument, we need only metadata. No actual data from datasource is required. + return paramIndex > 0; + } + } + + // Remove(collection:*[], source:*[], ["All"]) + internal class RemoveAllFunction : RemoveFunctionBase + { + public override bool ArgMatchesDatasourceType(int argNum) + { + return argNum == 1; + } + + public RemoveAllFunction() + : base(3, DType.EmptyTable, DType.EmptyTable, DType.String) + { + } + + public override IEnumerable GetSignatures() + { + yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveAllArg2 }; + yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveAllArg2, TexlStrings.RemoveArg3 }; + } + + public override bool IsLazyEvalParam(TexlNode node, int index, Features features) + { + // First argument to mutation functions is Lazy for datasources that are copy-on-write. + // If there are any side effects in the arguments, we want those to have taken place before we make the copy. + return index == 0; + } + + // This method returns true if there are special suggestions for a particular parameter of the function. + public override bool HasSuggestionsForParam(int argumentIndex) + { + Contracts.Assert(argumentIndex >= 0); + + return argumentIndex == 0; + } + + public override bool IsServerDelegatable(CallNode callNode, TexlBinding binding) + { + Contracts.AssertValue(callNode); + Contracts.AssertValue(binding); + + if (!CheckArgsCount(callNode, binding)) + { + return false; + } + + // Use ECS flag as a guard. + if (binding.Document != null) + { + if (!binding.Features.IsRemoveAllDelegationEnabled) + { + return false; + } + } + + if (!binding.TryGetDataSourceInfo(callNode.Args.Children[0], out IExternalDataSource dataSource)) + { + return false; + } + + // Currently we delegate only to CDS. This is the first version of the feature and not a limitation of other datasources + if (dataSource == null || dataSource?.Kind != DataSourceKind.CdsNative) + { + TrackingProvider.Instance.SetDelegationTrackerStatus(DelegationStatus.DataSourceNotDelegatable, callNode, binding, this); + return false; + } + + // Right now we delegate only if the set of records is a table/queried table to mitigate the performance impact of the remove operation. + // Deleting single records (via Lookup) does not have the same performance impact + var dsType = binding.GetType(callNode.Args.Children[1]).Kind; + if (dsType != DKind.Table) + { + TrackingProvider.Instance.SetDelegationTrackerStatus(DelegationStatus.InvalidArgType, callNode, binding, this); + return false; + } + + TrackingProvider.Instance.SetDelegationTrackerStatus(DelegationStatus.DelegationSuccessful, callNode, binding, this); + return true; + } + + public override bool SupportsPaging(CallNode callNode, TexlBinding binding) + { + if (!binding.TryGetDataSourceInfo(callNode.Args.Children[0], out IExternalDataSource dataSource)) + { + return false; + } + + // Currently we delegate only to CDS. This is the first version of the feature and not a limitation of other datasources + if (dataSource == null || dataSource?.Kind == DataSourceKind.CdsNative) + { + return false; + } + + return base.SupportsPaging(callNode, binding); + } + } +} diff --git a/src/libraries/Microsoft.PowerFx.Core/Types/Enums/BuiltInEnums.cs b/src/libraries/Microsoft.PowerFx.Core/Types/Enums/BuiltInEnums.cs index e560eb2c06..ddbfc8f736 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Types/Enums/BuiltInEnums.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Types/Enums/BuiltInEnums.cs @@ -210,5 +210,14 @@ private static Dictionary TraceSeverityDictionary() { "Right", "right" }, { "Full", "full" }, }); + + public static readonly EnumSymbol RemoveFlagsEnum = new EnumSymbol( + new DName(LanguageConstants.RemoveFlagsEnumString), + DType.String, + new Dictionary() + { + { "First", "first" }, + { "All", "all" }, + }); } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Types/Enums/EnumStoreBuilder.cs b/src/libraries/Microsoft.PowerFx.Core/Types/Enums/EnumStoreBuilder.cs index 435a06a989..8d1159fed1 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Types/Enums/EnumStoreBuilder.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Types/Enums/EnumStoreBuilder.cs @@ -31,6 +31,7 @@ internal sealed class EnumStoreBuilder { LanguageConstants.TraceSeverityEnumString, BuiltInEnums.TraceSeverityEnum }, { LanguageConstants.TraceOptionsEnumString, BuiltInEnums.TraceOptionsEnum }, { LanguageConstants.JoinTypeEnumString, BuiltInEnums.JoinTypeEnum }, + { LanguageConstants.RemoveFlagsEnumString, BuiltInEnums.RemoveFlagsEnum }, }; // DefaultEnums, with enum strings, is legacy and only used by Power Apps @@ -84,6 +85,10 @@ internal sealed class EnumStoreBuilder { LanguageConstants.JoinTypeEnumString, $"%s[{string.Join(", ", BuiltInEnums.JoinTypeEnum.EnumType.ValueTree.GetPairs().Select(pair => $@"{pair.Key}:""{pair.Value.Object}"""))}]" + }, + { + LanguageConstants.RemoveFlagsEnumString, + $"%s[{string.Join(", ", BuiltInEnums.RemoveFlagsEnum.EnumType.ValueTree.GetPairs().Select(pair => $@"{pair.Key}:""{pair.Value.Object}"""))}]" } }; #endregion diff --git a/src/libraries/Microsoft.PowerFx.Core/Utils/LanguageConstants.cs b/src/libraries/Microsoft.PowerFx.Core/Utils/LanguageConstants.cs index ca301dc4f8..ea341c9f2e 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Utils/LanguageConstants.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Utils/LanguageConstants.cs @@ -90,5 +90,10 @@ internal class LanguageConstants /// The string value representing the join type literal. /// public const string JoinTypeEnumString = "JoinType"; + + /// + /// The string value representing the remove flag option. + /// + public const string RemoveFlagsEnumString = "RemoveFlags"; } } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Environment/PowerFxConfigExtensions.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Environment/PowerFxConfigExtensions.cs index 78be4b4db7..d8dd875a99 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Environment/PowerFxConfigExtensions.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Environment/PowerFxConfigExtensions.cs @@ -49,7 +49,8 @@ public static void EnableMutationFunctions(this SymbolTable symbolTable) symbolTable.AddFunction(new PatchSingleRecordImpl()); symbolTable.AddFunction(new PatchAggregateImpl()); symbolTable.AddFunction(new PatchAggregateSingleTableImpl()); - symbolTable.AddFunction(new RemoveFunction()); + symbolTable.AddFunction(new RemoveImpl()); + symbolTable.AddFunction(new RemoveAllImpl()); symbolTable.AddFunction(new ClearImpl()); symbolTable.AddFunction(new ClearCollectImpl()); symbolTable.AddFunction(new ClearCollectScalarImpl()); diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs index bbb726a7ca..cc8c2599d5 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs @@ -402,4 +402,22 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ return await new ClearCollectImpl().InvokeAsync(irContext, args, cancellationToken).ConfigureAwait(false); } } + + // Remove(collection:*[], item1:![], item2:![], ..., ["All"]) + internal class RemoveImpl : RemoveFunction, IAsyncTexlFunction3 + { + public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + { + return await MutationUtils.RemoveCore(irContext, args, cancellationToken).ConfigureAwait(false); + } + } + + // Remove(collection:*[], source:*[], ["All"]) + internal class RemoveAllImpl : RemoveAllFunction, IAsyncTexlFunction3 + { + public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + { + return await MutationUtils.RemoveCore(irContext, args, cancellationToken).ConfigureAwait(false); + } + } } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/MutationUtils.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/MutationUtils.cs index b970442c79..f57c52bc59 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/MutationUtils.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/MutationUtils.cs @@ -3,12 +3,10 @@ using System.Collections.Generic; using System.Linq; -using Microsoft.PowerFx.Core.App.ErrorContainers; -using Microsoft.PowerFx.Core.Entities; -using Microsoft.PowerFx.Core.Errors; -using Microsoft.PowerFx.Core.Localization; -using Microsoft.PowerFx.Core.Types; -using Microsoft.PowerFx.Syntax; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.PowerFx.Core.IR; +using Microsoft.PowerFx.Interpreter.Localization; using Microsoft.PowerFx.Types; namespace Microsoft.PowerFx.Interpreter @@ -47,5 +45,89 @@ public static DValue MergeRecords(IEnumerable records return DValue.Of(FormulaValue.NewRecordFromFields(mergedFields.Select(kvp => new NamedValue(kvp.Key, kvp.Value)))); } + + public static async Task RemoveCore(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + FormulaValue arg0; + + if (args[0] is LambdaFormulaValue arg0lazy) + { + arg0 = await arg0lazy.EvalAsync().ConfigureAwait(false); + } + else + { + arg0 = args[0]; + } + + if (arg0 is BlankValue || arg0 is ErrorValue) + { + return arg0; + } + + // If any of the argN (N>0) is error, return the error. + foreach (var arg in args.Skip(1)) + { + cancellationToken.ThrowIfCancellationRequested(); + + if (arg is ErrorValue) + { + return arg; + } + + if (arg is TableValue tableValue) + { + var errorRecord = tableValue.Rows.First(row => row.IsError); + if (errorRecord != null) + { + return errorRecord.Error; + } + } + } + + var all = false; + var datasource = (TableValue)arg0; + + if (args.Count() >= 3 && args.Last() is OptionSetValue opv) + { + all = opv.Option == "All"; + } + + List recordsToRemove = null; + + if (args[1] is TableValue sourceTable) + { + recordsToRemove = sourceTable.Rows.Select(row => row.Value).ToList(); + } + else + { + recordsToRemove = args + .Skip(1) + .Where(arg => arg is RecordValue) + .OfType() + .ToList(); + } + + // At this point all errors have been handled. + var response = await datasource.RemoveAsync(recordsToRemove, all, cancellationToken).ConfigureAwait(false); + + if (response.IsError) + { + var errors = new List(); + foreach (var error in response.Error.Errors) + { + errors.Add(new ExpressionError() + { + ResourceKey = RuntimeStringResources.ErrRecordNotFound, + Kind = ErrorKind.NotFound + }); + } + + return new ErrorValue(IRContext.NotInSource(irContext), errors); + } + + return irContext == FormulaType.Void ? FormulaValue.NewVoid() : FormulaValue.NewBlank(); + } } } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/RemoveFunction.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/RemoveFunction.cs deleted file mode 100644 index e0bb724273..0000000000 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/RemoveFunction.cs +++ /dev/null @@ -1,239 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System.Collections.Generic; -using System.Linq; -using System.Numerics; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.PowerFx.Core.App.ErrorContainers; -using Microsoft.PowerFx.Core.Binding; -using Microsoft.PowerFx.Core.Errors; -using Microsoft.PowerFx.Core.Functions; -using Microsoft.PowerFx.Core.Localization; -using Microsoft.PowerFx.Core.Types; -using Microsoft.PowerFx.Core.Utils; -using Microsoft.PowerFx.Syntax; -using Microsoft.PowerFx.Types; -using static Microsoft.PowerFx.Core.Localization.TexlStrings; -using static Microsoft.PowerFx.Syntax.PrettyPrintVisitor; - -namespace Microsoft.PowerFx.Functions -{ - internal abstract class RemoveFunctionBase : BuiltinFunction - { - public override bool IsSelfContained => false; - - public override bool RequiresDataSourceScope => true; - - public override bool CanSuggestInputColumns => true; - - public override bool ManipulatesCollections => true; - - public override bool ArgMatchesDatasourceType(int argNum) - { - return argNum >= 1; - } - - public override bool MutatesArg(int argIndex, TexlNode arg) => argIndex == 0; - - public override bool IsLazyEvalParam(TexlNode node, int index, Features features) - { - // First argument to mutation functions is Lazy for datasources that are copy-on-write. - // If there are any side effects in the arguments, we want those to have taken place before we make the copy. - return index == 0; - } - - public RemoveFunctionBase(DPath theNamespace, string name, StringGetter description, FunctionCategories fc, DType returnType, BigInteger maskLambdas, int arityMin, int arityMax, params DType[] paramTypes) - : base(theNamespace, name, /*localeSpecificName*/string.Empty, description, fc, returnType, maskLambdas, arityMin, arityMax, paramTypes) - { - } - - public RemoveFunctionBase(string name, StringGetter description, FunctionCategories fc, DType returnType, BigInteger maskLambdas, int arityMin, int arityMax, params DType[] paramTypes) - : this(DPath.Root, name, description, fc, returnType, maskLambdas, arityMin, arityMax, paramTypes) - { - } - - protected static bool CheckArgs(FormulaValue[] args, out FormulaValue faultyArg) - { - // If any args are error, propagate up. - foreach (var arg in args) - { - if (arg is ErrorValue) - { - faultyArg = arg; - - return false; - } - } - - faultyArg = null; - - return true; - } - } - - internal class RemoveFunction : RemoveFunctionBase, IAsyncTexlFunction3 - { - public override bool IsSelfContained => false; - - public override bool TryGetTypeForArgSuggestionAt(int argIndex, out DType type) - { - if (argIndex == 1) - { - type = default; - return false; - } - - return base.TryGetTypeForArgSuggestionAt(argIndex, out type); - } - - public RemoveFunction() - : base("Remove", AboutRemove, FunctionCategories.Table | FunctionCategories.Behavior, DType.Unknown, 0, 2, int.MaxValue, DType.EmptyTable, DType.EmptyRecord) - { - } - - public override IEnumerable GetSignatures() - { - yield return new[] { RemoveDataSourceArg, RemoveRecordsArg }; - yield return new[] { RemoveDataSourceArg, RemoveRecordsArg, RemoveRecordsArg }; - } - - public override IEnumerable GetSignatures(int arity) - { - if (arity > 2) - { - return GetGenericSignatures(arity, RemoveDataSourceArg, RemoveRecordsArg, RemoveRecordsArg); - } - - return base.GetSignatures(arity); - } - - public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DType[] argTypes, IErrorContainer errors, out DType returnType, out Dictionary nodeToCoercedTypeMap) - { - Contracts.AssertValue(args); - Contracts.AssertAllValues(args); - Contracts.AssertValue(argTypes); - Contracts.Assert(args.Length == argTypes.Length); - Contracts.AssertValue(errors); - Contracts.Assert(MinArity <= args.Length && args.Length <= MaxArity); - - var fValid = base.CheckTypes(context, args, argTypes, errors, out returnType, out nodeToCoercedTypeMap); - - DType collectionType = argTypes[0]; - if (!collectionType.IsTable) - { - errors.EnsureError(args[0], ErrNeedTable_Func, Name); - fValid = false; - } - - var argCount = argTypes.Length; - - for (var i = 1; i < argCount; i++) - { - DType argType = argTypes[i]; - - // The subsequent args should all be records. - if (!argType.IsRecord) - { - // The last arg may be the optional "ALL" parameter. - if (argCount >= 3 && i == argCount - 1 && DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) - { - var strNode = (StrLitNode)args[i]; - - if (strNode.Value.ToUpperInvariant() != "ALL") - { - fValid = false; - errors.EnsureError(args[i], ErrRemoveAllArg, args[i]); - } - - continue; - } - - fValid = false; - errors.EnsureError(args[i], ErrNeedRecord, args[i]); - continue; - } - - var collectionAcceptsRecord = collectionType.Accepts(argType.ToTable(), exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules); - var recordAcceptsCollection = argType.ToTable().Accepts(collectionType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules); - - var featuresWithPFxV1RulesDisabled = new Features(context.Features) { PowerFxV1CompatibilityRules = false }; - bool checkAggregateNames = argType.CheckAggregateNames(collectionType, args[i], errors, featuresWithPFxV1RulesDisabled, SupportsParamCoercion); - - // The item schema should be compatible with the collection schema. - if (!checkAggregateNames) - { - fValid = false; - if (!SetErrorForMismatchedColumns(collectionType, argType, args[i], errors, context.Features)) - { - errors.EnsureError(DocumentErrorSeverity.Severe, args[i], ErrTableDoesNotAcceptThisType); - } - } - } - - returnType = context.Features.PowerFxV1CompatibilityRules ? DType.Void : collectionType; - - return fValid; - } - - public override void CheckSemantics(TexlBinding binding, TexlNode[] args, DType[] argTypes, IErrorContainer errors) - { - base.CheckSemantics(binding, args, argTypes, errors); - base.ValidateArgumentIsMutable(binding, args[0], errors); - } - - public async Task InvokeAsync(FormulaType irContextRet, FormulaValue[] args, CancellationToken cancellationToken) - { - var validArgs = CheckArgs(args, out FormulaValue faultyArg); - - if (!validArgs) - { - return faultyArg; - } - - var arg0lazy = (LambdaFormulaValue)args[0]; - var arg0 = await arg0lazy.EvalAsync().ConfigureAwait(false); - - if (arg0 is BlankValue) - { - return arg0; - } - - var argCount = args.Count(); - var lastArg = args.Last() as FormulaValue; - var all = false; - var toExclude = 1; - - if (argCount >= 3 && DType.String.Accepts(lastArg.Type._type, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: true)) - { - var lastArgValue = (string)lastArg.ToObject(); - - if (lastArgValue.ToUpperInvariant() == "ALL") - { - all = true; - toExclude = 2; - } - } - - var datasource = (TableValue)arg0; - var recordsToRemove = args.Skip(1).Take(args.Length - toExclude); - - cancellationToken.ThrowIfCancellationRequested(); - var ret = await datasource.RemoveAsync(recordsToRemove, all, cancellationToken).ConfigureAwait(false); - - // If the result is an error, propagate it up. else return blank. - FormulaValue result; - if (ret.IsError) - { - result = FormulaValue.NewError(ret.Error.Errors, irContextRet == FormulaType.Void ? FormulaType.Void : FormulaType.Blank); - } - else - { - result = irContextRet == FormulaType.Void ? FormulaValue.NewVoid() : FormulaValue.NewBlank(); - } - - return result; - } - } -} diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt index 72415f84fe..e69e895ef3 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt @@ -1,11 +1,11 @@ -#SETUP: PowerFxV1CompatibilityRules,EnableExpressionChaining,MutationFunctionsTestSetup +#SETUP: PowerFxV1CompatibilityRules,EnableExpressionChaining,StronglyTypedBuiltinEnums,MutationFunctionsTestSetup // Check MutationFunctionsTestSetup handler (PowerFxEvaluationTests.cs) for documentation. >> Collect(t1, r2);Remove(t1, r1);t1 Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) ->> Collect(t1, r1);Collect(t1, r1);Collect(t1, r1);Collect(t1, r2);Remove(t1, r1, "All");t1 +>> Collect(t1, r1);Collect(t1, r1);Collect(t1, r1);Collect(t1, r2);Remove(t1, r1, RemoveFlags.All);t1 Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) >> Collect(t1, r2); @@ -13,12 +13,13 @@ Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); Remove(t1, {Field4:false}); t1 -Error({Kind:ErrorKind.NotFound}) +Errors: Error 200-206: The function 'Remove' has some invalid arguments.|Error 211-225: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'. >> Collect(t1, r2); Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); Remove(t1, {Field4:false}, "All"); + Remove(t1, {Field4:false}, RemoveFlags.All); t1 Error({Kind:ErrorKind.NotFound}) @@ -26,6 +27,7 @@ Error({Kind:ErrorKind.NotFound}) Collect(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); Collect(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); Remove(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:true}, "All"); + Remove(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:true}, RemoveFlags.All); t1 Error({Kind:ErrorKind.NotFound}) @@ -33,6 +35,7 @@ Error({Kind:ErrorKind.NotFound}) Collect(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); Collect(t1, {Field1:1/0,Field2:"moon",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}); Remove(t1, {Field2:"earth"}, {Field2:"moon"}, "All"); + Remove(t1, {Field2:"earth"}, {Field2:"moon"}, RemoveFlags.All); t1 Error(Table({Kind:ErrorKind.NotFound},{Kind:ErrorKind.NotFound})) @@ -46,7 +49,7 @@ Table({Field1:1,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true},{F >> Collect(t1, r2); Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Remove(t1, {DisplayNameField1:3,DisplayNameField2:"earth",DisplayNameField3:DateTime(2022,2,1,0,0,0,0),DisplayNameField4:false}, "All"); + Remove(t1, {DisplayNameField1:3,DisplayNameField2:"earth",DisplayNameField3:DateTime(2022,2,1,0,0,0,0),DisplayNameField4:false}, RemoveFlags.All); t1 Table({Field1:1,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true},{Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) @@ -54,13 +57,13 @@ Table({Field1:1,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true},{F Table({Field1:2,Field2:{Field2_1:221,Field2_2:"2_2",Field2_3:{Field2_3_1:2231,Field2_3_2:"common"}},Field3:false},{Field1:3,Field2:{Field2_1:321,Field2_2:"2_2",Field2_3:{Field2_3_1:3231,Field2_3_2:"common"}},Field3:true}) >> Remove(t2, {Field1:1,Field2:{Field2_1:121,Field2_2:"2_2",Field2_3:{Field2_3_1:1231}},Field3:false}); t2 -Errors: Error 11-98: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-98: Missing column. Your formula is missing a column 'DisplayNameField2_3.DisplayNameField2_3_2' with a type of 'Text'.|Error 0-6: The function 'Remove' has some invalid arguments. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-98: Missing column. Your formula is missing a column 'DisplayNameField2.DisplayNameField2_3.Field2_3_2' with a type of 'Text'. >> Remove(t2, {Field1:2,Field2:{Field2_1:221,Field2_3:{Field2_3_1:2231,Field2_3_2:"common"}},Field3:false}); t2 -Errors: Error 11-103: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-103: Missing column. Your formula is missing a column 'DisplayNameField2_2' with a type of 'Text'.|Error 0-6: The function 'Remove' has some invalid arguments. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-103: Missing column. Your formula is missing a column 'DisplayNameField2.Field2_2' with a type of 'Text'. >> Remove(t2, {Field2:{Field2_1:321, Field2_2:"Moon"}}); t2 -Errors: Error 11-51: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-51: Missing column. Your formula is missing a column 'DisplayNameField2_3' with a type of 'Record'.|Error 0-6: The function 'Remove' has some invalid arguments. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-51: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'. >> Remove(t2, {Field2:{Field2_3:{Field2_3_1:3231}}}) ; t2 Errors: Error 11-48: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-48: Missing column. Your formula is missing a column 'DisplayNameField2_1' with a type of 'Decimal'.|Error 0-6: The function 'Remove' has some invalid arguments. @@ -69,28 +72,28 @@ Errors: Error 11-48: Invalid argument type. Expecting a Record value, but of a d Error({Kind:ErrorKind.NotFound}) >> Remove(t2, {Field1:5}) -Error({Kind:ErrorKind.NotFound}) +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-21: Missing column. Your formula is missing a column 'Field2' with a type of 'Record'. ->> Remove(t2, {Field2:{Field2_1:555,Field2_2:"2_2",Field2_3:{Field2_3_1:3231,Field2_3_2:"common"}}}); -Error({Kind:ErrorKind.NotFound}) +>> Remove(t2, {Field1:99,Field2:{Field2_1:555,Field2_2:"2_2",Field2_3:{Field2_3_1:3231,Field2_3_2:"common"}}}); +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-106: Missing column. Your formula is missing a column 'Field3' with a type of 'Boolean'. >> Remove(t2, {Field2:{Field2_1:321,Field2_2:"2_2",Field2_3:{Field2_3_1:555,Field2_3_2:"common"}}}); -Error({Kind:ErrorKind.NotFound}) +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-95: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'. >> Remove(t2, {Field2:{Field2_3:{Field2_3_2:"common"}}}, "All") -Errors: Error 11-52: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-52: Missing column. Your formula is missing a column 'DisplayNameField2_1' with a type of 'Decimal'.|Error 0-6: The function 'Remove' has some invalid arguments. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-52: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'.|Error 54-59: If provided, last argument must be 'RemoveFlags.All'. Is there a typo? ->> Remove(t2, {Field1:5}, "All") +>> Remove(t2, {Field1:5}, RemoveFlags.All) Error({Kind:ErrorKind.NotFound}) >> Remove(t2, {Field2:{Field2_1:321}});t2 -Errors: Error 11-34: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-34: Missing column. Your formula is missing a column 'DisplayNameField2_2' with a type of 'Text'.|Error 0-6: The function 'Remove' has some invalid arguments. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-34: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'. >> Remove(t2, {Field2:{Field2_3:{Field2_3_1:3231}}});t2 -Errors: Error 11-48: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-48: Missing column. Your formula is missing a column 'DisplayNameField2_1' with a type of 'Decimal'.|Error 0-6: The function 'Remove' has some invalid arguments. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-48: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'. ->> Remove(t2, {Field2:{Field2_1:321,Field2_2:"2_2",Field2_3:{Field2_3_1:5555,Field2_3_2:"common"}}}, "All");t2 +>> Remove(t2, {Field2:{Field2_1:321,Field2_2:"2_2",Field2_3:{Field2_3_1:5555,Field2_3_2:"common"}}}, RemoveFlags.All);t2 Error({Kind:ErrorKind.NotFound}) // Wrong arguments @@ -101,10 +104,10 @@ Errors: Error 14-18: If provided, last argument must be 'RemoveFlags.All'. Is th Errors: Error 14-16: If provided, last argument must be 'RemoveFlags.All'. Is there a typo?|Error 0-6: The function 'Remove' has some invalid arguments. >> Remove(t1, r1, r1, r1, r1, r1, r1, "Al"); -Errors: Error 35-39: If provided, last argument must be 'RemoveFlags.All'. Is there a typo?|Error 0-6: The function 'Remove' has some invalid arguments. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 35-39: If provided, last argument must be 'RemoveFlags.All'. Is there a typo? >> Remove(t1, "All"); -Errors: Error 11-16: Invalid argument type (Text). Expecting a Record value instead.|Error 11-16: Cannot use a non-record value in this context.|Error 0-6: The function 'Remove' has some invalid arguments. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-16: Cannot use a non-record value in this context: '"All"'. >> Collect(t1, r2); Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}); @@ -127,8 +130,8 @@ Table({Field1:1,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true},{F t1 Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false},{Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true},{Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}) ->> Remove(Foo, {Field1:5}, "All") -Errors: Error 7-10: Name isn't valid. 'Foo' isn't recognized.|Error 12-22: The specified column 'Field1' does not exist.|Error 0-6: The function 'Remove' has some invalid arguments. +>> Remove(Foo, {Field1:5}, RemoveFlags.All) +Errors: Errors: Error 7-10: Name isn't valid. 'Foo' isn't recognized. >> Remove(Foo, Bar) Errors: Error 7-10: Name isn't valid. 'Foo' isn't recognized.|Error 12-15: Name isn't valid. 'Bar' isn't recognized.|Error 0-6: The function 'Remove' has some invalid arguments. @@ -136,6 +139,9 @@ Errors: Error 7-10: Name isn't valid. 'Foo' isn't recognized.|Error 12-15: Name >> Remove(t1, {Field1:2,Field2:"not in the table",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) Error({Kind:ErrorKind.NotFound}) -// Remove propagates error. >> Remove(t1, If(1/0<2, {Field1:2})) +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-32: Missing column. Your formula is missing a column 'Field2' with a type of 'Text'. + +// Remove propagates error. +>> Remove(t1, If(1/0<2, {Field1:1,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true})) Error({Kind:ErrorKind.Div0}) \ No newline at end of file diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/AndOr_V1Compat.txt b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/AndOr_V1Compat.txt index b7ac223105..fafc0746e7 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/AndOr_V1Compat.txt +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/AndOr_V1Compat.txt @@ -1,4 +1,4 @@ -#SETUP: PowerFxV1CompatibilityRules +#SETUP: PowerFxV1CompatibilityRules,StronglyTypedBuiltinEnums // Case to test how shortcut verification work along with behavior functions @@ -98,7 +98,7 @@ true Table({Value:false},{Value:true}) // replaced with if from _V1CompatDisabled since short circuiting not supported with Void ->> If( !true, Remove(t1, First(t1), "All")); t1 // !true || operator +>> If( !true, Remove(t1, First(t1), RemoveFlags.All)); t1 // !true || operator Table({Value:false},{Value:true}) >> -3;t1 @@ -126,7 +126,7 @@ true Table({Value:true},{Value:true},{Value:true}) // replaced with if from _V1CompatDisabled since short circuiting not supported with Void ->> If( !false, Remove(t1, First(t1), "All")); t1 // || Operator +>> If( !false, Remove(t1, First(t1), RemoveFlags.All)); t1 // || Operator Table() >> 3;t1 @@ -152,5 +152,5 @@ true Table({Value:true},{Value:true},{Value:true}) // replaced with if from _V1CompatDisabled since short circuiting not supported with Void ->> If( !false, Remove(t1, First(t1), "All")); t1 // Or Function +>> If( !false, Remove(t1, First(t1), RemoveFlags.All)); t1 // Or Function Table() diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt new file mode 100644 index 0000000000..b9024ff4d1 --- /dev/null +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt @@ -0,0 +1,18 @@ +#SETUP: EnableExpressionChaining,MutationFunctionsTestSetup,StronglyTypedBuiltinEnums + +// Check MutationFunctionsTestSetup handler (PowerFxEvaluationTests.cs) for documentation. + +>> Remove(t2, {Field1:1,Field2:{Field2_1:121,Field2_2:"2_2",Field2_3:{Field2_3_1:1231,Field2_3_2:"common"}},Field3:false}, RemoveFlags.All) +If(true, {test:1}, "Void value (result of the expression can't be used).") + +>> Collect(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); + Remove(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) +If(true, {test:1}, "Void value (result of the expression can't be used).") + +>> Collect(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); + If(true, Remove(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false})) +If(true, {test:1}, "Void value (result of the expression can't be used).") + +// Remove propagates error. +>> Remove(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) +Error({Kind:ErrorKind.NotFound}) \ No newline at end of file diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove_V1Compat.txt b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove_V1Compat.txt deleted file mode 100644 index 761f0ce599..0000000000 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove_V1Compat.txt +++ /dev/null @@ -1,22 +0,0 @@ -#SETUP: PowerFxV1CompatibilityRules - ->> Set(list, Table({ Name: "One", ID: 1}, { Name: "Two", ID: 2})) -Table({ID:1,Name:"One"},{ID:2,Name:"Two"}) - ->> Set(list2, Table(First(list))) -Table({ID:1,Name:"One"}) - ->> ForAll(list2, Remove(list, ThisRecord)) -If(true, {test:1}, "Void value (result of the expression can't be used).") - ->> list -Table({ID:2,Name:"Two"}) - ->> Set(list3, [1,2,3,4]) -Table({Value:1},{Value:2},{Value:3},{Value:4}) - ->> Remove(list3, {Value:3}) -If(true, {test:1}, "Void value (result of the expression can't be used).") - ->> Remove(list3, {Value:5}) -Error({Kind:ErrorKind.NotFound}) \ No newline at end of file diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove_V1CompatDisabled.txt b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove_V1CompatDisabled.txt deleted file mode 100644 index 5d8da13dc6..0000000000 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove_V1CompatDisabled.txt +++ /dev/null @@ -1,22 +0,0 @@ -#SETUP: disable:PowerFxV1CompatibilityRules - ->> Set(list, Table({ Name: "One", ID: 1}, { Name: "Two", ID: 2})) -Table({ID:1,Name:"One"},{ID:2,Name:"Two"}) - ->> Set(list2, Table(First(list))) -Table({ID:1,Name:"One"}) - ->> ForAll(list2, Remove(list, ThisRecord)) -Table(Blank()) - ->> list -Table({ID:2,Name:"Two"}) - ->> Set(list3, [1,2,3,4]) -Table({Value:1},{Value:2},{Value:3},{Value:4}) - ->> Remove(list3, {Value:3}) -Blank() - ->> Remove(list3, {Value:5}) -Error({Kind:ErrorKind.NotFound}) \ No newline at end of file diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs index 705ca728b7..fd37de852e 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs @@ -164,7 +164,7 @@ public void DataTableEvalTest2() var result7 = engine.Eval("Patch(robintable, First(robintable),{Names:\"new-name\"});robintable", options: opt); Assert.Equal("Table({Names:\"new-name\",Scores:10},{Names:\"name3\",Scores:30},{Names:\"name100\",Scores:10})", ((DataTableValue)result7).Dump()); - var result8 = engine.Eval("Remove(robintable, {Scores:10}, \"All\");robintable", options: opt); + var result8 = engine.Eval("Remove(robintable, {Scores:10}, RemoveFlags.All);robintable", options: opt); Assert.IsType(result8); Assert.Equal(3, table.Rows.Count); From cc5801974f608f2c34245ced5ab4c69ffc918140 Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Wed, 11 Dec 2024 11:00:29 -0600 Subject: [PATCH 02/15] Editing test cases. WIP. --- .../Microsoft.PowerFx.Core/Texl/Remove.cs | 410 ++++++++++++------ src/strings/PowerFxResources.en-US.resx | 90 +++- .../ExpressionTestCases/Remove.txt | 143 +----- .../ExpressionTestCases/Remove_V1Compact.txt | 5 +- .../MutationScripts/Remove.txt | 61 ++- 5 files changed, 424 insertions(+), 285 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs index 5cf13c02d8..99164a6d2f 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs @@ -7,17 +7,20 @@ 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.DLP; using Microsoft.PowerFx.Core.Localization; using Microsoft.PowerFx.Core.Logging.Trackers; using Microsoft.PowerFx.Core.Types; +using Microsoft.PowerFx.Core.Types.Enums; using Microsoft.PowerFx.Core.Utils; using Microsoft.PowerFx.Syntax; namespace Microsoft.PowerFx.Core.Texl.Builtins { - internal abstract class RemoveFunctionBase : BuiltinFunction + // Remove(collection:*[], item1:![], item2:![], ..., ["All"]) + internal class RemoveFunction : BuiltinFunction, ISuggestionAwareFunction { public override bool ManipulatesCollections => true; @@ -27,6 +30,8 @@ internal abstract class RemoveFunctionBase : BuiltinFunction public override bool IsSelfContained => false; + public bool CanSuggestThisItem => true; + public override bool RequiresDataSourceScope => true; public override bool SupportsParamCoercion => false; @@ -40,9 +45,54 @@ internal abstract class RemoveFunctionBase : BuiltinFunction public override bool RequireAllParamColumns => true; - public RemoveFunctionBase(int arityMax, params DType[] paramTypes) - : base("Remove", TexlStrings.AboutRemove, FunctionCategories.Behavior, DType.EmptyTable, 0, 2, arityMax, paramTypes) + public override bool ArgMatchesDatasourceType(int argNum) + { + return argNum >= 1; + } + + public override bool TryGetTypeForArgSuggestionAt(int argIndex, out DType type) + { + if (argIndex > 0) + { + type = default; + return false; + } + + return base.TryGetTypeForArgSuggestionAt(argIndex, out type); + } + + public RemoveFunction() + : base("Remove", TexlStrings.AboutRemove, FunctionCategories.Behavior, DType.EmptyTable, 0, 2, int.MaxValue, DType.EmptyTable) + { + } + + public override IEnumerable GetSignatures() { + yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveArg2 }; + yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveArg2, TexlStrings.RemoveArg2 }; + yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveArg2, TexlStrings.RemoveArg2, TexlStrings.RemoveArg2 }; + } + + public override IEnumerable GetSignatures(int arity) + { + if (arity > 2) + { + return GetGenericSignatures(arity, TexlStrings.RemoveArg1, TexlStrings.RemoveArg2); + } + + return base.GetSignatures(arity); + } + + public override IEnumerable GetRequiredEnumNames() + { + return new List() { LanguageConstants.RemoveFlagsEnumString }; + } + + public override bool IsLazyEvalParam(TexlNode node, int index, Features features) + { + // First argument to mutation functions is Lazy for datasources that are copy-on-write. + // If there are any side effects in the arguments, we want those to have taken place before we make the copy. + return index == 0; } public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DType[] argTypes, IErrorContainer errors, out DType returnType, out Dictionary nodeToCoercedTypeMap) @@ -64,54 +114,87 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp errors.EnsureError(args[0], TexlStrings.ErrNeedCollection_Func, Name); } - // !!!TODO New stuff. In case this doesnt work, delete from here to the end of the function - //var argCount = argTypes.Count(); - - //for (int i = 1; i < argCount; i++) - //{ - // var argType = argTypes[i]; - // var arg = args[i]; - - // if (!argType.IsAggregate) - // { - // if (argCount >= 3 && i == argCount - 1) - // { - // fValid = (context.Features.StronglyTypedBuiltinEnums && BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) || - // (!context.Features.StronglyTypedBuiltinEnums && DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)); - - // if (!fValid) - // { - // errors.EnsureError(DocumentErrorSeverity.Severe, arg, TexlStrings.ErrRemoveAllArg); - // } - // } - // else - // { - // fValid = false; - // errors.EnsureError(args[i], TexlStrings.ErrNeedRecord_Arg, args[i]); - // } - - // continue; - // } - - // var collectionAcceptsRecord = collectionType.Accepts(argType.ToTable(), exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules); - // var recordAcceptsCollection = argType.ToTable().Accepts(collectionType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules); - - // // PFxV1 is more restrictive than PA in terms of column matching. If the collection does not accept the record or vice versa, it is an error. - // if ((context.Features.PowerFxV1CompatibilityRules && (!collectionAcceptsRecord || !recordAcceptsCollection)) || - // (!context.Features.PowerFxV1CompatibilityRules && (!collectionAcceptsRecord && !recordAcceptsCollection))) - // { - // fValid = false; - // SetErrorForMismatchedColumns(collectionType, argType, arg, errors, context.Features); - // } - - // // Only warn about no-op record inputs if there are no data sources that would use reference identity for comparison. - // else if (!collectionType.AssociatedDataSources.Any() && !recordAcceptsCollection) - // { - // errors.EnsureError(DocumentErrorSeverity.Warning, args[i], TexlStrings.ErrCollectionDoesNotAcceptThisType); - // } - //} - - returnType = context.Features.PowerFxV1CompatibilityRules ? DType.Void : collectionType; + int argCount = argTypes.Length; + for (int i = 1; i < argCount; i++) + { + DType argType = argTypes[i]; + + if (!argType.IsRecord) + { + if (argCount >= 3 && i == argCount - 1) + { + if (context.AnalysisMode) + { + if (!DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules) && + !BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[i], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) + { + fValid = false; + errors.EnsureError(DocumentErrorSeverity.Severe, args[i], TexlStrings.ErrRemoveAllArg); + } + } + else + { + if (!BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[i], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) + { + fValid = false; + errors.EnsureError(DocumentErrorSeverity.Severe, args[i], TexlStrings.ErrRemoveAllArg); + } + } + + continue; + } + else + { + fValid = false; + errors.EnsureError(args[i], TexlStrings.ErrNeedRecord_Arg, args[i]); + continue; + } + } + + var collectionAcceptsRecord = collectionType.Accepts(argType.ToTable(), exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules); + var recordAcceptsCollection = argType.ToTable().Accepts(collectionType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules); + + // PFxV1 is more restrictive than PA in terms of column matching. If the collection does not accept the record or vice versa, it is an error. + // The item schema should be compatible with the collection schema. + if ((context.Features.PowerFxV1CompatibilityRules && (!collectionAcceptsRecord || !recordAcceptsCollection)) || + (!context.Features.PowerFxV1CompatibilityRules && (!collectionAcceptsRecord && !recordAcceptsCollection))) + { + fValid = false; + SetErrorForMismatchedColumns(collectionType, argType, args[i], errors, context.Features); + } + + // Only warn about no-op record inputs if there are no data sources that would use reference identity for comparison. + else if (!collectionType.AssociatedDataSources.Any() && !recordAcceptsCollection) + { + errors.EnsureError(DocumentErrorSeverity.Warning, args[i], TexlStrings.ErrCollectionDoesNotAcceptThisType); + } + + if (!context.AnalysisMode) + { + // ArgType[N] (0 GetRequiredEnumNames() + // This method returns true if there are special suggestions for a particular parameter of the function. + public override bool HasSuggestionsForParam(int argumentIndex) { - return new List() { LanguageConstants.RemoveFlagsEnumString }; - } + Contracts.Assert(argumentIndex >= 0); - public override bool IsLazyEvalParam(TexlNode node, int index, Features features) - { - // First argument to mutation functions is Lazy for datasources that are copy-on-write. - // If there are any side effects in the arguments, we want those to have taken place before we make the copy. - return index == 0; + return argumentIndex != 1; } public override IEnumerable GetIdentifierOfModifiedValue(TexlNode[] args, out TexlNode identifierNode) @@ -156,72 +235,11 @@ public override IEnumerable GetIdentifierOfModifiedValue(TexlNode[] { firstNameNode.Ident }; - return identifiers; } - public override bool IsAsyncInvocation(CallNode callNode, TexlBinding binding) - { - Contracts.AssertValue(callNode); - Contracts.AssertValue(binding); - - return Arg0RequiresAsync(callNode, binding); - } - } - - // Remove(collection:*[], item1:![], item2:![], ..., ["All"]) - internal class RemoveFunction : RemoveFunctionBase, ISuggestionAwareFunction - { - public bool CanSuggestThisItem => true; - - public override bool ArgMatchesDatasourceType(int argNum) - { - return argNum >= 1; - } - - public override bool TryGetTypeForArgSuggestionAt(int argIndex, out DType type) - { - if (argIndex > 0) - { - type = default; - return false; - } - - return base.TryGetTypeForArgSuggestionAt(argIndex, out type); - } - - public RemoveFunction() - : base(int.MaxValue, DType.EmptyTable) - { - } - - public override IEnumerable GetSignatures() - { - yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveArg2 }; - yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveArg2, TexlStrings.RemoveArg2 }; - yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveArg2, TexlStrings.RemoveArg2, TexlStrings.RemoveArg2 }; - } - - public override IEnumerable GetSignatures(int arity) - { - if (arity > 2) - { - return GetGenericSignatures(arity, TexlStrings.RemoveArg1, TexlStrings.RemoveArg2); - } - - return base.GetSignatures(arity); - } - - // This method returns true if there are special suggestions for a particular parameter of the function. - public override bool HasSuggestionsForParam(int argumentIndex) - { - Contracts.Assert(argumentIndex >= 0); - - return argumentIndex != 1; - } - /// - /// As Remove uses the source record in it's entirety to find the entry in table, uses deepcompare at runtime, we need all fields from source. + /// As Remove uses the source record in it's entirity to find the entry in table, uses deepcompare at runtime, we need all fields from source. /// So update the selects for all columns in the source in this case except when datasource is pageable. /// In that case, we can get the info at runtime. /// @@ -321,15 +339,31 @@ protected override bool RequiresPagedDataForParamCore(TexlNode[] args, int param } // Remove(collection:*[], source:*[], ["All"]) - internal class RemoveAllFunction : RemoveFunctionBase + internal class RemoveAllFunction : BuiltinFunction { + public override bool ManipulatesCollections => true; + + public override bool ModifiesValues => true; + + public override bool IsSelfContained => false; + + public override bool RequiresDataSourceScope => true; + + public override bool SupportsParamCoercion => false; + + public override RequiredDataSourcePermissions FunctionPermission => RequiredDataSourcePermissions.Delete; + + public override bool MutatesArg(int argIndex, TexlNode arg) => argIndex == 0; + + public override bool RequireAllParamColumns => true; + public override bool ArgMatchesDatasourceType(int argNum) { return argNum == 1; } public RemoveAllFunction() - : base(3, DType.EmptyTable, DType.EmptyTable, DType.String) + : base("Remove", TexlStrings.AboutRemove, FunctionCategories.Behavior, DType.EmptyTable, 0, 2, 3, DType.EmptyTable, DType.EmptyTable, DType.String) { } @@ -339,6 +373,11 @@ public RemoveAllFunction() yield return new[] { TexlStrings.RemoveArg1, TexlStrings.RemoveAllArg2, TexlStrings.RemoveArg3 }; } + public override IEnumerable GetRequiredEnumNames() + { + return new List() { LanguageConstants.RemoveFlagsEnumString }; + } + public override bool IsLazyEvalParam(TexlNode node, int index, Features features) { // First argument to mutation functions is Lazy for datasources that are copy-on-write. @@ -346,6 +385,99 @@ public override bool IsLazyEvalParam(TexlNode node, int index, Features features return index == 0; } + public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DType[] argTypes, IErrorContainer errors, out DType returnType, out Dictionary nodeToCoercedTypeMap) + { + Contracts.AssertValue(args); + Contracts.AssertAllValues(args); + Contracts.AssertValue(argTypes); + Contracts.Assert(args.Length == argTypes.Length); + Contracts.AssertValue(errors); + Contracts.Assert(MinArity <= args.Length && args.Length <= MaxArity); + + bool fValid = base.CheckTypes(context, args, argTypes, errors, out returnType, out nodeToCoercedTypeMap); + Contracts.Assert(returnType.IsTable); + + DType collectionType = argTypes[0]; + if (!collectionType.IsTable) + { + fValid = false; + errors.EnsureError(args[0], TexlStrings.ErrNeedTable_Func, Name); + } + + // The source to be collected must be a table. + DType sourceType = argTypes[1]; + if (!sourceType.IsTable) + { + fValid = false; + errors.EnsureError(args[1], TexlStrings.ErrNeedTable_Arg, args[1]); + } + + if (!context.AnalysisMode) + { + bool checkAggregateNames = sourceType.CheckAggregateNames(collectionType, args[1], errors, context.Features, SupportsParamCoercion); + + // The item schema should be compatible with the collection schema. + if (!checkAggregateNames) + { + fValid = false; + if (!SetErrorForMismatchedColumns(collectionType, sourceType, args[1], errors, context.Features)) + { + errors.EnsureError(DocumentErrorSeverity.Severe, args[1], TexlStrings.ErrTableDoesNotAcceptThisType); + } + } + } + + // The source schema should be compatible with the collection schema. + else if (!collectionType.Accepts(sourceType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules) && !sourceType.Accepts(collectionType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) + { + fValid = false; + if (!SetErrorForMismatchedColumns(collectionType, sourceType, args[1], errors, context.Features)) + { + errors.EnsureError(DocumentErrorSeverity.Severe, args[1], TexlStrings.ErrCollectionDoesNotAcceptThisType); + } + } + + if (args.Length == 3) + { + if (context.AnalysisMode) + { + if (!DType.String.Accepts(argTypes[2], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules) && + !BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[2], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) + { + fValid = false; + errors.EnsureError(DocumentErrorSeverity.Severe, args[2], TexlStrings.ErrRemoveAllArg); + } + } + else + { + if (!BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[2], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) + { + fValid = false; + errors.EnsureError(DocumentErrorSeverity.Severe, args[2], TexlStrings.ErrRemoveAllArg); + } + } + } + + if (context.AnalysisMode) + { + // Remove returns the new collection, so the return schema is the same as the collection schema. + returnType = collectionType; + } + else + { + returnType = DType.Void; + } + + return fValid; + } + + public override void CheckSemantics(TexlBinding binding, TexlNode[] args, DType[] argTypes, IErrorContainer errors) + { + base.CheckSemantics(binding, args, argTypes, errors); + base.ValidateArgumentIsMutable(binding, args[0], errors); + MutationUtils.CheckSemantics(binding, this, args, argTypes, errors); + } + // This method returns true if there are special suggestions for a particular parameter of the function. public override bool HasSuggestionsForParam(int argumentIndex) { @@ -354,6 +486,38 @@ public override bool HasSuggestionsForParam(int argumentIndex) return argumentIndex == 0; } + public override IEnumerable GetIdentifierOfModifiedValue(TexlNode[] args, out TexlNode identifierNode) + { + Contracts.AssertValue(args); + + identifierNode = null; + if (args.Length == 0) + { + return null; + } + + var firstNameNode = args[0]?.AsFirstName(); + identifierNode = firstNameNode; + if (firstNameNode == null) + { + return null; + } + + var identifiers = new List + { + firstNameNode.Ident + }; + return identifiers; + } + + public override bool IsAsyncInvocation(CallNode callNode, TexlBinding binding) + { + Contracts.AssertValue(callNode); + Contracts.AssertValue(binding); + + return Arg0RequiresAsync(callNode, binding); + } + public override bool IsServerDelegatable(CallNode callNode, TexlBinding binding) { Contracts.AssertValue(callNode); diff --git a/src/strings/PowerFxResources.en-US.resx b/src/strings/PowerFxResources.en-US.resx index bd38c3afe3..21957d5b81 100644 --- a/src/strings/PowerFxResources.en-US.resx +++ b/src/strings/PowerFxResources.en-US.resx @@ -3291,21 +3291,41 @@ function_parameter - Second argument to the Patch function - the updates to be applied to the given rows. - Removes a specific record or records from a data source + Removes (optionally All) items from the specified 'collection'. Description of 'Remove' function. - - data_source - function_parameter - First parameter for the Remove function. The data source that contains the records that you want to remove from. Translate this string. When translating, maintain as a single word (i.e., do not add spaces). + + collection + function_parameter - First parameter of the Remove function - the name of the collection to have an item removed. - - remove_record(s) - function_parameter - One or more records to be removed. Translate this string. When translating, maintain as a single word (i.e., do not add spaces). + + item + function_parameter - Second parameter to the Remove function - the item to be removed. If provided, last argument must be 'RemoveFlags.All'. Is there a typo? {Locked=RemoveFlags.All} Error Message, RemoveFlags.All is an enum value that does not get localized. + + The collection to remove rows from. + + + A record value specifying the row to remove. + + + source + function_parameter - Second parameter to the RemoveAll function - the source of the elements to be removed. + + + A table value that specifies multiple rows to remove from the given collection. + + + all + function_parameter - Third argument for the Remove function - value indicating whether to remove all matches or only the first one. + + + An optional argument that specifies whether to remove all matches, or just the first one. + Error Display text representing the Error value of NotificationType enum (NotificationType_Error_Name). This describes showing an error notification. The possible values for this enumeration are: Error, Warning, Success, Information. @@ -4922,4 +4942,60 @@ The '{0}' value is invalid in this context. It should be a reference to either '{1}' or '{2}' scope name. {Locked='RightRecord'} + + The first argument of '{0}' should be a collection. + Error Message. + + + Incompatible type. The collection can't contain values of this type. + Error Message. + + + You might need to convert the type of the item using, for example, a Datevalue, Text, or Value function. + 1 How to fix the error. + + + Article: Create and update a collection in a canvas app + Article: Create and update a collection in a canvas app. + + + https://go.microsoft.com/fwlink/?linkid=722609 + {Locked} + + + Module: Use basic formulas + 3 crown link on basic formulas + + + https://go.microsoft.com/fwlink/?linkid=2132396 + {Locked} + + + Module: Author basic formulas with tables and records + 3 crown link on tables and records + + + https://go.microsoft.com/fwlink/?linkid=2132700 + {Locked} + + + Incorrect argument. This formula expects an optional 'All' argument or no argument. + Error Message. + + + If you’re using an Update function, for example, you might supply an optional 'All' argument at the end of the formula. This feature is available because a record might occur more than once (e.g., in a collection) to make sure that all copies of a record are updated. + 1 How to fix the error. + + + Either supply the optional 'All' argument or remove it. + 1 How to fix the error. + + + Article: Formula reference for Power Apps + Article: Formula reference for Power Apps + + + https://go.microsoft.com/fwlink/?linkid=2132478 + {Locked} + \ No newline at end of file diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt index e69e895ef3..d2adc38985 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt @@ -3,145 +3,4 @@ // Check MutationFunctionsTestSetup handler (PowerFxEvaluationTests.cs) for documentation. >> Collect(t1, r2);Remove(t1, r1);t1 -Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) - ->> Collect(t1, r1);Collect(t1, r1);Collect(t1, r1);Collect(t1, r2);Remove(t1, r1, RemoveFlags.All);t1 -Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) - ->> Collect(t1, r2); - Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Remove(t1, {Field4:false}); - t1 -Errors: Error 200-206: The function 'Remove' has some invalid arguments.|Error 211-225: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'. - ->> Collect(t1, r2); - Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Remove(t1, {Field4:false}, "All"); - Remove(t1, {Field4:false}, RemoveFlags.All); - t1 -Error({Kind:ErrorKind.NotFound}) - ->> Collect(t1, r2); - Collect(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Collect(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Remove(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:true}, "All"); - Remove(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:true}, RemoveFlags.All); - t1 -Error({Kind:ErrorKind.NotFound}) - ->> Collect(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Collect(t1, {Field1:1/0,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Collect(t1, {Field1:1/0,Field2:"moon",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}); - Remove(t1, {Field2:"earth"}, {Field2:"moon"}, "All"); - Remove(t1, {Field2:"earth"}, {Field2:"moon"}, RemoveFlags.All); - t1 -Error(Table({Kind:ErrorKind.NotFound},{Kind:ErrorKind.NotFound})) - ->> Collect(t1, r2); - Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Remove(t1, {DisplayNameField1:3,DisplayNameField2:"earth",DisplayNameField3:DateTime(2022,2,1,0,0,0,0),DisplayNameField4:false}); - t1 -Table({Field1:1,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true},{Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false},{Field1:4,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) - ->> Collect(t1, r2); - Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Remove(t1, {DisplayNameField1:3,DisplayNameField2:"earth",DisplayNameField3:DateTime(2022,2,1,0,0,0,0),DisplayNameField4:false}, RemoveFlags.All); - t1 -Table({Field1:1,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true},{Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) - ->> Remove(t2, {Field1:1,Field2:{Field2_1:121,Field2_2:"2_2",Field2_3:{Field2_3_1:1231,Field2_3_2:"common"}},Field3:false}); t2 -Table({Field1:2,Field2:{Field2_1:221,Field2_2:"2_2",Field2_3:{Field2_3_1:2231,Field2_3_2:"common"}},Field3:false},{Field1:3,Field2:{Field2_1:321,Field2_2:"2_2",Field2_3:{Field2_3_1:3231,Field2_3_2:"common"}},Field3:true}) - ->> Remove(t2, {Field1:1,Field2:{Field2_1:121,Field2_2:"2_2",Field2_3:{Field2_3_1:1231}},Field3:false}); t2 -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-98: Missing column. Your formula is missing a column 'DisplayNameField2.DisplayNameField2_3.Field2_3_2' with a type of 'Text'. - ->> Remove(t2, {Field1:2,Field2:{Field2_1:221,Field2_3:{Field2_3_1:2231,Field2_3_2:"common"}},Field3:false}); t2 -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-103: Missing column. Your formula is missing a column 'DisplayNameField2.Field2_2' with a type of 'Text'. - ->> Remove(t2, {Field2:{Field2_1:321, Field2_2:"Moon"}}); t2 -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-51: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'. - ->> Remove(t2, {Field2:{Field2_3:{Field2_3_1:3231}}}) ; t2 -Errors: Error 11-48: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-48: Missing column. Your formula is missing a column 'DisplayNameField2_1' with a type of 'Decimal'.|Error 0-6: The function 'Remove' has some invalid arguments. - ->> Remove(t2, {Field2:{Field2_1:321,Field2_2:"2_2",Field2_3:{Field2_3_1:3231,Field2_3_2:"common"}}}); t2 -Error({Kind:ErrorKind.NotFound}) - ->> Remove(t2, {Field1:5}) -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-21: Missing column. Your formula is missing a column 'Field2' with a type of 'Record'. - ->> Remove(t2, {Field1:99,Field2:{Field2_1:555,Field2_2:"2_2",Field2_3:{Field2_3_1:3231,Field2_3_2:"common"}}}); -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-106: Missing column. Your formula is missing a column 'Field3' with a type of 'Boolean'. - ->> Remove(t2, {Field2:{Field2_1:321,Field2_2:"2_2",Field2_3:{Field2_3_1:555,Field2_3_2:"common"}}}); -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-95: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'. - - ->> Remove(t2, {Field2:{Field2_3:{Field2_3_2:"common"}}}, "All") -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-52: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'.|Error 54-59: If provided, last argument must be 'RemoveFlags.All'. Is there a typo? - ->> Remove(t2, {Field1:5}, RemoveFlags.All) -Error({Kind:ErrorKind.NotFound}) - ->> Remove(t2, {Field2:{Field2_1:321}});t2 -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-34: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'. - ->> Remove(t2, {Field2:{Field2_3:{Field2_3_1:3231}}});t2 -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-48: Missing column. Your formula is missing a column 'Field1' with a type of 'Decimal'. - ->> Remove(t2, {Field2:{Field2_1:321,Field2_2:"2_2",Field2_3:{Field2_3_1:5555,Field2_3_2:"common"}}}, RemoveFlags.All);t2 -Error({Kind:ErrorKind.NotFound}) - -// Wrong arguments ->> Remove(t1, r1,"Al"); -Errors: Error 14-18: If provided, last argument must be 'RemoveFlags.All'. Is there a typo?|Error 0-6: The function 'Remove' has some invalid arguments. - ->> Remove(t1, r1,""); -Errors: Error 14-16: If provided, last argument must be 'RemoveFlags.All'. Is there a typo?|Error 0-6: The function 'Remove' has some invalid arguments. - ->> Remove(t1, r1, r1, r1, r1, r1, r1, "Al"); -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 35-39: If provided, last argument must be 'RemoveFlags.All'. Is there a typo? - ->> Remove(t1, "All"); -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-16: Cannot use a non-record value in this context: '"All"'. - ->> Collect(t1, r2); - Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}); - Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}); - Remove(t1,LookUp(t1,DisplayNameField2="earth")); - t1 -Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false},{Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true},{Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}) - ->> Collect(t1, r2); - Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}); - Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}); - Remove(t1,Last(Filter(t1, DisplayNameField2 = "earth"))); - t1 -Table({Field1:1,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true},{Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false},{Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}) - ->> Collect(t1, r2); - Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}); - Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}); - Remove(t1,First(Filter(t1, DisplayNameField2 = "earth"))); - t1 -Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false},{Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true},{Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}) - ->> Remove(Foo, {Field1:5}, RemoveFlags.All) -Errors: Errors: Error 7-10: Name isn't valid. 'Foo' isn't recognized. - ->> Remove(Foo, Bar) -Errors: Error 7-10: Name isn't valid. 'Foo' isn't recognized.|Error 12-15: Name isn't valid. 'Bar' isn't recognized.|Error 0-6: The function 'Remove' has some invalid arguments. - ->> Remove(t1, {Field1:2,Field2:"not in the table",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) -Error({Kind:ErrorKind.NotFound}) - ->> Remove(t1, If(1/0<2, {Field1:2})) -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-32: Missing column. Your formula is missing a column 'Field2' with a type of 'Text'. - -// Remove propagates error. ->> Remove(t1, If(1/0<2, {Field1:1,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true})) -Error({Kind:ErrorKind.Div0}) \ No newline at end of file +Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) \ No newline at end of file diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove_V1Compact.txt b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove_V1Compact.txt index b9a95d1f57..c9c871faa2 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove_V1Compact.txt +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove_V1Compact.txt @@ -1,9 +1,8 @@ -#SETUP: PowerFxV1CompatibilityRules -#SETUP: EnableExpressionChaining,MutationFunctionsTestSetup +#SETUP: PowerFxV1CompatibilityRules,EnableExpressionChaining,MutationFunctionsTestSetup,StronglyTypedBuiltinEnums // Check MutationFunctionsTestSetup handler (PowerFxEvaluationTests.cs) for documentation. ->> Remove(t2, {Field1:1,Field2:{Field2_1:121,Field2_2:"2_2",Field2_3:{Field2_3_1:1231,Field2_3_2:"common"}},Field3:false}, "All") +>> Remove(t2, {Field1:1,Field2:{Field2_1:121,Field2_2:"2_2",Field2_3:{Field2_3_1:1231,Field2_3_2:"common"}},Field3:false}, RemoveFlags.All) If(true, {test:1}, "Void value (result of the expression can't be used).") >> Collect(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt index b9024ff4d1..a81bcfeed9 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt @@ -1,18 +1,59 @@ -#SETUP: EnableExpressionChaining,MutationFunctionsTestSetup,StronglyTypedBuiltinEnums +#SETUP: PowerFxV1CompatibilityRules,StronglyTypedBuiltinEnums -// Check MutationFunctionsTestSetup handler (PowerFxEvaluationTests.cs) for documentation. +>> Set(t1, Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)})) +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) ->> Remove(t2, {Field1:1,Field2:{Field2_1:121,Field2_2:"2_2",Field2_3:{Field2_3_1:1231,Field2_3_2:"common"}},Field3:false}, RemoveFlags.All) +>> Set(t2, t1) +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> Remove(t1, First(t1)) If(true, {test:1}, "Void value (result of the expression can't be used).") ->> Collect(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - Remove(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) +>> 0;t1 +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> 0;Set(t1, t2) +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> Remove(t1, First(t1), RemoveFlags.All) If(true, {test:1}, "Void value (result of the expression can't be used).") ->> Collect(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}); - If(true, Remove(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false})) +>> 1;t1 +Table({a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> 1;Set(t1, t2) +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> Remove(t1, {a:true}) +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-19: Missing column. Your formula is missing a column 'b' with a type of 'Text'. + +>> 2;t1 +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> Remove(t1, {a:true,b:"does not exist",c:Now()}) +Error({Kind:ErrorKind.NotFound}) + +>> Remove(t1, {a:true,b:"does not exist",c:Now()}, RemoveFlags.All) +Error({Kind:ErrorKind.NotFound}) + +>> Remove(t1, {a:true,b:"does not exist",c:Now()}, {a:false,b:"does not exist",c:Now()}, RemoveFlags.All) +Error(Table({Kind:ErrorKind.NotFound},{Kind:ErrorKind.NotFound})) + +>> 3;t1 +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> Set(t3, Table({a:{aa:{aaa:true,bbb:true}}})) +Table({a:{aa:{aaa:true,bbb:true}}}) + +>> Remove(t3, {a:{aa:{aaa:true}}}) +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-30: Missing column. Your formula is missing a column 'a.aa.bbb' with a type of 'Boolean'.|Error 11-30: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-30: Missing column. Your formula is missing a column 'aa.bbb' with a type of 'Boolean'. + +>> Remove(t3, {a:{aa:{aaa:true,bbb:false}}}) +Error({Kind:ErrorKind.NotFound}) + +>> Remove(t3, {a:{aa:{aaa:true,bbb:true}}}) If(true, {test:1}, "Void value (result of the expression can't be used).") -// Remove propagates error. ->> Remove(t1, {Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) -Error({Kind:ErrorKind.NotFound}) \ No newline at end of file +>> t3 +Table() + From e1ca34b6865568848253adeab73c308c05c3d532 Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Fri, 13 Dec 2024 13:41:15 -0600 Subject: [PATCH 03/15] WIP Remove function --- .../Microsoft.PowerFx.Core/Texl/Remove.cs | 1 + .../ExpressionTestCases/Remove.txt | 42 ++++++++++++++++++- .../MutationScripts/Remove.txt | 16 ++++++- .../PADIntegrationTests.cs | 6 ++- 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs index 99164a6d2f..13fec1473f 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs @@ -43,6 +43,7 @@ internal class RemoveFunction : BuiltinFunction, ISuggestionAwareFunction public override bool MutatesArg(int argIndex, TexlNode arg) => argIndex == 0; + // !!!TODO this might be a problem. public override bool RequireAllParamColumns => true; public override bool ArgMatchesDatasourceType(int argNum) diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt index d2adc38985..ecb4352eb1 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt @@ -2,5 +2,43 @@ // Check MutationFunctionsTestSetup handler (PowerFxEvaluationTests.cs) for documentation. ->> Collect(t1, r2);Remove(t1, r1);t1 -Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false}) \ No newline at end of file +// Wrong arguments +>> Remove(t1, r1,"Al"); +Errors: Error 14-18: If provided, last argument must be 'RemoveFlags.All'. Is there a typo?|Error 0-6: The function 'Remove' has some invalid arguments. + +>> Remove(t1, r1,""); +Errors: Error 14-16: If provided, last argument must be 'RemoveFlags.All'. Is there a typo?|Error 0-6: The function 'Remove' has some invalid arguments. + +>> Remove(t1, r1, r1, r1, r1, r1, r1, "Al"); +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 35-39: If provided, last argument must be 'RemoveFlags.All'. Is there a typo? + +>> Remove(t1, "All"); +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-16: Cannot use a non-record value in this context: '"All"'. + +>> Collect(t1, r2); + Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}); + Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}); + Remove(t1,LookUp(t1,DisplayNameField2="earth")); + t1 +Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false},{Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true},{Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}) + +>> Collect(t1, r2); + Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}); + Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}); + Remove(t1,Last(Filter(t1, DisplayNameField2 = "earth"))); + t1 +Table({Field1:1,Field2:"earth",Field3:DateTime(2022,1,1,0,0,0,0),Field4:true},{Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false},{Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}) + +>> Collect(t1, r2); + Collect(t1, {Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true}); + Collect(t1, {Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}); + Remove(t1,First(Filter(t1, DisplayNameField2 = "earth"))); + t1 +Table({Field1:2,Field2:"moon",Field3:DateTime(2022,2,1,0,0,0,0),Field4:false},{Field1:3,Field2:"earth",Field3:DateTime(2030,2,1,0,0,0,0),Field4:true},{Field1:4,Field2:"earth",Field3:DateTime(2040,2,1,0,0,0,0),Field4:false}) + +>> Remove(Foo, {Field1:5}, RemoveFlags.All) +Errors: Error 7-10: Name isn't valid. 'Foo' isn't recognized. + +>> Remove(Foo, Bar) +Errors: Error 7-10: Name isn't valid. 'Foo' isn't recognized.|Error 12-15: Name isn't valid. 'Bar' isn't recognized.|Error 0-6: The function 'Remove' has some invalid arguments. + diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt index a81bcfeed9..ead47ca656 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt @@ -42,6 +42,16 @@ Error(Table({Kind:ErrorKind.NotFound},{Kind:ErrorKind.NotFound})) >> 3;t1 Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) +>> Remove(t1, If(1/0<2, {a:true,b:"hello"})) +Errors: Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-40: Missing column. Your formula is missing a column 'c' with a type of 'DateTime'. + +>> 4;t1 +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +// Remove propagates error. +>> Remove(t1, If(1/0<2, {a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)})) +Error({Kind:ErrorKind.Div0}) + >> Set(t3, Table({a:{aa:{aaa:true,bbb:true}}})) Table({a:{aa:{aaa:true,bbb:true}}}) @@ -51,9 +61,11 @@ Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-30 >> Remove(t3, {a:{aa:{aaa:true,bbb:false}}}) Error({Kind:ErrorKind.NotFound}) +>> Remove(t3, {a:{aa:{aaa:true,bbb:false}}}, RemoveFlags.All) +Error({Kind:ErrorKind.NotFound}) + >> Remove(t3, {a:{aa:{aaa:true,bbb:true}}}) If(true, {test:1}, "Void value (result of the expression can't be used).") >> t3 -Table() - +Table() \ No newline at end of file diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs index fd37de852e..af0f8cef78 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Data; +using System.Linq; using Microsoft.PowerFx.Interpreter.Tests; using Microsoft.PowerFx.Types; using Xunit; @@ -164,8 +165,9 @@ public void DataTableEvalTest2() var result7 = engine.Eval("Patch(robintable, First(robintable),{Names:\"new-name\"});robintable", options: opt); Assert.Equal("Table({Names:\"new-name\",Scores:10},{Names:\"name3\",Scores:30},{Names:\"name100\",Scores:10})", ((DataTableValue)result7).Dump()); - var result8 = engine.Eval("Remove(robintable, {Scores:10}, RemoveFlags.All);robintable", options: opt); - Assert.IsType(result8); + var check = engine.Check("Remove(robintable, {Scores:10}, RemoveFlags.All)", options: opt); + Assert.False(check.IsSuccess); + Assert.Contains("ErrColumnMissing_ColName_ExpectedType", check.Errors.Select(err => err.MessageKey)); Assert.Equal(3, table.Rows.Count); } From af81e474118f1ebac9815ad8c714b5568d55495d Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Mon, 16 Dec 2024 10:55:59 -0600 Subject: [PATCH 04/15] Return type based on PFxV1 flag. --- .../Microsoft.PowerFx.Core/Texl/Remove.cs | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs index 13fec1473f..eb956e035d 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs @@ -187,15 +187,7 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp } } - if (context.AnalysisMode) - { - // Remove returns the new collection, so the return schema is the same as the collection schema. - returnType = collectionType; - } - else - { - returnType = DType.Void; - } + returnType = context.Features.PowerFxV1CompatibilityRules ? DType.Void : collectionType; return fValid; } @@ -459,15 +451,7 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp } } - if (context.AnalysisMode) - { - // Remove returns the new collection, so the return schema is the same as the collection schema. - returnType = collectionType; - } - else - { - returnType = DType.Void; - } + returnType = context.Features.PowerFxV1CompatibilityRules ? DType.Void : collectionType; return fValid; } From 81854f6a3a20bc877b7b36395d51fa3f148e2cda Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Tue, 17 Dec 2024 13:38:44 -0600 Subject: [PATCH 05/15] Printing better error message when PFx.V1 is active --- .../Microsoft.PowerFx.Core/Functions/TexlFunction.cs | 2 +- src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs | 5 ----- .../PADIntegrationTests.cs | 1 - 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs index 5778346a41..9abf51f816 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/TexlFunction.cs @@ -1167,7 +1167,7 @@ private bool SetErrorForMismatchedColumnsCore(DType expectedType, DType actualTy } // Second, set column missing message if applicable - if (RequireAllParamColumns && !expectedType.AreFieldsOptional) + if ((RequireAllParamColumns || features.PowerFxV1CompatibilityRules) && !expectedType.AreFieldsOptional) { errors.EnsureError( DocumentErrorSeverity.Severe, diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs index eb956e035d..f8e57f83cf 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs @@ -43,9 +43,6 @@ internal class RemoveFunction : BuiltinFunction, ISuggestionAwareFunction public override bool MutatesArg(int argIndex, TexlNode arg) => argIndex == 0; - // !!!TODO this might be a problem. - public override bool RequireAllParamColumns => true; - public override bool ArgMatchesDatasourceType(int argNum) { return argNum >= 1; @@ -348,8 +345,6 @@ internal class RemoveAllFunction : BuiltinFunction public override bool MutatesArg(int argIndex, TexlNode arg) => argIndex == 0; - public override bool RequireAllParamColumns => true; - public override bool ArgMatchesDatasourceType(int argNum) { return argNum == 1; diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs index af0f8cef78..1dd46080a3 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PADIntegrationTests.cs @@ -167,7 +167,6 @@ public void DataTableEvalTest2() var check = engine.Check("Remove(robintable, {Scores:10}, RemoveFlags.All)", options: opt); Assert.False(check.IsSuccess); - Assert.Contains("ErrColumnMissing_ColName_ExpectedType", check.Errors.Select(err => err.MessageKey)); Assert.Equal(3, table.Rows.Count); } From ce31e61383b8c4b04183378a376a6fef070e3c6d Mon Sep 17 00:00:00 2001 From: Anderson Ferreira da Silva Date: Thu, 19 Dec 2024 06:06:47 -0600 Subject: [PATCH 06/15] Fixing delegable feature. --- .../App/IExternalEnabledFeatures.cs | 8 ++++++-- .../Microsoft.PowerFx.Core/Public/Config/Features.cs | 5 ----- src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs | 7 ++----- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/App/IExternalEnabledFeatures.cs b/src/libraries/Microsoft.PowerFx.Core/App/IExternalEnabledFeatures.cs index 52cd752720..d149e2882e 100644 --- a/src/libraries/Microsoft.PowerFx.Core/App/IExternalEnabledFeatures.cs +++ b/src/libraries/Microsoft.PowerFx.Core/App/IExternalEnabledFeatures.cs @@ -21,7 +21,9 @@ internal interface IExternalEnabledFeatures bool IsEnhancedComponentFunctionPropertyEnabled { get; } - bool IsComponentFunctionPropertyDataflowEnabled { get; } + bool IsComponentFunctionPropertyDataflowEnabled { get; } + + bool IsRemoveAllDelegationEnabled { get; } } internal sealed class DefaultEnabledFeatures : IExternalEnabledFeatures @@ -36,6 +38,8 @@ internal sealed class DefaultEnabledFeatures : IExternalEnabledFeatures public bool IsEnhancedComponentFunctionPropertyEnabled => true; - public bool IsComponentFunctionPropertyDataflowEnabled => true; + public bool IsComponentFunctionPropertyDataflowEnabled => true; + + public bool IsRemoveAllDelegationEnabled => true; } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs b/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs index 8a4a109439..abf908f462 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs @@ -82,11 +82,6 @@ public sealed class Features /// internal bool IsUserDefinedTypesEnabled { get; init; } = false; - /// - /// Enables Remove All delegation. - /// - internal bool IsRemoveAllDelegationEnabled { get; set; } - internal static readonly Features None = new Features(); /// diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs index f8e57f83cf..e4cc808eec 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs @@ -509,12 +509,9 @@ public override bool IsServerDelegatable(CallNode callNode, TexlBinding binding) } // Use ECS flag as a guard. - if (binding.Document != null) + if (binding.Document != null && !binding.Document.Properties.EnabledFeatures.IsRemoveAllDelegationEnabled) { - if (!binding.Features.IsRemoveAllDelegationEnabled) - { - return false; - } + return false; } if (!binding.TryGetDataSourceInfo(callNode.Args.Children[0], out IExternalDataSource dataSource)) From 91db782691a2f9ff39df5b3d387920cb523a5064 Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Thu, 19 Dec 2024 11:25:05 -0600 Subject: [PATCH 07/15] RemoveAll delegation fixes. --- .../Microsoft.PowerFx.Core/App/IExternalEnabledFeatures.cs | 6 +----- .../Microsoft.PowerFx.Core/Public/Config/Features.cs | 6 ++++++ src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/App/IExternalEnabledFeatures.cs b/src/libraries/Microsoft.PowerFx.Core/App/IExternalEnabledFeatures.cs index d149e2882e..8599f838b7 100644 --- a/src/libraries/Microsoft.PowerFx.Core/App/IExternalEnabledFeatures.cs +++ b/src/libraries/Microsoft.PowerFx.Core/App/IExternalEnabledFeatures.cs @@ -21,9 +21,7 @@ internal interface IExternalEnabledFeatures bool IsEnhancedComponentFunctionPropertyEnabled { get; } - bool IsComponentFunctionPropertyDataflowEnabled { get; } - - bool IsRemoveAllDelegationEnabled { get; } + bool IsComponentFunctionPropertyDataflowEnabled { get; } } internal sealed class DefaultEnabledFeatures : IExternalEnabledFeatures @@ -39,7 +37,5 @@ internal sealed class DefaultEnabledFeatures : IExternalEnabledFeatures public bool IsEnhancedComponentFunctionPropertyEnabled => true; public bool IsComponentFunctionPropertyDataflowEnabled => true; - - public bool IsRemoveAllDelegationEnabled => true; } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs b/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs index abf908f462..7d7e92e3d7 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/Config/Features.cs @@ -82,6 +82,11 @@ public sealed class Features /// internal bool IsUserDefinedTypesEnabled { get; init; } = false; + /// + /// Enables RemoveAll delegation. + /// + internal bool IsRemoveAllDelegationEnabled { get; init; } + internal static readonly Features None = new Features(); /// @@ -124,6 +129,7 @@ internal Features(Features other) AsTypeLegacyCheck = other.AsTypeLegacyCheck; JsonFunctionAcceptsLazyTypes = other.JsonFunctionAcceptsLazyTypes; IsLookUpReductionDelegationEnabled = other.IsLookUpReductionDelegationEnabled; + IsRemoveAllDelegationEnabled = other.IsRemoveAllDelegationEnabled; } } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs index e4cc808eec..1a5a2b3458 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs @@ -509,7 +509,7 @@ public override bool IsServerDelegatable(CallNode callNode, TexlBinding binding) } // Use ECS flag as a guard. - if (binding.Document != null && !binding.Document.Properties.EnabledFeatures.IsRemoveAllDelegationEnabled) + if (!binding.Features.IsRemoveAllDelegationEnabled) { return false; } From d4a9d841807f59559482a821f3fb0eaef3d047fe Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Thu, 26 Dec 2024 14:45:21 -0600 Subject: [PATCH 08/15] Covering some corner cases. Fixing a bug when removing the same record (value) multiple times. --- .../Public/Values/CollectionTableValue.cs | 16 ++++-- .../Microsoft.PowerFx.Core/Texl/Remove.cs | 57 +++++-------------- .../Functions/Mutation/MutationUtils.cs | 2 +- .../ExpressionTestCases/Remove.txt | 6 +- .../MutationScripts/Remove.txt | 46 +++++++++++++-- 5 files changed, 68 insertions(+), 59 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs b/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs index 5b967c555e..b77fbbdde8 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs @@ -184,7 +184,7 @@ public override DValue Last(bool mutationCopy = false) public override async Task> RemoveAsync(IEnumerable recordsToRemove, bool all, CancellationToken cancellationToken) { var ret = false; - var deleteList = new List(); + var markedToDeletion = new HashSet(); var errors = new List(); cancellationToken.ThrowIfCancellationRequested(); @@ -206,9 +206,15 @@ public override async Task> RemoveAsync(IEnumerable> RemoveAsync(IEnumerable= 3 && i == argCount - 1) + if (argCount >= 3 && i == argCount - 1 && + ((context.Features.PowerFxV1CompatibilityRules && BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[i], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) || + (!context.Features.PowerFxV1CompatibilityRules && DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)))) { - if (context.AnalysisMode) - { - if (!DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules) && - !BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[i], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) - { - fValid = false; - errors.EnsureError(DocumentErrorSeverity.Severe, args[i], TexlStrings.ErrRemoveAllArg); - } - } - else - { - if (!BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[i], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) - { - fValid = false; - errors.EnsureError(DocumentErrorSeverity.Severe, args[i], TexlStrings.ErrRemoveAllArg); - } - } - - continue; - } - else - { - fValid = false; - errors.EnsureError(args[i], TexlStrings.ErrNeedRecord_Arg, args[i]); continue; } + + fValid = false; + errors.EnsureError(args[i], TexlStrings.ErrNeedRecord_Arg, args[i]); + continue; } var collectionAcceptsRecord = collectionType.Accepts(argType.ToTable(), exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules); @@ -351,7 +333,7 @@ public override bool ArgMatchesDatasourceType(int argNum) } public RemoveAllFunction() - : base("Remove", TexlStrings.AboutRemove, FunctionCategories.Behavior, DType.EmptyTable, 0, 2, 3, DType.EmptyTable, DType.EmptyTable, DType.String) + : base("Remove", TexlStrings.AboutRemove, FunctionCategories.Behavior, DType.EmptyTable, 0, 2, 3, DType.EmptyTable, DType.EmptyTable) { } @@ -425,25 +407,12 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp } } - if (args.Length == 3) + if (args.Length == 3 && + ((context.Features.PowerFxV1CompatibilityRules && !BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[2], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) || + (!context.Features.PowerFxV1CompatibilityRules && !DType.String.Accepts(argTypes[2], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)))) { - if (context.AnalysisMode) - { - if (!DType.String.Accepts(argTypes[2], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules) && - !BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[2], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) - { - fValid = false; - errors.EnsureError(DocumentErrorSeverity.Severe, args[2], TexlStrings.ErrRemoveAllArg); - } - } - else - { - if (!BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[2], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) - { - fValid = false; - errors.EnsureError(DocumentErrorSeverity.Severe, args[2], TexlStrings.ErrRemoveAllArg); - } - } + fValid = false; + errors.EnsureError(DocumentErrorSeverity.Severe, args[2], TexlStrings.ErrRemoveAllArg); } returnType = context.Features.PowerFxV1CompatibilityRules ? DType.Void : collectionType; diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/MutationUtils.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/MutationUtils.cs index f57c52bc59..8e33fe5476 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/MutationUtils.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/Mutation/MutationUtils.cs @@ -78,7 +78,7 @@ public static async Task RemoveCore(FormulaType irContext, Formula if (arg is TableValue tableValue) { - var errorRecord = tableValue.Rows.First(row => row.IsError); + var errorRecord = tableValue.Rows.FirstOrDefault(row => row.IsError); if (errorRecord != null) { return errorRecord.Error; diff --git a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt index ecb4352eb1..df1ce9d7f4 100644 --- a/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt +++ b/src/tests/Microsoft.PowerFx.Core.Tests.Shared/ExpressionTestCases/Remove.txt @@ -4,13 +4,13 @@ // Wrong arguments >> Remove(t1, r1,"Al"); -Errors: Error 14-18: If provided, last argument must be 'RemoveFlags.All'. Is there a typo?|Error 0-6: The function 'Remove' has some invalid arguments. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 14-18: Cannot use a non-record value in this context: '"Al"'. >> Remove(t1, r1,""); -Errors: Error 14-16: If provided, last argument must be 'RemoveFlags.All'. Is there a typo?|Error 0-6: The function 'Remove' has some invalid arguments. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 14-16: Cannot use a non-record value in this context: '""'. >> Remove(t1, r1, r1, r1, r1, r1, r1, "Al"); -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 35-39: If provided, last argument must be 'RemoveFlags.All'. Is there a typo? +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 35-39: Cannot use a non-record value in this context: '"Al"'. >> Remove(t1, "All"); Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-16: Cannot use a non-record value in this context: '"All"'. diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt index ead47ca656..52478926c4 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt @@ -48,24 +48,58 @@ Errors: Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Err >> 4;t1 Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) +>> Set(t3, t1) +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +// Removing multiple rows with the same values. +>> Remove(t3, {a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)}, {a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)}, {a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)}) +If(true, {test:1}, "Void value (result of the expression can't be used).") + +>> 0;t3 +Table({a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> 0;Set(t3, t1) +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> Remove(t3, {a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)}, {a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)}, {a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)}, RemoveFlags.All) +Error(Table({Kind:ErrorKind.NotFound},{Kind:ErrorKind.NotFound})) + +>> 1;t3 +Table({a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> 1;Set(t3, t1) +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> Remove(t3, t3) +If(true, {test:1}, "Void value (result of the expression can't be used).") + +>> 2;t3 +Table() + +>> 2;Set(t3, t1) +Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) + +>> Remove(t3, t3, RemoveFlags.All) +Error(Table({Kind:ErrorKind.NotFound},{Kind:ErrorKind.NotFound})) + // Remove propagates error. >> Remove(t1, If(1/0<2, {a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)})) Error({Kind:ErrorKind.Div0}) ->> Set(t3, Table({a:{aa:{aaa:true,bbb:true}}})) +>> Set(t4, Table({a:{aa:{aaa:true,bbb:true}}})) Table({a:{aa:{aaa:true,bbb:true}}}) ->> Remove(t3, {a:{aa:{aaa:true}}}) +>> Remove(t4, {a:{aa:{aaa:true}}}) Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-30: Missing column. Your formula is missing a column 'a.aa.bbb' with a type of 'Boolean'.|Error 11-30: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-30: Missing column. Your formula is missing a column 'aa.bbb' with a type of 'Boolean'. ->> Remove(t3, {a:{aa:{aaa:true,bbb:false}}}) +>> Remove(t4, {a:{aa:{aaa:true,bbb:false}}}) Error({Kind:ErrorKind.NotFound}) ->> Remove(t3, {a:{aa:{aaa:true,bbb:false}}}, RemoveFlags.All) +>> Remove(t4, {a:{aa:{aaa:true,bbb:false}}}, RemoveFlags.All) Error({Kind:ErrorKind.NotFound}) ->> Remove(t3, {a:{aa:{aaa:true,bbb:true}}}) +>> Remove(t4, {a:{aa:{aaa:true,bbb:true}}}) If(true, {test:1}, "Void value (result of the expression can't be used).") ->> t3 +>> t4 Table() \ No newline at end of file From 5fcbff8fe18ad1b0d13579b5ca3f4e7396025542 Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Fri, 27 Dec 2024 18:47:40 -0600 Subject: [PATCH 09/15] PA errors fixing. --- .../Microsoft.PowerFx.Core/Texl/Remove.cs | 43 ++++--------------- .../MutationScripts/Remove.txt | 2 +- 2 files changed, 9 insertions(+), 36 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs index 4dcf71fde8..44448845de 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs @@ -120,8 +120,9 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp if (!argType.IsRecord) { if (argCount >= 3 && i == argCount - 1 && - ((context.Features.PowerFxV1CompatibilityRules && BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[i], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) || - (!context.Features.PowerFxV1CompatibilityRules && DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)))) + ((!context.AnalysisMode && BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) || + (context.AnalysisMode && (DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules) || + BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules))))) { continue; } @@ -140,7 +141,10 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp (!context.Features.PowerFxV1CompatibilityRules && (!collectionAcceptsRecord && !recordAcceptsCollection))) { fValid = false; - SetErrorForMismatchedColumns(collectionType, argType, args[i], errors, context.Features); + if (!SetErrorForMismatchedColumns(collectionType, argType, args[i], errors, context.Features)) + { + errors.EnsureError(DocumentErrorSeverity.Severe, args[i], TexlStrings.ErrTableDoesNotAcceptThisType); + } } // Only warn about no-op record inputs if there are no data sources that would use reference identity for comparison. @@ -148,22 +152,6 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp { errors.EnsureError(DocumentErrorSeverity.Warning, args[i], TexlStrings.ErrCollectionDoesNotAcceptThisType); } - - if (!context.AnalysisMode) - { - // ArgType[N] (0> Remove(t4, {a:{aa:{aaa:true}}}) -Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-30: Missing column. Your formula is missing a column 'a.aa.bbb' with a type of 'Boolean'.|Error 11-30: Invalid argument type. Expecting a Record value, but of a different schema.|Error 11-30: Missing column. Your formula is missing a column 'aa.bbb' with a type of 'Boolean'. +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-30: Missing column. Your formula is missing a column 'a.aa.bbb' with a type of 'Boolean'. >> Remove(t4, {a:{aa:{aaa:true,bbb:false}}}) Error({Kind:ErrorKind.NotFound}) From fa3ac548efc0f4344f9f7497480c662869b62ad9 Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Mon, 30 Dec 2024 09:17:45 -0600 Subject: [PATCH 10/15] Checkpoint. PA build was working before these changes. --- .../Microsoft.PowerFx.Core/Texl/Remove.cs | 191 +++++++----------- .../MutationScripts/Remove.txt | 3 + 2 files changed, 73 insertions(+), 121 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs index 44448845de..eeb666db20 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; +using System.Numerics; using Microsoft.PowerFx.Core.App.ErrorContainers; using Microsoft.PowerFx.Core.Binding; using Microsoft.PowerFx.Core.Entities; @@ -19,8 +20,7 @@ namespace Microsoft.PowerFx.Core.Texl.Builtins { - // Remove(collection:*[], item1:![], item2:![], ..., ["All"]) - internal class RemoveFunction : BuiltinFunction, ISuggestionAwareFunction + internal abstract class RemoveBaseFunction : BuiltinFunction { public override bool ManipulatesCollections => true; @@ -30,24 +30,14 @@ internal class RemoveFunction : BuiltinFunction, ISuggestionAwareFunction public override bool IsSelfContained => false; - public bool CanSuggestThisItem => true; - public override bool RequiresDataSourceScope => true; public override bool SupportsParamCoercion => false; public override RequiredDataSourcePermissions FunctionPermission => RequiredDataSourcePermissions.Delete; - // Return true if this function affects datasource query options. - public override bool AffectsDataSourceQueryOptions => true; - public override bool MutatesArg(int argIndex, TexlNode arg) => argIndex == 0; - public override bool ArgMatchesDatasourceType(int argNum) - { - return argNum >= 1; - } - public override bool TryGetTypeForArgSuggestionAt(int argIndex, out DType type) { if (argIndex > 0) @@ -59,8 +49,73 @@ public override bool TryGetTypeForArgSuggestionAt(int argIndex, out DType type) return base.TryGetTypeForArgSuggestionAt(argIndex, out type); } + public RemoveBaseFunction(int arityMax, params DType[] paramTypes) + : base("Remove", TexlStrings.AboutRemove, FunctionCategories.Behavior, DType.EmptyTable, 0, 2, arityMax, paramTypes) + { + } + + public override bool IsLazyEvalParam(TexlNode node, int index, Features features) + { + // First argument to mutation functions is Lazy for datasources that are copy-on-write. + // If there are any side effects in the arguments, we want those to have taken place before we make the copy. + return index == 0; + } + + public override void CheckSemantics(TexlBinding binding, TexlNode[] args, DType[] argTypes, IErrorContainer errors) + { + base.CheckSemantics(binding, args, argTypes, errors); + base.ValidateArgumentIsMutable(binding, args[0], errors); + MutationUtils.CheckSemantics(binding, this, args, argTypes, errors); + } + + public override IEnumerable GetIdentifierOfModifiedValue(TexlNode[] args, out TexlNode identifierNode) + { + Contracts.AssertValue(args); + + identifierNode = null; + if (args.Length == 0) + { + return null; + } + + var firstNameNode = args[0]?.AsFirstName(); + identifierNode = firstNameNode; + if (firstNameNode == null) + { + return null; + } + + var identifiers = new List + { + firstNameNode.Ident + }; + return identifiers; + } + + public override bool IsAsyncInvocation(CallNode callNode, TexlBinding binding) + { + Contracts.AssertValue(callNode); + Contracts.AssertValue(binding); + + return Arg0RequiresAsync(callNode, binding); + } + } + + // Remove(collection:*[], item1:![], item2:![], ..., ["All"]) + internal class RemoveFunction : RemoveBaseFunction, ISuggestionAwareFunction + { + public bool CanSuggestThisItem => true; + + // Return true if this function affects datasource query options. + public override bool AffectsDataSourceQueryOptions => true; + + public override bool ArgMatchesDatasourceType(int argNum) + { + return argNum >= 1; + } + public RemoveFunction() - : base("Remove", TexlStrings.AboutRemove, FunctionCategories.Behavior, DType.EmptyTable, 0, 2, int.MaxValue, DType.EmptyTable) + : base(int.MaxValue, DType.EmptyTable) { } @@ -86,13 +141,6 @@ public override IEnumerable GetRequiredEnumNames() return new List() { LanguageConstants.RemoveFlagsEnumString }; } - public override bool IsLazyEvalParam(TexlNode node, int index, Features features) - { - // First argument to mutation functions is Lazy for datasources that are copy-on-write. - // If there are any side effects in the arguments, we want those to have taken place before we make the copy. - return index == 0; - } - public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DType[] argTypes, IErrorContainer errors, out DType returnType, out Dictionary nodeToCoercedTypeMap) { Contracts.AssertValue(args); @@ -159,13 +207,6 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp return fValid; } - public override void CheckSemantics(TexlBinding binding, TexlNode[] args, DType[] argTypes, IErrorContainer errors) - { - base.CheckSemantics(binding, args, argTypes, errors); - base.ValidateArgumentIsMutable(binding, args[0], errors); - MutationUtils.CheckSemantics(binding, this, args, argTypes, errors); - } - // This method returns true if there are special suggestions for a particular parameter of the function. public override bool HasSuggestionsForParam(int argumentIndex) { @@ -174,30 +215,6 @@ public override bool HasSuggestionsForParam(int argumentIndex) return argumentIndex != 1; } - public override IEnumerable GetIdentifierOfModifiedValue(TexlNode[] args, out TexlNode identifierNode) - { - Contracts.AssertValue(args); - - identifierNode = null; - if (args.Length == 0) - { - return null; - } - - var firstNameNode = args[0]?.AsFirstName(); - identifierNode = firstNameNode; - if (firstNameNode == null) - { - return null; - } - - var identifiers = new List - { - firstNameNode.Ident - }; - return identifiers; - } - /// /// As Remove uses the source record in it's entirity to find the entry in table, uses deepcompare at runtime, we need all fields from source. /// So update the selects for all columns in the source in this case except when datasource is pageable. @@ -277,14 +294,6 @@ public override bool IsSuggestionTypeValid(int paramIndex, DType type) return type.IsRecord || type.Kind == DKind.String; } - public override bool IsAsyncInvocation(CallNode callNode, TexlBinding binding) - { - Contracts.AssertValue(callNode); - Contracts.AssertValue(binding); - - return Arg0RequiresAsync(callNode, binding); - } - protected override bool RequiresPagedDataForParamCore(TexlNode[] args, int paramIndex, TexlBinding binding) { Contracts.AssertValue(args); @@ -299,29 +308,15 @@ protected override bool RequiresPagedDataForParamCore(TexlNode[] args, int param } // Remove(collection:*[], source:*[], ["All"]) - internal class RemoveAllFunction : BuiltinFunction + internal class RemoveAllFunction : RemoveBaseFunction { - public override bool ManipulatesCollections => true; - - public override bool ModifiesValues => true; - - public override bool IsSelfContained => false; - - public override bool RequiresDataSourceScope => true; - - public override bool SupportsParamCoercion => false; - - public override RequiredDataSourcePermissions FunctionPermission => RequiredDataSourcePermissions.Delete; - - public override bool MutatesArg(int argIndex, TexlNode arg) => argIndex == 0; - public override bool ArgMatchesDatasourceType(int argNum) { return argNum == 1; } public RemoveAllFunction() - : base("Remove", TexlStrings.AboutRemove, FunctionCategories.Behavior, DType.EmptyTable, 0, 2, 3, DType.EmptyTable, DType.EmptyTable) + : base(3, DType.EmptyTable, DType.EmptyTable) { } @@ -336,13 +331,6 @@ public override IEnumerable GetRequiredEnumNames() return new List() { LanguageConstants.RemoveFlagsEnumString }; } - public override bool IsLazyEvalParam(TexlNode node, int index, Features features) - { - // First argument to mutation functions is Lazy for datasources that are copy-on-write. - // If there are any side effects in the arguments, we want those to have taken place before we make the copy. - return index == 0; - } - public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DType[] argTypes, IErrorContainer errors, out DType returnType, out Dictionary nodeToCoercedTypeMap) { Contracts.AssertValue(args); @@ -393,13 +381,6 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp return fValid; } - public override void CheckSemantics(TexlBinding binding, TexlNode[] args, DType[] argTypes, IErrorContainer errors) - { - base.CheckSemantics(binding, args, argTypes, errors); - base.ValidateArgumentIsMutable(binding, args[0], errors); - MutationUtils.CheckSemantics(binding, this, args, argTypes, errors); - } - // This method returns true if there are special suggestions for a particular parameter of the function. public override bool HasSuggestionsForParam(int argumentIndex) { @@ -408,38 +389,6 @@ public override bool HasSuggestionsForParam(int argumentIndex) return argumentIndex == 0; } - public override IEnumerable GetIdentifierOfModifiedValue(TexlNode[] args, out TexlNode identifierNode) - { - Contracts.AssertValue(args); - - identifierNode = null; - if (args.Length == 0) - { - return null; - } - - var firstNameNode = args[0]?.AsFirstName(); - identifierNode = firstNameNode; - if (firstNameNode == null) - { - return null; - } - - var identifiers = new List - { - firstNameNode.Ident - }; - return identifiers; - } - - public override bool IsAsyncInvocation(CallNode callNode, TexlBinding binding) - { - Contracts.AssertValue(callNode); - Contracts.AssertValue(binding); - - return Arg0RequiresAsync(callNode, binding); - } - public override bool IsServerDelegatable(CallNode callNode, TexlBinding binding) { Contracts.AssertValue(callNode); diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt index 4fb012cb4b..7d7965b101 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/MutationScripts/Remove.txt @@ -27,6 +27,9 @@ Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTi >> Remove(t1, {a:true}) Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-19: Missing column. Your formula is missing a column 'b' with a type of 'Text'. +>> Remove(t1, {a:true,b:true}) +Errors: Error 0-6: The function 'Remove' has some invalid arguments.|Error 11-26: Incompatible type. The 'b' column in the data source you’re updating expects a 'Text' type and you’re using a 'Boolean' type. + >> 2;t1 Table({a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:true,b:"hi",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hello",c:DateTime(2024,1,1,0,0,0,0)},{a:false,b:"hi",c:DateTime(2024,1,1,0,0,0,0)}) From a28021be25e92d79a25c54deb7bd6105165daa9b Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Mon, 30 Dec 2024 14:37:32 -0600 Subject: [PATCH 11/15] Analysis mode removal. --- src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs index eeb666db20..96a9b5475e 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs @@ -168,8 +168,8 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp if (!argType.IsRecord) { if (argCount >= 3 && i == argCount - 1 && - ((!context.AnalysisMode && BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) || - (context.AnalysisMode && (DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules) || + ((context.Features.PowerFxV1CompatibilityRules && BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) || + (!context.Features.PowerFxV1CompatibilityRules && (DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules) || BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules))))) { continue; From 243b0742425804009308dfe89573ab2637cbc071 Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Tue, 31 Dec 2024 09:31:55 -0600 Subject: [PATCH 12/15] Fix. --- .../Microsoft.PowerFx.Core/Texl/Remove.cs | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs index 96a9b5475e..a6f7943471 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Texl/Remove.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Linq; -using System.Numerics; using Microsoft.PowerFx.Core.App.ErrorContainers; using Microsoft.PowerFx.Core.Binding; using Microsoft.PowerFx.Core.Entities; @@ -49,7 +48,7 @@ public override bool TryGetTypeForArgSuggestionAt(int argIndex, out DType type) return base.TryGetTypeForArgSuggestionAt(argIndex, out type); } - public RemoveBaseFunction(int arityMax, params DType[] paramTypes) + public RemoveBaseFunction(int arityMax, params DType[] paramTypes) : base("Remove", TexlStrings.AboutRemove, FunctionCategories.Behavior, DType.EmptyTable, 0, 2, arityMax, paramTypes) { } @@ -99,6 +98,14 @@ public override bool IsAsyncInvocation(CallNode callNode, TexlBinding binding) return Arg0RequiresAsync(callNode, binding); } + + public bool CheckEnumType(Features features, DType argType) + { + var enumValid = BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: features.PowerFxV1CompatibilityRules); + + return (features.StronglyTypedBuiltinEnums && enumValid) || + (!features.StronglyTypedBuiltinEnums && (DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: features.PowerFxV1CompatibilityRules) || enumValid)); + } } // Remove(collection:*[], item1:![], item2:![], ..., ["All"]) @@ -167,10 +174,7 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp if (!argType.IsRecord) { - if (argCount >= 3 && i == argCount - 1 && - ((context.Features.PowerFxV1CompatibilityRules && BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) || - (!context.Features.PowerFxV1CompatibilityRules && (DType.String.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules) || - BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argType, exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules))))) + if (argCount >= 3 && i == argCount - 1 && CheckEnumType(context.Features, argType)) { continue; } @@ -368,9 +372,7 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp } } - if (args.Length == 3 && - ((context.Features.PowerFxV1CompatibilityRules && !BuiltInEnums.RemoveFlagsEnum.FormulaType._type.Accepts(argTypes[2], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)) || - (!context.Features.PowerFxV1CompatibilityRules && !DType.String.Accepts(argTypes[2], exact: true, useLegacyDateTimeAccepts: false, usePowerFxV1CompatibilityRules: context.Features.PowerFxV1CompatibilityRules)))) + if (args.Length == 3 && !CheckEnumType(context.Features, argTypes[2])) { fValid = false; errors.EnsureError(DocumentErrorSeverity.Severe, args[2], TexlStrings.ErrRemoveAllArg); From 1bf13e3cec8d5275ebcb0db9bc60dee3c18ab6e5 Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Thu, 2 Jan 2025 16:59:59 -0600 Subject: [PATCH 13/15] PR feedback --- .../Public/Values/CollectionTableValue.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs b/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs index b77fbbdde8..f5ffc29e48 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/Values/CollectionTableValue.cs @@ -184,7 +184,7 @@ public override DValue Last(bool mutationCopy = false) public override async Task> RemoveAsync(IEnumerable recordsToRemove, bool all, CancellationToken cancellationToken) { var ret = false; - var markedToDeletion = new HashSet(); + var markedToDeletionIndexes = new HashSet(); var errors = new List(); cancellationToken.ThrowIfCancellationRequested(); @@ -194,26 +194,29 @@ public override async Task> RemoveAsync(IEnumerable dRecord = Marshal(item); if (await MatchesAsync(dRecord.Value, recordToRemove, cancellationToken).ConfigureAwait(false)) { - if (markedToDeletion.Contains(item)) + if (markedToDeletionIndexes.Contains(i)) { continue; } else { found = true; - markedToDeletion.Add(item); + markedToDeletionIndexes.Add(i); } if (!all) @@ -230,9 +233,9 @@ public override async Task> RemoveAsync(IEnumerable Date: Mon, 20 Jan 2025 19:28:21 +0100 Subject: [PATCH 14/15] Use item dynamic properties special handling for body parameter (#2823) Add new connector setting _UseItemDynamicPropertiesSpecialHandling_ Remove _UseDefaultBodyNameForSinglePropertyObject_ as it was not fixing the issue as expected Special case handling when - body name is 'item' - body inner object is 'dynamicProperties' - there is only one property in inner object In that base the body will be fully flattened and we will retain the 'body' name for the parameter. --- .../ConnectorFunction.cs | 25 +++-- .../Execution/FormulaValueSerializer.cs | 39 ++++---- .../Execution/HttpFunctionInvoker.cs | 24 ++++- .../Internal/ConnectorParameterInternals.cs | 2 + .../Public/ConnectorSettings.cs | 17 ++-- .../PowerPlatformConnectorTests.cs | 93 ++++++++++++++++++- 6 files changed, 158 insertions(+), 42 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Connectors/ConnectorFunction.cs b/src/libraries/Microsoft.PowerFx.Connectors/ConnectorFunction.cs index ee49cf5002..522c8b8017 100644 --- a/src/libraries/Microsoft.PowerFx.Connectors/ConnectorFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Connectors/ConnectorFunction.cs @@ -1373,8 +1373,9 @@ private ConnectorParameterInternals Initialize() string bodySchemaReferenceId = null; bool schemaLessBody = false; bool fatalError = false; + bool specialBodyHandling = false; string contentType = OpenApiExtensions.ContentType_ApplicationJson; - ConnectorErrors errorsAndWarnings = new ConnectorErrors(); + ConnectorErrors errorsAndWarnings = new ConnectorErrors(); foreach (OpenApiParameter parameter in Operation.Parameters) { @@ -1458,14 +1459,21 @@ private ConnectorParameterInternals Initialize() foreach (KeyValuePair bodyProperty in bodySchema.Properties) { OpenApiSchema bodyPropertySchema = bodyProperty.Value; - string bodyPropertyName = bodyProperty.Key; - bool bodyPropertyRequired = bodySchema.Required.Contains(bodyPropertyName); - bool bodyPropertyHiddenRequired = false; - - if (ConnectorSettings.UseDefaultBodyNameForSinglePropertyObject && bodySchema.Properties.Count == 1) + string bodyPropertyName = bodyProperty.Key; + bool bodyPropertyHiddenRequired = false; + + // Power Apps has a special handling for the body in this case + // where it doesn't follow the swagger file + if (ConnectorSettings.UseItemDynamicPropertiesSpecialHandling && + bodyName == "item" && + bodyPropertyName == "dynamicProperties" && + bodySchema.Properties.Count == 1) { bodyPropertyName = bodyName; - } + specialBodyHandling = true; + } + + bool bodyPropertyRequired = bodySchema.Required.Contains(bodyPropertyName) || (ConnectorSettings.UseItemDynamicPropertiesSpecialHandling && requestBody.Required); if (bodyPropertySchema.IsInternal()) { @@ -1611,7 +1619,8 @@ private ConnectorParameterInternals Initialize() ContentType = contentType, BodySchemaReferenceId = bodySchemaReferenceId, ParameterDefaultValues = parameterDefaultValues, - SchemaLessBody = schemaLessBody + SchemaLessBody = schemaLessBody, + SpecialBodyHandling = specialBodyHandling }; } diff --git a/src/libraries/Microsoft.PowerFx.Connectors/Execution/FormulaValueSerializer.cs b/src/libraries/Microsoft.PowerFx.Connectors/Execution/FormulaValueSerializer.cs index 048d6ed780..d1da3872f4 100644 --- a/src/libraries/Microsoft.PowerFx.Connectors/Execution/FormulaValueSerializer.cs +++ b/src/libraries/Microsoft.PowerFx.Connectors/Execution/FormulaValueSerializer.cs @@ -108,21 +108,8 @@ private async Task WriteObjectAsync(string objectName, ISwaggerSchema schema, IE await WritePropertyAsync( nv.Name, new SwaggerSchema( - type: nv.Value.Type._type.Kind switch - { - DKind.Number => "number", - DKind.Decimal => "number", - DKind.String or - DKind.Date or - DKind.DateTime or - DKind.DateTimeNoTimeZone => "string", - DKind.Boolean => "boolean", - DKind.Record => "object", - DKind.Table => "array", - DKind.ObjNull => "null", - _ => $"type: unknown_dkind {nv.Value.Type._type.Kind}" - }, - format: GetDateFormat(nv.Value.Type._type.Kind)), + type: GetType(nv.Value.Type), + format: GetFormat(nv.Value.Type)), nv.Value).ConfigureAwait(false); } } @@ -130,9 +117,27 @@ DKind.DateTime or EndObject(objectName); } - private static string GetDateFormat(DKind kind) + internal static string GetType(FormulaType type) { - return kind switch + return type._type.Kind switch + { + DKind.Number => "number", + DKind.Decimal => "number", + DKind.String or + DKind.Date or + DKind.DateTime or + DKind.DateTimeNoTimeZone => "string", + DKind.Boolean => "boolean", + DKind.Record => "object", + DKind.Table => "array", + DKind.ObjNull => "null", + _ => $"type: unknown_dkind {type._type.Kind}" + }; + } + + internal static string GetFormat(FormulaType type) + { + return type._type.Kind switch { DKind.Date => "date", DKind.DateTime => "date-time", diff --git a/src/libraries/Microsoft.PowerFx.Connectors/Execution/HttpFunctionInvoker.cs b/src/libraries/Microsoft.PowerFx.Connectors/Execution/HttpFunctionInvoker.cs index 1a01418490..b304ce4f19 100644 --- a/src/libraries/Microsoft.PowerFx.Connectors/Execution/HttpFunctionInvoker.cs +++ b/src/libraries/Microsoft.PowerFx.Connectors/Execution/HttpFunctionInvoker.cs @@ -57,15 +57,28 @@ public async Task BuildRequest(FormulaValue[] args, IConvert // Header names are not case sensitive. // From RFC 2616 - "Hypertext Transfer Protocol -- HTTP/1.1", Section 4.2, "Message Headers" var headers = new Dictionary(StringComparer.OrdinalIgnoreCase); - Dictionary bodyParts = new (); + Dictionary bodyParts = new (); Dictionary incomingParameters = ConvertToNamedParameters(args); string contentType = null; foreach (KeyValuePair param in _function._internals.OpenApiBodyParameters) - { + { if (incomingParameters.TryGetValue(param.Key.Name, out var paramValue)) { - bodyParts.Add(param.Key.Name, (param.Key.Schema, paramValue)); + if (_function._internals.SpecialBodyHandling && paramValue is RecordValue rv) + { + foreach (NamedValue field in rv.Fields) + { + string type = FormulaValueSerializer.GetType(field.Value.Type); + string format = FormulaValueSerializer.GetFormat(field.Value.Type); + + bodyParts.Add(field.Name, (new SwaggerSchema(type, format), field.Value)); + } + } + else + { + bodyParts.Add(param.Key.Name, (param.Key.Schema, paramValue)); + } } else if (param.Key.Schema.Default != null && param.Value != null) { @@ -200,6 +213,7 @@ public Dictionary ConvertToNamedParameters(FormulaValue[] // Parameter names are case sensitive. Dictionary map = new (); + bool specialBodyHandling = _function._internals.SpecialBodyHandling; // Seed with default values. This will get overwritten if provided. foreach (KeyValuePair kv in _function._internals.ParameterDefaultValues) @@ -217,9 +231,9 @@ public Dictionary ConvertToNamedParameters(FormulaValue[] { string parameterName = _function.RequiredParameters[i].Name; FormulaValue paramValue = args[i]; - + // Objects are always flattenned - if (paramValue is RecordValue record && !_function.RequiredParameters[i].IsBodyParameter) + if (paramValue is RecordValue record && (specialBodyHandling || !_function.RequiredParameters[i].IsBodyParameter)) { foreach (NamedValue field in record.Fields) { diff --git a/src/libraries/Microsoft.PowerFx.Connectors/Internal/ConnectorParameterInternals.cs b/src/libraries/Microsoft.PowerFx.Connectors/Internal/ConnectorParameterInternals.cs index 5d5ea43c14..49d8471ed5 100644 --- a/src/libraries/Microsoft.PowerFx.Connectors/Internal/ConnectorParameterInternals.cs +++ b/src/libraries/Microsoft.PowerFx.Connectors/Internal/ConnectorParameterInternals.cs @@ -20,5 +20,7 @@ internal class ConnectorParameterInternals internal string BodySchemaReferenceId { get; init; } internal Dictionary ParameterDefaultValues { get; init; } + + internal bool SpecialBodyHandling { get; init; } } } diff --git a/src/libraries/Microsoft.PowerFx.Connectors/Public/ConnectorSettings.cs b/src/libraries/Microsoft.PowerFx.Connectors/Public/ConnectorSettings.cs index 82431b299a..87b360205e 100644 --- a/src/libraries/Microsoft.PowerFx.Connectors/Public/ConnectorSettings.cs +++ b/src/libraries/Microsoft.PowerFx.Connectors/Public/ConnectorSettings.cs @@ -92,16 +92,17 @@ public bool ExposeInternalParamsWithoutDefaultValue /// This flag will force all enums to be returns as FormulaType.String or FormulaType.Decimal regardless of x-ms-enum-*. /// This flag is only in effect when SupportXMsEnumValues is true. /// - public bool ReturnEnumsAsPrimitive { get; init; } = false; - + public bool ReturnEnumsAsPrimitive { get; init; } = false; + /// - /// In Power Apps, when a body parameter is used it's flattened and we create one parameter for each - /// body object property. With that logic each parameter name will be the object property name. - /// When set, this setting will use the real body name specified in the swagger instead of the property name - /// of the object, provided there is only one property. + /// This flag enables some special handling for the body parameter, when + /// - body name is 'item' + /// - body inner object is 'dynamicProperties' + /// - there is only one property in inner object + /// In that base the body will be fully flattened and we will retain the 'body' name for the parameter. /// - public bool UseDefaultBodyNameForSinglePropertyObject { get; init; } = false; - + public bool UseItemDynamicPropertiesSpecialHandling { get; init; } = false; + public ConnectorCompatibility Compatibility { get; init; } = ConnectorCompatibility.Default; } diff --git a/src/tests/Microsoft.PowerFx.Connectors.Tests.Shared/PowerPlatformConnectorTests.cs b/src/tests/Microsoft.PowerFx.Connectors.Tests.Shared/PowerPlatformConnectorTests.cs index 77c7cf72fe..4a7cabe87e 100644 --- a/src/tests/Microsoft.PowerFx.Connectors.Tests.Shared/PowerPlatformConnectorTests.cs +++ b/src/tests/Microsoft.PowerFx.Connectors.Tests.Shared/PowerPlatformConnectorTests.cs @@ -2402,20 +2402,105 @@ public async Task SQL_ExecuteStoredProc_Scoped() [Theory] [InlineData(true)] [InlineData(false)] - public void ExchangeOnlineTest2(bool useDefaultBodyNameForSinglePropertyObject) + public async Task ExchangeOnlineTest2(bool useItemDynamicPropertiesSpecialHandling) { + bool live = false; using var testConnector = new LoggingTestServer(@"Swagger\ExcelOnlineBusiness.swagger.json", _output); List functions = OpenApiParser.GetFunctions( - new ConnectorSettings("Excel") + new ConnectorSettings("ExcelOnline") { Compatibility = ConnectorCompatibility.Default, - UseDefaultBodyNameForSinglePropertyObject = useDefaultBodyNameForSinglePropertyObject + UseItemDynamicPropertiesSpecialHandling = useItemDynamicPropertiesSpecialHandling }, testConnector._apiDocument).ToList(); ConnectorFunction patchItem = functions.First(f => f.Name == "PatchItem"); + ConnectorParameter itemparam = useItemDynamicPropertiesSpecialHandling ? patchItem.RequiredParameters[6] : patchItem.OptionalParameters[2]; - Assert.Equal(!useDefaultBodyNameForSinglePropertyObject ? "dynamicProperties" : "item", patchItem.OptionalParameters[2].Name); + Assert.Equal(!useItemDynamicPropertiesSpecialHandling ? "dynamicProperties" : "item", itemparam.Name); + + FormulaValue[] parameters = new FormulaValue[7]; + parameters[0] = FormulaValue.New("b!IbvdIRe4LEGypNQpzV_eHMlG3PtubVREtOzk7doKeFvkIs8VRqloT4mtkIOb6aTB"); + parameters[1] = FormulaValue.New("013DZ3QDGY2Y23HOQN5BC2HUMJWD7G4UPL"); + parameters[2] = FormulaValue.New("{E5A21CC6-3B17-48DE-84D7-0326A06B38F4}"); + parameters[3] = FormulaValue.New("035fd7a2-34d6-4a6f-a885-a646b1398012"); + parameters[4] = FormulaValue.New("me"); + parameters[5] = FormulaValue.New("__PowerAppsId__"); + + parameters[6] = useItemDynamicPropertiesSpecialHandling + + ? // Required parameter + RecordValue.NewRecordFromFields( + new NamedValue("item", RecordValue.NewRecordFromFields( + new NamedValue("Column1", FormulaValue.New(171))))) + + : // Optional parameters + RecordValue.NewRecordFromFields( + new NamedValue("dynamicProperties", RecordValue.NewRecordFromFields( + new NamedValue("Column1", FormulaValue.New(171))))); + + using var httpClient = live ? new HttpClient() : new HttpClient(testConnector); + + if (!live) + { + string output = @"{ + ""@odata.context"": ""https://excelonline-wcus.azconn-wcus-001.p.azurewebsites.net/$metadata#drives('b%21IbvdIRe4LEGypNQpzV_eHMlG3PtubVREtOzk7doKeFvkIs8VRqloT4mtkIOb6aTB')/Files('013DZ3QDGY2Y23HOQN5BC2HUMJWD7G4UPL')/Tables('%7BE5A21CC6-3B17-48DE-84D7-0326A06B38F4%7D')/items/$entity"", + ""@odata.etag"": """", + ""ItemInternalId"": ""035fd7a2-34d6-4a6f-a885-a646b1398012"", + ""Column1"": ""171"", + ""Column2"": ""Customer1"", + ""Column3"": """", + ""__PowerAppsId__"": ""035fd7a2-34d6-4a6f-a885-a646b1398012"" +}"; + testConnector.SetResponse(output, HttpStatusCode.OK); + } + + string jwt = "eyJ0e..."; + using PowerPlatformConnectorClient client = new PowerPlatformConnectorClient("https://49970107-0806-e5a7-be5e-7c60e2750f01.12.common.firstrelease.azure-apihub.net", "49970107-0806-e5a7-be5e-7c60e2750f01", "e24a1ac719284479a4817a0c5bb6ef58", () => jwt, httpClient) + { + SessionId = "a41bd03b-6c3c-4509-a844-e8c51b61f878", + }; + + BaseRuntimeConnectorContext context = new TestConnectorRuntimeContext("ExcelOnline", client, console: _output); + FormulaValue result = await patchItem.InvokeAsync(parameters, context, CancellationToken.None); + + // Can't test the result as it's ![] and is an empty RecordValue + + if (live) + { + return; + } + + string version = PowerPlatformConnectorClient.Version; + string expected = useItemDynamicPropertiesSpecialHandling + ? $@"POST https://49970107-0806-e5a7-be5e-7c60e2750f01.12.common.firstrelease.azure-apihub.net/invoke + authority: 49970107-0806-e5a7-be5e-7c60e2750f01.12.common.firstrelease.azure-apihub.net + Authorization: Bearer {jwt} + path: /invoke + scheme: https + x-ms-client-environment-id: /providers/Microsoft.PowerApps/environments/49970107-0806-e5a7-be5e-7c60e2750f01 + x-ms-client-session-id: a41bd03b-6c3c-4509-a844-e8c51b61f878 + x-ms-request-method: PATCH + x-ms-request-url: /apim/excelonlinebusiness/e24a1ac719284479a4817a0c5bb6ef58/drives/b%21IbvdIRe4LEGypNQpzV_eHMlG3PtubVREtOzk7doKeFvkIs8VRqloT4mtkIOb6aTB/files/013DZ3QDGY2Y23HOQN5BC2HUMJWD7G4UPL/tables/%7BE5A21CC6-3B17-48DE-84D7-0326A06B38F4%7D/items/035fd7a2-34d6-4a6f-a885-a646b1398012?source=me&idColumn=__PowerAppsId__ + x-ms-user-agent: PowerFx/{version} + [content-header] Content-Type: application/json; charset=utf-8 + [body] {{""Column1"":171}} +" + : $@"POST https://49970107-0806-e5a7-be5e-7c60e2750f01.12.common.firstrelease.azure-apihub.net/invoke + authority: 49970107-0806-e5a7-be5e-7c60e2750f01.12.common.firstrelease.azure-apihub.net + Authorization: Bearer {jwt} + path: /invoke + scheme: https + x-ms-client-environment-id: /providers/Microsoft.PowerApps/environments/49970107-0806-e5a7-be5e-7c60e2750f01 + x-ms-client-session-id: a41bd03b-6c3c-4509-a844-e8c51b61f878 + x-ms-request-method: PATCH + x-ms-request-url: /apim/excelonlinebusiness/e24a1ac719284479a4817a0c5bb6ef58/drives/b%21IbvdIRe4LEGypNQpzV_eHMlG3PtubVREtOzk7doKeFvkIs8VRqloT4mtkIOb6aTB/files/013DZ3QDGY2Y23HOQN5BC2HUMJWD7G4UPL/tables/%7BE5A21CC6-3B17-48DE-84D7-0326A06B38F4%7D/items/035fd7a2-34d6-4a6f-a885-a646b1398012?source=me&idColumn=__PowerAppsId__ + x-ms-user-agent: PowerFx/{version} + [content-header] Content-Type: application/json; charset=utf-8 + [body] {{""dynamicProperties"":{{""Column1"":171}}}} +"; + + Assert.Equal(expected, testConnector._log.ToString()); } public class HttpLogger : HttpClient From cc8c3b1b4428ed86febaa3750c5a9fa6d8907a80 Mon Sep 17 00:00:00 2001 From: Mike Stall Date: Wed, 22 Jan 2025 13:54:58 -0800 Subject: [PATCH 15/15] Unify IAsyncTexlFunction (#2812) Address #2751. This unifies most of them. To scope this PR, there are subissues on 2751 for remaining issues. We have a plethora of IAsyncTexlFunction* interfaces. Want to consolidate on 1, and make it public. This Ttake the parameters on the various IAsyncTexlFunction* overloads and move them as properties on a new public `FunctionInvokerInfo` class. And then have a new IAsyncTexlFunction* that takes the invoker. And remove all the others, migrating them onto this - and fixing the various bugs/hacks along the way that would impede migration. Most fundamentally, this FunctionInvokerInfo must have access to interpreter state and so must live in the interpreter. (Whereas some of the IAsyncFunction* interfaces live in core). Successfully removes several here so we get a net reduction, and shows a path to remove the rest. This problem quickly touches other problems: - **How to map from the TexlFunction to the invoker**. This is made more complicated because we register on PowerFxConfig, but that lives in Fx.Core and can't refer to interpreter implementations. - **dll layering around Fx.Json**: Some function implementations live in Fx.Json, but that specifically _does not_ depend on interpreter (because it is included by PowerApps). - **Standard Pipeline**: how to handle default args, common error handling scenarios, etc. Today, that's still in interpreter. Whereas we really want those checks to be in the IR and shared across front-ends (and available to designer). - **Split up Library.cs and class** - we have one giant class, partial over many files, containing 100s of function impls. Just give each impl its own file, similar to how we did for TexlFunction and the static declarations. - **lack of unit testing**: Today, most of our interpreter coverage comes from .txt files. But we should have C# unit tests on interpreter classes that provide coverage on core classes, like EvalVisitor. Some related prereq fixes that will directly help here: - Can we remove runner/context from Join? and leverage LambdaValue - which already closes over these. - fix Json layering problem. - IsType/AsType _UO shouldn't live in Json. They should work on any UO. - Remove _additionalFunctions from the serviceProvider. --------- Co-authored-by: Mike --- .../Texl/ConnectorTexlFunction.cs | 8 +- .../Functions/IAsyncTexlFunction.cs | 1 + .../Functions/IAsyncTexlFunction3.cs | 16 -- .../Functions/IAsyncTexlFunction4.cs | 1 + .../Functions/IAsyncTexlFunction5.cs | 4 +- .../Public/Config/PowerFxConfig.cs | 1 + .../CustomFunction/CustomTexlFunction.cs | 7 +- .../EvalVisitor.cs | 190 ++++++++++++------ .../Functions/FileInfoImpl.cs | 7 +- .../Functions/FunctionInvokeInfo.cs | 79 ++++++++ .../Functions/IAsyncConnectorTexlFunction.cs | 16 -- .../Functions/IAsyncTexlFunction2.cs | 18 -- .../Functions/IAsyncTexlFunctionJoin.cs | 18 -- .../Functions/IFunctionInvoker.cs | 17 ++ .../Functions/LibraryMutation.cs | 77 ++++--- .../Functions/LibraryTable.cs | 16 +- .../Values/LambdaFormulaValue.cs | 2 + .../ClearFunctionTests.cs | 5 +- .../EvalVisitorTests.cs | 50 +++++ .../PatchFunctionTests.cs | 12 +- .../RecalcEngineTests.cs | 18 +- 21 files changed, 383 insertions(+), 180 deletions(-) delete mode 100644 src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction3.cs create mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/FunctionInvokeInfo.cs delete mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncConnectorTexlFunction.cs delete mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunction2.cs delete mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunctionJoin.cs create mode 100644 src/libraries/Microsoft.PowerFx.Interpreter/Functions/IFunctionInvoker.cs create mode 100644 src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/EvalVisitorTests.cs diff --git a/src/libraries/Microsoft.PowerFx.Connectors/Texl/ConnectorTexlFunction.cs b/src/libraries/Microsoft.PowerFx.Connectors/Texl/ConnectorTexlFunction.cs index b139be1570..12effc3ce1 100644 --- a/src/libraries/Microsoft.PowerFx.Connectors/Texl/ConnectorTexlFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Connectors/Texl/ConnectorTexlFunction.cs @@ -12,13 +12,14 @@ using Microsoft.PowerFx.Core.Localization; using Microsoft.PowerFx.Core.Types; using Microsoft.PowerFx.Core.Utils; +using Microsoft.PowerFx.Functions; using Microsoft.PowerFx.Intellisense; using Microsoft.PowerFx.Types; using static Microsoft.PowerFx.Connectors.ConnectorHelperFunctions; namespace Microsoft.PowerFx.Connectors { - internal class ConnectorTexlFunction : TexlFunction, IAsyncConnectorTexlFunction, IHasUnsupportedFunctions + internal class ConnectorTexlFunction : TexlFunction, IFunctionInvoker, IHasUnsupportedFunctions { public ConnectorFunction ConnectorFunction { get; } @@ -85,8 +86,11 @@ public override bool TryGetParamDescription(string paramName, out string paramDe public override bool HasSuggestionsForParam(int argumentIndex) => argumentIndex <= MaxArity; - public async Task InvokeAsync(FormulaValue[] args, IServiceProvider serviceProvider, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + FormulaValue[] args = invokeInfo.Args.ToArray(); // https://github.com/microsoft/Power-Fx/issues/2817 + var serviceProvider = invokeInfo.FunctionServices; + cancellationToken.ThrowIfCancellationRequested(); BaseRuntimeConnectorContext runtimeContext = serviceProvider.GetService(typeof(BaseRuntimeConnectorContext)) as BaseRuntimeConnectorContext ?? throw new InvalidOperationException("RuntimeConnectorContext is missing from service provider"); diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction.cs index cb15c3e963..0ecc98c78e 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction.cs @@ -8,6 +8,7 @@ namespace Microsoft.PowerFx.Core.Functions { // A Texl function capable of async invokes. + // This only lives in Core to enable Fx.Json funcs impl (which doesn't depend on interpreter). internal interface IAsyncTexlFunction { Task InvokeAsync(FormulaValue[] args, CancellationToken cancellationToken); diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction3.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction3.cs deleted file mode 100644 index e4ac6bfbe8..0000000000 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction3.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.PowerFx.Types; - -namespace Microsoft.PowerFx.Core.Functions -{ - // A Texl function capable of async invokes, using IRContext. - internal interface IAsyncTexlFunction3 - { - Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken); - } -} diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction4.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction4.cs index c68908571f..0ab689bd91 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction4.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction4.cs @@ -9,6 +9,7 @@ namespace Microsoft.PowerFx.Core.Functions { // A Texl function capable of async invokes, using TimeZoneInfo and IRContext. + // Remove this: https://github.com/microsoft/Power-Fx/issues/2818 internal interface IAsyncTexlFunction4 { Task InvokeAsync(TimeZoneInfo timezoneInfo, FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken); diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction5.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction5.cs index 9dff48e73a..69b408d9a1 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction5.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/IAsyncTexlFunction5.cs @@ -7,8 +7,10 @@ using Microsoft.PowerFx.Types; namespace Microsoft.PowerFx.Core.Functions -{ +{ // Texl function interface with IServiceProvider + // Only product impl is JsonFunctionImpl. + // Remove this: https://github.com/microsoft/Power-Fx/issues/2818 internal interface IAsyncTexlFunction5 { Task InvokeAsync(IServiceProvider runtimeServiceProvider, FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken); diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/Config/PowerFxConfig.cs b/src/libraries/Microsoft.PowerFx.Core/Public/Config/PowerFxConfig.cs index d873582a56..8942026dde 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/Config/PowerFxConfig.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/Config/PowerFxConfig.cs @@ -52,6 +52,7 @@ public SymbolTable SymbolTable set => _symbolTable = value; } + // Remove this: https://github.com/microsoft/Power-Fx/issues/2821 internal readonly Dictionary AdditionalFunctions = new (); [Obsolete("Use Config.EnumStore or symboltable directly")] diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/CustomFunction/CustomTexlFunction.cs b/src/libraries/Microsoft.PowerFx.Interpreter/CustomFunction/CustomTexlFunction.cs index d0db543958..44c38a1f17 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/CustomFunction/CustomTexlFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/CustomFunction/CustomTexlFunction.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Numerics; using System.Threading; using System.Threading.Tasks; @@ -22,7 +23,7 @@ namespace Microsoft.PowerFx /// /// Internal adapter for adding custom functions. /// - internal class CustomTexlFunction : TexlFunction + internal class CustomTexlFunction : TexlFunction, IFunctionInvoker { public Func> _impl; @@ -53,8 +54,10 @@ public CustomTexlFunction(DPath ns, string name, FunctionCategories functionCate yield return CustomFunctionUtility.GenerateArgSignature(_argNames, ParamTypes); } - public virtual Task InvokeAsync(IServiceProvider serviceProvider, FormulaValue[] args, CancellationToken cancellationToken) + public virtual Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var serviceProvider = invokeInfo.FunctionServices; + var args = invokeInfo.Args.ToArray(); // remove ToArray: https://github.com/microsoft/Power-Fx/issues/2817 return _impl(serviceProvider, args, cancellationToken); } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/EvalVisitor.cs b/src/libraries/Microsoft.PowerFx.Interpreter/EvalVisitor.cs index 7c93430b9b..621f62d28a 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/EvalVisitor.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/EvalVisitor.cs @@ -265,6 +265,100 @@ private async Task TryHandleSetProperty(CallNode node, EvalVisitor return result; } + + // Given a TexlFunction, get the implementation to invoke. + private IFunctionInvoker GetInvoker(TexlFunction func) + { + if (func is IFunctionInvoker invoker) + { + return invoker; + } + + if (func is UserDefinedFunction userDefinedFunc) + { + return new UserDefinedFunctionAdapter(userDefinedFunc); + } + + if (FunctionImplementations.TryGetValue(func, out AsyncFunctionPtr ptr)) + { + return new AsyncFunctionPtrAdapter(ptr); + } + + return null; + } + + // Adapter for AsyncFunctionPtr to common invoker interface. + private class AsyncFunctionPtrAdapter : IFunctionInvoker + { + private readonly AsyncFunctionPtr _ptr; + + public AsyncFunctionPtrAdapter(AsyncFunctionPtr ptr) + { + _ptr = ptr; + } + + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) + { + var args = invokeInfo.Args.ToArray(); + var context = invokeInfo.Context; + var evalVisitor = invokeInfo.Runner; + var irContext = invokeInfo.IRContext; + + var result = await _ptr(evalVisitor, context, irContext, args).ConfigureAwait(false); + + return result; + } + } + + // Adapter for UDF to common invoker. + // This still ensures that *invoking* a UDF has the same semantics as invoking other function calls. + private class UserDefinedFunctionAdapter : IFunctionInvoker + { + private readonly UserDefinedFunction _udf; + + public UserDefinedFunctionAdapter(UserDefinedFunction udf) + { + _udf = udf; + } + + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) + { + var args = invokeInfo.Args.ToArray(); + var context = invokeInfo.Context; + var evalVisitor = invokeInfo.Runner; + + var udfStack = evalVisitor._udfStack; + + UDFStackFrame frame = new UDFStackFrame(_udf, args); + UDFStackFrame framePop = null; + FormulaValue result = null; + + try + { + // Push this so that we have access to args. + udfStack.Push(frame); + + // https://github.com/microsoft/Power-Fx/issues/2822 + // This repeats IRTranslator each time. Do once and save. + (var irnode, _) = _udf.GetIRTranslator(); + + evalVisitor.CheckCancel(); + + result = await irnode.Accept(evalVisitor, context).ConfigureAwait(false); + } + finally + { + framePop = udfStack.Pop(); + } + + if (frame != framePop) + { + throw new Exception("Something went wrong. UDF stack values didn't match."); + } + + return result; + } + } public override async ValueTask Visit(CallNode node, EvalVisitorContext context) { @@ -300,7 +394,8 @@ public override async ValueTask Visit(CallNode node, EvalVisitorCo args[i] = await child.Accept(this, context.IncrementStackDepthCounter()).ConfigureAwait(false); } else - { + { + // This is where Lambdas are created. They close over key values to invoke. args[i] = new LambdaFormulaValue(node.IRContext, child, this, context); } } @@ -308,32 +403,50 @@ public override async ValueTask Visit(CallNode node, EvalVisitorCo var childContext = context.SymbolContext.WithScope(node.Scope); FormulaValue result; + + // Remove this: https://github.com/microsoft/Power-Fx/issues/2821 IReadOnlyDictionary extraFunctions = _services.GetService>(); try { - if (func is IAsyncTexlFunction asyncFunc || extraFunctions?.TryGetValue(func, out asyncFunc) == true) + IFunctionInvoker invoker = GetInvoker(func); + + // Standard invoke path. Make everything go through here. + // Eventually collapse all cases to this. + if (invoker != null) { - result = await asyncFunc.InvokeAsync(args, _cancellationToken).ConfigureAwait(false); + var invokeInfo = new FunctionInvokeInfo + { + Args = args, + FunctionServices = _services, + Runner = this, + Context = context.IncrementStackDepthCounter(childContext), + IRContext = node.IRContext, + }; + + result = await invoker.InvokeAsync(invokeInfo, _cancellationToken).ConfigureAwait(false); } -#pragma warning disable CS0618 // Type or member is obsolete - else if (func is IAsyncTexlFunction2 asyncFunc2) -#pragma warning restore CS0618 // Type or member is obsolete + else if (func is IAsyncTexlFunction asyncFunc) { - result = await asyncFunc2.InvokeAsync(this.GetFormattingInfo(), args, _cancellationToken).ConfigureAwait(false); + result = await asyncFunc.InvokeAsync(args, _cancellationToken).ConfigureAwait(false); } - else if (func is IAsyncTexlFunction3 asyncFunc3) + else if (extraFunctions?.TryGetValue(func, out asyncFunc) == true) { - result = await asyncFunc3.InvokeAsync(node.IRContext.ResultType, args, _cancellationToken).ConfigureAwait(false); + result = await asyncFunc.InvokeAsync(args, _cancellationToken).ConfigureAwait(false); } else if (func is IAsyncTexlFunction4 asyncFunc4) { + // https://github.com/microsoft/Power-Fx/issues/2818 + // This is used for Json() functions. IsType, AsType result = await asyncFunc4.InvokeAsync(TimeZoneInfo, node.IRContext.ResultType, args, _cancellationToken).ConfigureAwait(false); } else if (func is IAsyncTexlFunction5 asyncFunc5) { + // https://github.com/microsoft/Power-Fx/issues/2818 + // This is used for Json() functions. BasicServiceProvider services2 = new BasicServiceProvider(_services); + // Invocation should not get its own provider. if (services2.GetService(typeof(TimeZoneInfo)) == null) { services2.AddService(TimeZoneInfo); @@ -346,63 +459,18 @@ public override async ValueTask Visit(CallNode node, EvalVisitorCo result = await asyncFunc5.InvokeAsync(services2, node.IRContext.ResultType, args, _cancellationToken).ConfigureAwait(false); } - else if (func is IAsyncConnectorTexlFunction asyncConnectorTexlFunction) - { - return await asyncConnectorTexlFunction.InvokeAsync(args, _services, _cancellationToken).ConfigureAwait(false); - } - else if (func is CustomTexlFunction customTexlFunc) - { - // If custom function throws an exception, don't catch it - let it propagate up to the host. - result = await customTexlFunc.InvokeAsync(FunctionServices, args, _cancellationToken).ConfigureAwait(false); - } - else if (func is UserDefinedFunction userDefinedFunc) + else { - UDFStackFrame frame = new UDFStackFrame(userDefinedFunc, args); - UDFStackFrame framePop = null; - - try - { - _udfStack.Push(frame); - - (var irnode, _) = userDefinedFunc.GetIRTranslator(); - - this.CheckCancel(); - - result = await irnode.Accept(this, context).ConfigureAwait(false); - } - finally - { - framePop = _udfStack.Pop(); - } - - if (frame != framePop) - { - throw new Exception("Something went wrong. UDF stack values didn't match."); - } + result = CommonErrors.NotYetImplementedFunctionError(node.IRContext, func.Name); } -#pragma warning disable CS0618 // Type or member is obsolete - - // This is a temporary solution to release the Join function for host that want to use it. - else if (func is IAsyncTexlFunctionJoin asyncJoin) + // https://github.com/microsoft/Power-Fx/issues/2820 + // We should remove this check that limits to just Adapter1, so we apply this check to all impls. + if (invoker is AsyncFunctionPtrAdapter) { - result = await asyncJoin.InvokeAsync(this, context.IncrementStackDepthCounter(childContext), node.IRContext, args).ConfigureAwait(false); - } -#pragma warning restore CS0618 // Type or member is obsolete - else - { - if (FunctionImplementations.TryGetValue(func, out AsyncFunctionPtr ptr)) - { - result = await ptr(this, context.IncrementStackDepthCounter(childContext), node.IRContext, args).ConfigureAwait(false); - - if (!(result.IRContext.ResultType._type == node.IRContext.ResultType._type || result is ErrorValue || result.IRContext.ResultType is BlankType)) - { - throw CommonExceptions.RuntimeMisMatch; - } - } - else + if (!(result.IRContext.ResultType._type == node.IRContext.ResultType._type || result is ErrorValue || result.IRContext.ResultType is BlankType)) { - result = CommonErrors.NotYetImplementedFunctionError(node.IRContext, func.Name); + throw CommonExceptions.RuntimeMisMatch; } } } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FileInfoImpl.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FileInfoImpl.cs index 5ce5fe1f96..dff6e02834 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FileInfoImpl.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FileInfoImpl.cs @@ -6,15 +6,18 @@ using System.Threading.Tasks; using Microsoft.PowerFx.Core.Functions; using Microsoft.PowerFx.Core.Texl.Builtins; +using Microsoft.PowerFx.Functions; using Microsoft.PowerFx.Interpreter; using Microsoft.PowerFx.Types; namespace Microsoft.PowerFx { - internal class FileInfoFunctionImpl : FileInfoFunction, IAsyncTexlFunction3 + internal class FileInfoFunctionImpl : FileInfoFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var arg0 = args[0]; if (arg0 is BlankValue || arg0 is ErrorValue) { diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FunctionInvokeInfo.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FunctionInvokeInfo.cs new file mode 100644 index 0000000000..f59553cac3 --- /dev/null +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/FunctionInvokeInfo.cs @@ -0,0 +1,79 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System; +using System.Collections.Generic; +using Microsoft.PowerFx.Core.IR; +using Microsoft.PowerFx.Types; + +namespace Microsoft.PowerFx.Functions +{ + /// + /// The parameters to use with . + /// The information necessary to invoke a function via . + /// The arguments here may be closured over an instance of EvalVisitor/EvalContext, so this + /// is only valid for a specific invocation and can't be reused across invokes. + /// + [ThreadSafeImmutable] + public class FunctionInvokeInfo + { + /// + /// The arguments passed to this function. + /// These may be closed over context and specific to this invocation, so they should + /// not be saved or used outside of this invocation. + /// + public IReadOnlyList Args { get; init; } + + /// + /// The expected return type of this function. This is computed by the binder. + /// Some functions are polymorphic and the return type depends on the intput argument types. + /// + public FormulaType ReturnType => this.IRContext.ResultType; + + /// + /// services for function invocation. This is the set from . + /// + public IServiceProvider FunctionServices { get; init; } + + // Since every method impl would get this, consider add other useful operators on here, like: + // - CheckCancel? + // - error checks, blank checks. + + // Can we find a way to get rid of these ones? Keep internal to limit usage. + // Keep internal until we kind a way to remove. + #region Remove these + + // IrContext has a mutability flag. + internal IRContext IRContext { get; init; } + + // https://github.com/microsoft/Power-Fx/issues/2819 + // Get rid of these... Capture in them in the closure of a lambdaValue. + internal EvalVisitor Runner { get; init; } + + internal EvalVisitorContext Context { get; init; } + #endregion + + // Since this is immutable, clone to get adjusted parameters. + public FunctionInvokeInfo CloneWith(IReadOnlyList newArgs) + { + return new FunctionInvokeInfo + { + Args = newArgs, + FunctionServices = this.FunctionServices, + IRContext = this.IRContext, + Runner = this.Runner, + Context = this.Context + }; + } + + // Helper to create simple case, primarily for testing. + internal static FunctionInvokeInfo New(FormulaType returnType, params FormulaValue[] args) + { + return new FunctionInvokeInfo + { + Args = args, + IRContext = IRContext.NotInSource(returnType) + }; + } + } +} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncConnectorTexlFunction.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncConnectorTexlFunction.cs deleted file mode 100644 index 3fe07d7bbe..0000000000 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncConnectorTexlFunction.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.PowerFx.Types; - -namespace Microsoft.PowerFx.Core.Functions -{ - // A Texl function capable of async invokes. - internal interface IAsyncConnectorTexlFunction - { - Task InvokeAsync(FormulaValue[] args, IServiceProvider context, CancellationToken cancellationToken); - } -} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunction2.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunction2.cs deleted file mode 100644 index fa2313c38a..0000000000 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunction2.cs +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.PowerFx.Functions; -using Microsoft.PowerFx.Types; - -namespace Microsoft.PowerFx.Core.Functions -{ - // A Texl function capable of async invokes. - [Obsolete("This interface is obsolete and will be removed in a future release. Please use IAsyncConnectorTexlFunction instead.")] - internal interface IAsyncTexlFunction2 - { - Task InvokeAsync(FormattingInfo context, FormulaValue[] args, CancellationToken cancellationToken); - } -} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunctionJoin.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunctionJoin.cs deleted file mode 100644 index cd9a2b08cf..0000000000 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IAsyncTexlFunctionJoin.cs +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.PowerFx.Core.IR; -using Microsoft.PowerFx.Functions; -using Microsoft.PowerFx.Types; - -namespace Microsoft.PowerFx.Core.Functions -{ - [Obsolete("This interface is temporary and its meant only to support Join function. It will be soon removed.")] - internal interface IAsyncTexlFunctionJoin - { - Task InvokeAsync(EvalVisitor runner, EvalVisitorContext context, IRContext irContext, FormulaValue[] args); - } -} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IFunctionInvoker.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IFunctionInvoker.cs new file mode 100644 index 0000000000..506e07568f --- /dev/null +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/IFunctionInvoker.cs @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Threading; +using System.Threading.Tasks; +using Microsoft.PowerFx.Types; + +namespace Microsoft.PowerFx.Functions +{ + /// + /// Invoker to execute a function via the interpreter. + /// + public interface IFunctionInvoker + { + Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken); + } +} diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs index cc8c2599d5..a6e986a855 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryMutation.cs @@ -15,10 +15,12 @@ namespace Microsoft.PowerFx.Core.Texl.Builtins { // Patch(dataSource:*[], Record, Updates1, Updates2,…) - internal class PatchImpl : PatchFunction, IAsyncTexlFunction3 + internal class PatchImpl : PatchFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var arg0 = args[0]; if (args[0] is LambdaFormulaValue arg0lazy) @@ -57,10 +59,12 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ // If arg1 is pure PFx record, it will return a runtime not supported error. // Patch(DS, record_with_keys_and_updates) - internal class PatchSingleRecordImpl : PatchSingleRecordFunction, IAsyncTexlFunction3 + internal class PatchSingleRecordImpl : PatchSingleRecordFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var arg0 = args[0]; if (args[0] is LambdaFormulaValue arg0lazy) @@ -93,10 +97,12 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ } // Patch(DS, table_of_rows, table_of_updates) - internal class PatchAggregateImpl : PatchAggregateFunction, IAsyncTexlFunction3 + internal class PatchAggregateImpl : PatchAggregateFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var arg0 = args[0]; if (args[0] is LambdaFormulaValue arg0lazy) @@ -114,7 +120,7 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ if (arg1Rows.Count() != arg2Rows.Count()) { - return CommonErrors.InvalidArgumentError(IRContext.NotInSource(irContext), RuntimeStringResources.ErrAggregateArgsSameNumberOfRecords); + return CommonErrors.InvalidArgumentError(IRContext.NotInSource(invokeInfo.ReturnType), RuntimeStringResources.ErrAggregateArgsSameNumberOfRecords); } List> resultRows = new List>(); @@ -170,10 +176,12 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ // If arg1 is pure PFx record, it will return a runtime not supported error. // Patch(DS, table_of_rows_with_updates) - internal class PatchAggregateSingleTableImpl : PatchAggregateSingleTableFunction, IAsyncTexlFunction3 + internal class PatchAggregateSingleTableImpl : PatchAggregateSingleTableFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var arg0 = args[0]; if (args[0] is LambdaFormulaValue arg0lazy) @@ -229,28 +237,34 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ } } - internal class CollectImpl : CollectFunction, IAsyncTexlFunction3 + internal class CollectImpl : CollectFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { - return await CollectProcess.Process(irContext, args, cancellationToken).ConfigureAwait(false); + var args = invokeInfo.Args; + var returnType = invokeInfo.ReturnType; + + return await CollectProcess.Process(returnType, args, cancellationToken).ConfigureAwait(false); } } - internal class CollectScalarImpl : CollectScalarFunction, IAsyncTexlFunction3 + internal class CollectScalarImpl : CollectScalarFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { - return await CollectProcess.Process(irContext, args, cancellationToken).ConfigureAwait(false); + var args = invokeInfo.Args; + var returnType = invokeInfo.ReturnType; + + return await CollectProcess.Process(returnType, args, cancellationToken).ConfigureAwait(false); } } internal class CollectProcess { - internal static async Task Process(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + internal static async Task Process(FormulaType irContext, IReadOnlyList args, CancellationToken cancellationToken) { FormulaValue arg0; - var argc = args.Length; + var argc = args.Count; // Need to check if the Lazy first argument has been evaluated since it may have already been // evaluated in the ClearCollect case. @@ -341,10 +355,13 @@ internal static async Task Process(FormulaType irContext, FormulaV } // Clear(collection_or_table) - internal class ClearImpl : ClearFunction, IAsyncTexlFunction3 + internal class ClearImpl : ClearFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args; + var returnType = invokeInfo.ReturnType; + if (args[0] is ErrorValue errorValue) { return errorValue; @@ -352,13 +369,13 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ if (args[0] is BlankValue) { - return irContext == FormulaType.Void ? FormulaValue.NewVoid() : FormulaValue.NewBlank(FormulaType.Boolean); + return returnType == FormulaType.Void ? FormulaValue.NewVoid() : FormulaValue.NewBlank(FormulaType.Boolean); } var datasource = (TableValue)args[0]; var ret = await datasource.ClearAsync(cancellationToken).ConfigureAwait(false); - if (irContext == FormulaType.Void) + if (returnType == FormulaType.Void) { return ret.IsError ? FormulaValue.NewError(ret.Error.Errors, FormulaType.Void) : FormulaValue.NewVoid(); } @@ -370,18 +387,22 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ } // ClearCollect(table_or_collection, table|record, ...) - internal class ClearCollectImpl : ClearCollectFunction, IAsyncTexlFunction3 + internal class ClearCollectImpl : ClearCollectFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { + var args = invokeInfo.Args.ToArray(); + if (args[0] is LambdaFormulaValue arg0lazy) { args[0] = await arg0lazy.EvalAsync().ConfigureAwait(false); } + invokeInfo = invokeInfo.CloneWith(args); + var clearFunction = new ClearImpl(); - var cleared = await clearFunction.InvokeAsync(FormulaType.Void, args, cancellationToken).ConfigureAwait(false); + var cleared = await clearFunction.InvokeAsync(invokeInfo, cancellationToken).ConfigureAwait(false); if (cleared is ErrorValue) { @@ -390,16 +411,16 @@ public async Task InvokeAsync(FormulaType irContext, FormulaValue[ var collectFunction = new CollectImpl(); - return await collectFunction.InvokeAsync(irContext, args, cancellationToken).ConfigureAwait(false); + return await collectFunction.InvokeAsync(invokeInfo, cancellationToken).ConfigureAwait(false); } } // ClearCollect(table_or_collection, scalar, ...) - internal class ClearCollectScalarImpl : ClearCollectScalarFunction, IAsyncTexlFunction3 + internal class ClearCollectScalarImpl : ClearCollectScalarFunction, IFunctionInvoker { - public async Task InvokeAsync(FormulaType irContext, FormulaValue[] args, CancellationToken cancellationToken) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { - return await new ClearCollectImpl().InvokeAsync(irContext, args, cancellationToken).ConfigureAwait(false); + return await new ClearCollectImpl().InvokeAsync(invokeInfo, cancellationToken).ConfigureAwait(false); } } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs index a89e8a0f98..e3896aa133 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Functions/LibraryTable.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.PowerFx.Core.Entities; using Microsoft.PowerFx.Core.Functions; @@ -524,8 +525,13 @@ public static async ValueTask FilterTable(EvalVisitor runner, Eval } // Join(t1, t2, LeftRecord.Id = RightRecord.RefId, JoinType.Inner) - public static async ValueTask JoinTables(EvalVisitor runner, EvalVisitorContext context, IRContext irContext, FormulaValue[] args) + public static async ValueTask JoinTables(FunctionInvokeInfo invokeInfo) { + var args = invokeInfo.Args; + var runner = invokeInfo.Runner; + var context = invokeInfo.Context; + var irContext = invokeInfo.IRContext; + if (args[0] is not TableValue leftTable) { return args[0]; @@ -1575,15 +1581,13 @@ public static NamedLambda[] Parse(FormulaValue[] args) } } -#pragma warning disable CS0618 // Type or member is obsolete - internal class JoinImpl : JoinFunction, IAsyncTexlFunctionJoin -#pragma warning restore CS0618 // Type or member is obsolete + internal class JoinImpl : JoinFunction, IFunctionInvoker { public override Type DeclarationType => typeof(JoinFunction); - public async Task InvokeAsync(EvalVisitor runner, EvalVisitorContext context, IRContext irContext, FormulaValue[] args) + public async Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) { - return await Library.JoinTables(runner, context, irContext, args).ConfigureAwait(false); + return await Library.JoinTables(invokeInfo).ConfigureAwait(false); } } } diff --git a/src/libraries/Microsoft.PowerFx.Interpreter/Values/LambdaFormulaValue.cs b/src/libraries/Microsoft.PowerFx.Interpreter/Values/LambdaFormulaValue.cs index 2380b874eb..bd8e2f506e 100644 --- a/src/libraries/Microsoft.PowerFx.Interpreter/Values/LambdaFormulaValue.cs +++ b/src/libraries/Microsoft.PowerFx.Interpreter/Values/LambdaFormulaValue.cs @@ -34,6 +34,8 @@ public async ValueTask EvalAsync() return await EvalInRowScopeAsync(_context).ConfigureAwait(false); } + // Can we remove this and replace with safer implementations? + // https://github.com/microsoft/Power-Fx/issues/2819 public async ValueTask EvalInRowScopeAsync(EvalVisitorContext context) { _runner.CheckCancel(); diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/ClearFunctionTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/ClearFunctionTests.cs index cb9e2a267c..ef5e545dda 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/ClearFunctionTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/ClearFunctionTests.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using Microsoft.PowerFx.Core.Tests; using Microsoft.PowerFx.Core.Texl.Builtins; +using Microsoft.PowerFx.Functions; using Microsoft.PowerFx.Types; using Xunit; @@ -34,7 +35,7 @@ public async Task CheckArgsTestAsync() foreach (var arg in faultyArs) { - var result = await function.InvokeAsync(FormulaType.Void, new FormulaValue[] { arg }, CancellationToken.None); + var result = await function.InvokeAsync(FunctionInvokeInfo.New(FormulaType.Void, arg), CancellationToken.None); if (arg is ErrorValue) { @@ -66,7 +67,7 @@ public async Task CheckArgsTestAsync_V1CompatDisabled() foreach (var arg in faultyArs) { - var result = await function.InvokeAsync(FormulaType.Boolean, new FormulaValue[] { arg }, CancellationToken.None); + var result = await function.InvokeAsync(FunctionInvokeInfo.New(FormulaType.Boolean, arg), CancellationToken.None); if (arg is ErrorValue) { diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/EvalVisitorTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/EvalVisitorTests.cs new file mode 100644 index 0000000000..38deb077a6 --- /dev/null +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/EvalVisitorTests.cs @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +using System.Text; +using Xunit; + +namespace Microsoft.PowerFx.Interpreter.Tests +{ + // This file should eventually get 100% code-coverage on EvalVistor. + // Exercise via RecalcEngine + public class EvalVistitorTests + { + [Theory] + [InlineData( + "Join([10,20],[1,2], LeftRecord.Value = RightRecord.Value*10, JoinType.Left, RightRecord.Value As R2)", "Table({R2:1,Value:10},{R2:2,Value:20})")] + [InlineData( + "Filter([10,30,20], ThisRecord.Value > 15)", + "Table({Value:30},{Value:20})")] + public void Dispatch(string expr, string expected) + { + string answer = Run(expr); + + Assert.Equal(expected, answer); + } + + private static readonly RecalcEngine _engine = NewEngine(); + + private static RecalcEngine NewEngine() + { + var config = new PowerFxConfig(); +#pragma warning disable CS0618 // Type or member is obsolete + config.EnableJoinFunction(); +#pragma warning restore CS0618 // Type or member is obsolete + var engine = new RecalcEngine(config); + + return engine; + } + + private static string Run(string expr) + { + var result = _engine.Eval(expr); + + var sb = new StringBuilder(); + result.ToExpression(sb, new FormulaValueSerializerSettings { UseCompactRepresentation = true }); + + var answer = sb.ToString(); + return answer; + } + } +} diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PatchFunctionTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PatchFunctionTests.cs index 7aa55c7492..0054beb15f 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PatchFunctionTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/PatchFunctionTests.cs @@ -5,8 +5,10 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.PowerFx.Core.Functions; +using Microsoft.PowerFx.Core.IR; using Microsoft.PowerFx.Core.Tests; using Microsoft.PowerFx.Core.Texl.Builtins; +using Microsoft.PowerFx.Functions; using Microsoft.PowerFx.Types; using Xunit; @@ -36,8 +38,14 @@ public async Task CheckArgsTestAsync(Type type) innerServices.AddService(Features.PowerFxV1); - var function = Activator.CreateInstance(type) as IAsyncTexlFunction3; - var result = await function.InvokeAsync(FormulaType.Unknown, args, CancellationToken.None); + var function = Activator.CreateInstance(type) as IFunctionInvoker; + var invokeInfo = new FunctionInvokeInfo + { + Args = args, + IRContext = IRContext.NotInSource(FormulaType.Unknown) + }; + + var result = await function.InvokeAsync(invokeInfo, CancellationToken.None); Assert.IsType(result); } diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index 5efafa90d1..ab6a942972 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -61,7 +61,7 @@ public void PublicSurfaceTests() $"{ns}.{nameof(TableMarshallerProvider)}", $"{ns}.{nameof(TypeMarshallerCache)}", $"{ns}.{nameof(TypeMarshallerCacheExtensions)}", - $"{ns}.{nameof(SymbolExtensions)}", + $"{ns}.{nameof(SymbolExtensions)}", $"{nsType}.{nameof(ObjectRecordValue)}", #pragma warning disable CS0618 // Type or member is obsolete $"{nsType}.{nameof(QueryableTableValue)}", @@ -71,7 +71,9 @@ public void PublicSurfaceTests() $"{ns}.Interpreter.{nameof(CustomFunctionErrorException)}", $"{ns}.{nameof(TypeCoercionProvider)}", - // Services for functions. + // Services for functions. + "Microsoft.PowerFx.Functions.IFunctionInvoker", + "Microsoft.PowerFx.Functions.FunctionInvokeInfo", $"{ns}.Functions.IRandomService", $"{ns}.Functions.IClockService" }; @@ -1216,8 +1218,10 @@ public TestFunctionMultiply() yield return new[] { TexlStrings.IsBlankArg1 }; } - public override Task InvokeAsync(IServiceProvider serviceProvider, FormulaValue[] args, CancellationToken cancellationToken) - { + public override Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) + { + var args = invokeInfo.Args; + var arg0 = args[0] as NumberValue; var arg1 = args[1] as StringValue; @@ -1239,8 +1243,10 @@ public TestFunctionSubstract() yield return new[] { TexlStrings.IsBlankArg1 }; } - public override Task InvokeAsync(IServiceProvider serviceProvider, FormulaValue[] args, CancellationToken cancellationToken) - { + public override Task InvokeAsync(FunctionInvokeInfo invokeInfo, CancellationToken cancellationToken) + { + var args = invokeInfo.Args; + var arg0 = args[0] as StringValue; var arg1 = args[1] as NumberValue;