From b89656de2e08c4c6cddcc2a19db1ca07a371859b Mon Sep 17 00:00:00 2001 From: rick-nguyen <97256426+rick-nguyen@users.noreply.github.com> Date: Mon, 14 Oct 2024 08:19:43 -0700 Subject: [PATCH] NF/UDF with Delegation warning is not passed on when NF/UDF is used (#2692) 1. UDF():DS = Filter(DS, Sqrt(Value) > 5)); 2. use UDF() elsewhere, no delegation warning is shown This has always been the case for named formulas as well, change for NF delegation warning flag is in other repo so test cannot be written to for that scenario. --- .../Microsoft.PowerFx.Core/Binding/Binder.cs | 14 ++++- .../External/IExternalNamedFormula.cs | 4 +- .../Functions/UserDefinedFunction.cs | 2 + .../Localization/Strings.cs | 2 + .../Public/DefinitionsCheckResult.cs | 2 +- src/strings/PowerFxResources.en-US.resx | 54 +++++++++++++++++++ .../RecalcEngineTests.cs | 37 +++++++++++++ 7 files changed, 112 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs b/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs index 680f45a98..30b8770ed 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Binding/Binder.cs @@ -2940,7 +2940,13 @@ namespace Microsoft.PowerFx.Core.Binding _txb.SetSetMutable(node, nameSymbol?.Props.CanSetMutate ?? false); if (lookupInfo.Data is IExternalNamedFormula formula) { - isConstantNamedFormula = formula.IsConstant; + isConstantNamedFormula = formula.IsConstant; + + // If the definition of the named formula has a delegation warning, every use should also inherit this warning + if (formula.HasDelegationWarning) + { + _txb.ErrorContainer.EnsureError(DocumentErrorSeverity.Warning, node, TexlStrings.SuggestRemoteExecutionHint_NF, node.Ident.Name); + } } } else if (lookupInfo.Kind == BindKind.Data) @@ -4871,6 +4877,12 @@ namespace Microsoft.PowerFx.Core.Binding { _txb.ErrorContainer.EnsureError(node, errorKey, badAncestor.Head.Name); } + } + + // If the definition of the user-defined function has a delegation warning, every usage should also inherit this warning + if (func is UserDefinedFunction udf && udf.HasDelegationWarning) + { + _txb.ErrorContainer.EnsureError(DocumentErrorSeverity.Warning, node, TexlStrings.SuggestRemoteExecutionHint_UDF, udf.Name); } _txb.CheckAndMarkAsDelegatable(node); diff --git a/src/libraries/Microsoft.PowerFx.Core/Entities/External/IExternalNamedFormula.cs b/src/libraries/Microsoft.PowerFx.Core/Entities/External/IExternalNamedFormula.cs index a35cf95eb..880b0f44e 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Entities/External/IExternalNamedFormula.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Entities/External/IExternalNamedFormula.cs @@ -13,6 +13,8 @@ namespace Microsoft.PowerFx.Core.Entities bool IsConstant { get; } - bool ContainsReferenceToView { get; } + bool ContainsReferenceToView { get; } + + bool HasDelegationWarning { get; } } } diff --git a/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs b/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs index 662bf0eeb..f86086b4e 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs @@ -75,6 +75,8 @@ namespace Microsoft.PowerFx.Core.Functions return TryGetExternalDataSource(out dsInfo); } + public bool HasDelegationWarning => _binding?.ErrorContainer.GetErrors().Any(error => error.MessageKey.Contains("SuggestRemoteExecutionHint")) ?? false; + /// /// Initializes a new instance of the class. /// diff --git a/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs b/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs index 66ed74e2d..4b399d644 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs @@ -758,6 +758,8 @@ namespace Microsoft.PowerFx.Core.Localization public static ErrorResourceKey SuggestRemoteExecutionHint_InOpRhs = new ErrorResourceKey("SuggestRemoteExecutionHint_InOpRhs"); public static ErrorResourceKey SuggestRemoteExecutionHint_StringMatchSecondParam = new ErrorResourceKey("SuggestRemoteExecutionHint_StringMatchSecondParam"); public static ErrorResourceKey SuggestRemoteExecutionHint_InOpInvalidColumn = new ErrorResourceKey("SuggestRemoteExecutionHint_InOpInvalidColumn"); + public static ErrorResourceKey SuggestRemoteExecutionHint_NF = new ErrorResourceKey("SuggestRemoteExecutionHint_NF"); + public static ErrorResourceKey SuggestRemoteExecutionHint_UDF = new ErrorResourceKey("SuggestRemoteExecutionHint_UDF"); public static ErrorResourceKey OpNotSupportedByColumnSuggestionMessage_OpNotSupportedByColumn = new ErrorResourceKey("SuggestRemoteExecutionHint_OpNotSupportedByColumn"); public static ErrorResourceKey OpNotSupportedByServiceSuggestionMessage_OpNotSupportedByService = new ErrorResourceKey("SuggestRemoteExecutionHint_OpNotSupportedByService"); public static ErrorResourceKey OpNotSupportedByClientSuggestionMessage_OpNotSupportedByClient = new ErrorResourceKey("SuggestRemoteExecutionHint_OpNotSupportedByClient"); diff --git a/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs b/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs index 51cc3a5ad..7b708f3c3 100644 --- a/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs +++ b/src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs @@ -173,7 +173,7 @@ namespace Microsoft.PowerFx List bindErrors = new List(); - if (binding.ErrorContainer.HasErrors()) + if (binding.ErrorContainer.GetErrors().Any(error => error.Severity > DocumentErrorSeverity.Warning)) { _errors.AddRange(ExpressionError.New(binding.ErrorContainer.GetErrors(), _defaultErrorCulture)); } diff --git a/src/strings/PowerFxResources.en-US.resx b/src/strings/PowerFxResources.en-US.resx index d51308d23..a745ad5ab 100644 --- a/src/strings/PowerFxResources.en-US.resx +++ b/src/strings/PowerFxResources.en-US.resx @@ -4026,6 +4026,60 @@ https://go.microsoft.com/fwlink/?linkid=2132702 {Locked} + + + Delegation warning. The named formula {0} is not delegable so its usage in this formula might not work correctly on large data sets. + Error Message. + + + The data source might not be able to process the formula and might return an incomplete data set. Your application might not return correct results or behave correctly if the data set is incomplete. + + + Try simplifying the definition for the named formula. + 1 How to fix the error. + + + Article: Understand delegation in a canvas app + Article on delegation + + + https://go.microsoft.com/fwlink/?linkid=2132701 + {Locked} + + + Blog: Data row limits for delegation + Blog: Data row limits for delegation + + + https://go.microsoft.com/fwlink/?linkid=2132702 + {Locked} + + + Delegation warning. The user-defined function {0} is not delegable so its usage in this formula might not work correctly on large data sets. + Error Message. + + + The data source might not be able to process the formula and might return an incomplete data set. Your application might not return correct results or behave correctly if the data set is incomplete. + + + Try simplifying the definition for the user-defined function. + 1 How to fix the error. + + + Article: Understand delegation in a canvas app + Article on delegation + + + https://go.microsoft.com/fwlink/?linkid=2132701 + {Locked} + + + Blog: Data row limits for delegation + Blog: Data row limits for delegation + + + https://go.microsoft.com/fwlink/?linkid=2132702 + {Locked} Delegation warning. The highlighted part of this formula might not work correctly on large data sets. The "{0}" operation is not supported by this connector. diff --git a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs index 831d36f68..9f9a3b16b 100644 --- a/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs +++ b/src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs @@ -798,6 +798,43 @@ namespace Microsoft.PowerFx.Tests // Binding fails for recursive definitions and hence function is not added. Assert.False(recalcEngine.AddUserDefinedFunction("E():Void = { E(); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true).IsSuccess); + } + + [Fact] + public void TestInheritanceOfDelegationWarningsInUDFs() + { + var symbolTable = new DelegatableSymbolTable(); + var schema = DType.CreateTable( + new TypedName(DType.Number, new DName("Value"))); + symbolTable.AddEntity(new TestDelegableDataSource( + "MyDataSource", + schema, + new TestDelegationMetadata( + new DelegationCapability(DelegationCapability.Filter), + schema, + new FilterOpMetadata( + schema, + new Dictionary(), + new Dictionary(), + new DelegationCapability(DelegationCapability.GreaterThan), + null)), + true)); + symbolTable.AddType(new DName("MyDataSourceTableType"), FormulaType.Build(schema)); + var config = new PowerFxConfig() + { + SymbolTable = symbolTable + }; + var engine = new RecalcEngine(config); + + var result = engine.AddUserDefinedFunction("NonDelegatableUDF():MyDataSourceTableType = Filter(MyDataSource, Sqrt(Value) > 5);", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); + Assert.True(result.IsSuccess); + var func = engine.Functions.WithName("NonDelegatableUDF").First() as UserDefinedFunction; + Assert.True(func.HasDelegationWarning); + + result = engine.AddUserDefinedFunction("NonDelegatableUDF2():MyDataSourceTableType = NonDelegatableUDF();", CultureInfo.InvariantCulture, symbolTable: engine.EngineSymbols, allowSideEffects: true); + Assert.True(result.IsSuccess); + func = engine.Functions.WithName("NonDelegatableUDF2").First() as UserDefinedFunction; + Assert.True(func.HasDelegationWarning); } // Binding to inner functions does not impact outer functions.