Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix delegation check on UDF for datasource type #2816

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Fix delegation check on UDF for datasource type #2816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9b10b0b
Fix custom type declaration on UDF for datasource type
yuchenglin Jan 16, 2025
3d347e6
Option 2: fix with interface implementation
yuchenglin Jan 22, 2025
03dc32e
Revert "Option 2: fix with interface implementation"
yuchenglin Jan 22, 2025
19bf60f
Revert "Fix custom type declaration on UDF for datasource type"
yuchenglin Jan 22, 2025
6d3adac
Option 1 with specific scope
yuchenglin Jan 22, 2025
fbb0a07
address comment: remove delegatable interface to align with other fun…
yuchenglin Jan 24, 2025
f865a25
Merge branch 'main' of https://github.com/microsoft/Power-Fx into yuc…
yuchenglin Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/libraries/Microsoft.PowerFx.Core/Functions/UserDefinedFunction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace Microsoft.PowerFx.Core.Functions
/// This includings the binding (and hence IR for evaluation) -
/// This is conceptually immutable after initialization - if the body or signature changes, you need to create a new instance.
/// </summary>
internal class UserDefinedFunction : TexlFunction, IExternalPageableSymbol, IExternalDelegatableSymbol
internal class UserDefinedFunction : TexlFunction, IExternalPageableSymbol
{
private readonly bool _isImperative;
private readonly IEnumerable<UDFArg> _args;
Expand All @@ -44,15 +44,23 @@ internal class UserDefinedFunction : TexlFunction, IExternalPageableSymbol, IExt

public bool IsPageable => _binding.IsPageable(_binding.Top);

public bool IsDelegatable => _binding.IsDelegatable(_binding.Top);

public override bool IsServerDelegatable(CallNode callNode, TexlBinding binding)
{
Contracts.AssertValue(callNode);
Contracts.AssertValue(binding);
Contracts.Assert(binding.GetInfo(callNode).Function is UserDefinedFunction udf && udf.Binding != null);

return base.IsServerDelegatable(callNode, binding) || IsDelegatable;
if (base.IsServerDelegatable(callNode, binding) || _binding.IsDelegatable(_binding.Top))
{
return true;
}

if (_binding.Top.Kind == NodeKind.FirstName && ArgValidators.DelegatableDataSourceInfoValidator.TryGetValidValue(_binding.Top, _binding, out _))
{
return true;
}
yuchenglin marked this conversation as resolved.
Show resolved Hide resolved

return false;
}

public override bool SupportsParamCoercion => true;
Expand Down
8 changes: 4 additions & 4 deletions src/tests/Microsoft.PowerFx.Interpreter.Tests.Shared/RecalcEngineTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,23 +799,23 @@ public void DelegableUDFTest()
var func = recalcEngine.Functions.WithName("A").First() as UserDefinedFunction;

Assert.True(func.IsAsync);
Assert.True(func.IsDelegatable);
Assert.True(func.Binding.IsDelegatable(func.Binding.Top));

func = recalcEngine.Functions.WithName("C").First() as UserDefinedFunction;

Assert.True(func.IsAsync);
Assert.True(func.IsDelegatable);
Assert.True(func.Binding.IsDelegatable(func.Binding.Top));

func = recalcEngine.Functions.WithName("B").First() as UserDefinedFunction;

Assert.True(func.IsAsync);
Assert.True(func.IsDelegatable);
Assert.True(func.Binding.IsDelegatable(func.Binding.Top));

func = recalcEngine.Functions.WithName("D").First() as UserDefinedFunction;

// Imperative function is not delegable
Assert.True(func.IsAsync);
Assert.True(!func.IsDelegatable);
Assert.True(!func.Binding.IsDelegatable(func.Binding.Top));

// 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);
Expand Down