Skip to content

Commit

Permalink
Fix UDFs not properly handled in TryGetDataSource recursion (#2615)
Browse files Browse the repository at this point in the history
`FilterUDF():DataSource = Filter(DataSource, Value > 10);
FilterUDF2():DataSource = Filter(FilterUDF(), Value > 11);`

`TryGetDsInfo(Callnode)` would not get a datasource for `FilterUDF()`,
leading to Binder not marking this node async. This led to an issue of
delegation not being preserved on `FilterUDF2()`.
  • Loading branch information
rick-nguyen authored Aug 29, 2024
1 parent 0089d2f commit a82afed
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ internal class UserDefinedFunction : TexlFunction, IExternalPageableSymbol, IExt
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);
{
Contracts.AssertValue(callNode);
Contracts.AssertValue(binding);
Contracts.Assert(binding.GetInfo(callNode).Function is UserDefinedFunction udf && udf.Binding != null);

return base.IsServerDelegatable(callNode, binding) || IsDelegatable;
}
Expand All @@ -70,6 +70,11 @@ public bool TryGetExternalDataSource(out IExternalDataSource dataSource)
return ArgValidators.DelegatableDataSourceInfoValidator.TryGetValidValue(_binding.Top, _binding, out dataSource);
}

public override bool TryGetDataSource(CallNode callNode, TexlBinding binding, out IExternalDataSource dsInfo)
{
return TryGetExternalDataSource(out dsInfo);
}

/// <summary>
/// Initializes a new instance of the <see cref="UserDefinedFunction"/> class.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
using System.Threading.Tasks;
using Microsoft.PowerFx.Core;
using Microsoft.PowerFx.Core.Functions;
using Microsoft.PowerFx.Core.Functions.Delegation;
using Microsoft.PowerFx.Core.Functions.Delegation.DelegationMetadata;
using Microsoft.PowerFx.Core.IR;
using Microsoft.PowerFx.Core.Localization;
using Microsoft.PowerFx.Core.Tests;
Expand Down Expand Up @@ -679,34 +681,54 @@ public void ImperativeUserDefinedFunctionTest(string udfExpression, string expre
[Fact]

public void DelegatableUDFTest()
{
var config = new PowerFxConfig();
config.EnableSetFunction();

{
var symbolTable = new DelegatableSymbolTable();
var schema = DType.CreateTable(
new TypedName(DType.Guid, new DName("ID")),
new TypedName(DType.Number, new DName("Value")));
config.SymbolTable.AddEntity(new TestDataSource("MyDataSource", schema));
config.SymbolTable.AddType(new DName("MyDataSourceTableType"), FormulaType.Build(schema));
symbolTable.AddEntity(new TestDelegableDataSource(
"MyDataSource",
schema,
new TestDelegationMetadata(
new DelegationCapability(DelegationCapability.Filter),
schema,
new FilterOpMetadata(
schema,
new Dictionary<DPath, DelegationCapability>(),
new Dictionary<DPath, DelegationCapability>(),
new DelegationCapability(DelegationCapability.GreaterThan),
null)),
true));
symbolTable.AddType(new DName("MyDataSourceTableType"), FormulaType.Build(schema));
var config = new PowerFxConfig()
{
SymbolTable = symbolTable
};
config.EnableSetFunction();

var recalcEngine = new RecalcEngine(config);

recalcEngine.AddUserDefinedFunction("A():MyDataSourceTableType = Filter(Sort(MyDataSource,Value), Value > 10);C():MyDataSourceTableType = A();", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true);
var func = recalcEngine.Functions.WithName("A");
recalcEngine.AddUserDefinedFunction("A():MyDataSourceTableType = Filter(MyDataSource, Value > 10);C():MyDataSourceTableType = A(); B():MyDataSourceTableType = Filter(C(), Value > 11); D():MyDataSourceTableType = { Filter(B(), Value > 12); };", CultureInfo.InvariantCulture, symbolTable: recalcEngine.EngineSymbols, allowSideEffects: true);
var func = recalcEngine.Functions.WithName("A").First() as UserDefinedFunction;

if (func is UserDefinedFunction udf)
{
Assert.True(udf.IsAsync);
Assert.True(udf.IsDelegatable);
}
Assert.True(func.IsAsync);
Assert.True(func.IsDelegatable);

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

if (func is UserDefinedFunction udf2)
{
Assert.True(udf2.IsAsync);
Assert.True(udf2.IsDelegatable);
}
Assert.True(func.IsAsync);
Assert.True(func.IsDelegatable);

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

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

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

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

// Binding to inner functions does not impact outer functions.
Expand Down

0 comments on commit a82afed

Please sign in to comment.