Skip to content

Commit

Permalink
add tests, improve error message , impl
Browse files Browse the repository at this point in the history
  • Loading branch information
adithyaselv committed Mar 6, 2024
1 parent 3e3baa5 commit ecec364
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 60 deletions.
3 changes: 3 additions & 0 deletions src/libraries/Microsoft.PowerFx.Core/Localization/Strings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,7 @@ internal static class TexlStrings
public static ErrorResourceKey ErrInvalidSchemaNeedTypeCol_Col = new ErrorResourceKey("ErrInvalidSchemaNeedTypeCol_Col");
public static ErrorResourceKey ErrInvalidSchemaNeedCol = new ErrorResourceKey("ErrInvalidSchemaNeedCol");
public static ErrorResourceKey ErrNeedRecord = new ErrorResourceKey("ErrNeedRecord");
public static ErrorResourceKey ErrNeedRecordOrTable = new ErrorResourceKey("ErrNeedRecordOrTable");
public static ErrorResourceKey ErrAutoRefreshNotAllowed = new ErrorResourceKey("ErrAutoRefreshNotAllowed");
public static ErrorResourceKey ErrIncompatibleRecord = new ErrorResourceKey("ErrIncompatibleRecord");
public static ErrorResourceKey ErrNeedRecord_Func = new ErrorResourceKey("ErrNeedRecord_Func");
Expand Down Expand Up @@ -788,5 +789,7 @@ internal static class TexlStrings
public static ErrorResourceKey ErrOnlyPartialAttribute = new ErrorResourceKey("ErrOnlyPartialAttribute");
public static ErrorResourceKey ErrOperationDoesntMatch = new ErrorResourceKey("ErrOperationDoesntMatch");
public static ErrorResourceKey ErrUnknownPartialOp = new ErrorResourceKey("ErrUnknownPartialOp");

public static ErrorResourceKey ErrTruncatedArgWarning = new ErrorResourceKey("ErrTruncatedArgWarning");
}
}
8 changes: 4 additions & 4 deletions src/libraries/Microsoft.PowerFx.Core/Texl/Builtins/Table.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

namespace Microsoft.PowerFx.Core.Texl.Builtins
{
// Table(rec, rec, ...)
// Table(rec/table, rec/table, ...)
internal class TableFunction : BuiltinFunction
{
public override bool IsSelfContained => true;
Expand Down Expand Up @@ -62,12 +62,12 @@ public override bool CheckTypes(CheckTypesContext context, TexlNode[] args, DTyp
for (var i = 0; i < argTypes.Length; i++)
{
var argType = argTypes[i];
var argTypeRecord = argType.IsTable ? argType.ToRecord() : argType;
var argTypeRecord = argType.IsTableNonObjNull ? argType.ToRecord() : argType;
var isChildTypeAllowedInTable = !argType.IsDeferred && !argType.IsVoid;

if (!argTypeRecord.IsRecord)
{
errors.EnsureError(DocumentErrorSeverity.Severe, args[i], TexlStrings.ErrNeedRecord);
errors.EnsureError(DocumentErrorSeverity.Severe, args[i], TexlStrings.ErrNeedRecordOrTable);
isValid = false;
}
else if (!isChildTypeAllowedInTable)
Expand Down Expand Up @@ -118,7 +118,7 @@ public override void CheckSemantics(TexlBinding binding, TexlNode[] args, DType[

if (ads is IExternalDataSource tDsInfo && tDsInfo is IExternalTabularDataSource)
{
errors.EnsureError(DocumentErrorSeverity.Warning, args[i], TexlStrings.SuggestRemoteExecutionHint, args[i].ToString());
errors.EnsureError(DocumentErrorSeverity.Warning, args[i], TexlStrings.ErrTruncatedArgWarning, args[i].ToString(), Name);
continue;
}
}
Expand Down
36 changes: 25 additions & 11 deletions src/libraries/Microsoft.PowerFx.Interpreter/Functions/Library.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2125,20 +2125,34 @@ public static IEnumerable<DValue<RecordValue>> StandardTableNodeRecords(IRContex

public static FormulaValue Table(IRContext irContext, FormulaValue[] args)
{
// Table literal - change to for loop
var tables = Array.ConvertAll(
args,
arg => arg switch
// Table literal
var table = new List<DValue<RecordValue>>();

for (var i = 0; i < args.Length; i++)
{
switch (args[i])
{
TableValue r => r.Rows,
RecordValue r => new List<DValue<RecordValue>> { DValue<RecordValue>.Of(r) },
BlankValue b when b.Type._type.IsRecord => new List<DValue<RecordValue>> { DValue<RecordValue>.Of(b) },
BlankValue b => new List<DValue<RecordValue>>(),
_ => new List<DValue<RecordValue>> { DValue<RecordValue>.Of((ErrorValue)arg) },
});
case TableValue t:
table.AddRange(t.Rows);
break;
case RecordValue r:
table.Add(DValue<RecordValue>.Of(r));
break;
case BlankValue b when b.Type._type.IsTableNonObjNull:
break;
case BlankValue b:
table.Add(DValue<RecordValue>.Of(b));
break;
case ErrorValue e when e.Type._type.IsTableNonObjNull:
return e;
default:
table.Add(DValue<RecordValue>.Of((ErrorValue)args[i]));
break;
}
}

// Returning List to ensure that the returned table is mutable
return new InMemoryTableValue(irContext, tables.SelectMany(x => x));
return new InMemoryTableValue(irContext, table);
}

public static ValueTask<FormulaValue> Blank(EvalVisitor runner, EvalVisitorContext context, IRContext irContext, FormulaValue[] args)
Expand Down
18 changes: 13 additions & 5 deletions src/strings/PowerFxResources.en-US.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1962,6 +1962,10 @@
<value>Cannot use a non-record value in this context.</value>
<comment>Error Message.</comment>
</data>
<data name="ErrNeedRecordOrTable" xml:space="preserve">
<value>In this context, only record or table values can be used.</value>
<comment>Error Message. If a record or table is expected.</comment>
</data>
<data name="ErrIncompatibleRecord" xml:space="preserve">
<value>Cannot use this record. It may contain colliding fields of incompatible types.</value>
<comment>Error Message.</comment>
Expand Down Expand Up @@ -2883,15 +2887,15 @@
<value>Language code of the supplied text.</value>
</data>
<data name="AboutTable" xml:space="preserve">
<value>Creates a table from the specified records, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)</value>
<value>Creates a table from the specified records and tables, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)</value>
<comment>Description of 'Table' function.</comment>
</data>
<data name="TableArg1" xml:space="preserve">
<value>record</value>
<comment>function_parameter - Argument of the Table function - a record that will become a row in the resulting table.</comment>
<value>record/table</value>
<comment>function_parameter - Argument of the Table function - a record/table that will be part of the the resulting table.</comment>
</data>
<data name="AboutTable_record" xml:space="preserve">
<value>A record that will become a row in the resulting table.</value>
<data name="AboutTable_record/table" xml:space="preserve">
<value>A record/table that will be part of the the resulting table.</value>
</data>
<data name="AboutShowColumns" xml:space="preserve">
<value>Returns a table with all columns removed from the 'source' table except the specified columns.</value>
Expand Down Expand Up @@ -4508,4 +4512,8 @@
A partial operator is the 2nd part of a statement `[Partial Op]` and can be one of "And", "Or", "Table" or "Record".
It's used to determine how to combine multiple expressions with the same name and operator.</comment>
</data>
<data name="ErrTruncatedArgWarning" xml:space="preserve">
<value>Delegation warning. The result of this argument '{0}' may be truncated for large data sets before being passed to the '{1}' function.</value>
<comment>Error message when an argument to non-delegable function has possible delegation and resulting rows may be truncated</comment>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,17 @@ Table({a:0,b:Blank(),c:Blank()},{a:Blank(),b:true,c:Blank()},{a:Blank(),b:Blank(
>> Table([{a:0}], [{b:true}], [{c:"Hello", d: {x: "World"}}])
Table({a:0,b:Blank(),c:Blank(),d:Blank()},{a:Blank(),b:true,c:Blank(),d:Blank()},{a:Blank(),b:Blank(),c:"Hello",d:{x:"World"}})

// Typed blank should be treated as blank table
>> CountRows(Table(If(1<0,[1,2,3],Blank())))
0

// untyped blank should be treated as blank record
>> CountRows(Table([{a:0, b:"hello"}], Blank()))
2

>> CountRows(Table(Sequence(3000)))
3000

>> If(CountRows(Table([{a:0, b:"hello"}], Blank())) = 2, "As Expected")
"As Expected"

// Mixing - record and table
>> Table({c:"Hello", d: {x: "World"}}, [{c:"PowerFx", d: {x: "Cool"}}])
Table({c:"Hello",d:{x:"World"}},{c:"PowerFx",d:{x:"Cool"}})
Expand All @@ -36,28 +38,56 @@ Errors: Error 0-63: The function 'Table' has some invalid arguments.|Error 28-62
>> Table([1, 2], If(1<0, Table({Value:{a:2}})))
Errors: Error 0-44: The function 'Table' has some invalid arguments.|Error 14-43: Incompatible type. The item you are trying to put into a table has a type that is not compatible with the table.

// No arg
>> Table()
Table()

// Single argument with table
>> Table([{a:0, b:false, c:"Hello"}])
Table({a:0,b:false,c:"Hello"})

// Single argument with record
>> Table({a:0, b:false, c:"Hello"})
Table({a:0,b:false,c:"Hello"})

// Blank inputs
>> Table(Blank(), Blank())
Table(Blank(),Blank())

>> Table(If(1<0,Blank()))
Table(Blank())

>> Table([1, 2], Blank(), [4, 5], Blank(), [7, 8])
Table({Value:1},{Value:2},Blank(),{Value:4},{Value:5},Blank(),{Value:7},{Value:8})

// Runtime Error inputs
>> Table([1,2], If(1/0<2,[3,4]), [5,6])
Table({Value:1},{Value:2},Error({Kind:ErrorKind.Div0}),{Value:5},{Value:6})

// Tables containing runtime errors
>> Table([1, 2, 3/0, 4], [5, Sqrt(-1), 7, 8])
Table({Value:1},{Value:2},{Value:Error({Kind:ErrorKind.Div0})},{Value:4},{Value:5},{Value:Error({Kind:ErrorKind.Numeric})},{Value:7},{Value:8})

>> Table(Filter([2,1,0,-1,-2], 1/Value>0), Filter([-2,-1,0,1,2], Log(Value)>0))
Table({Value:2},{Value:1},Error({Kind:ErrorKind.Div0}),Error({Kind:ErrorKind.Numeric}),Error({Kind:ErrorKind.Numeric}),Error({Kind:ErrorKind.Numeric}),{Value:2})

// coercion failures
>> Table([42],["everything"])
Table({Value:42},{Value:Error({Kind:ErrorKind.InvalidArgument})})

>> Table(["everything"], [42])
Table({Value:"everything"},{Value:"42"})
Table({Value:"everything"},{Value:"42"})

// Error function has type ObjNull, which can is both a record and a table; we treat it as a record
>> Table([{a:1}], Error({Kind:ErrorKind.Div0, Message:"Please don't divide by zero"}), {a:3}, [{a:4}])
Table({a:1},Error({Kind:ErrorKind.Div0}),{a:3},{a:4})

>> Table([{a:1}], 1/0, {a:3}, [{a:4}])
Errors: Error 0-35: The function 'Table' has some invalid arguments.|Error 16-17: In this context, only record or table values can be used.

// The second argument is a record, no ambiguity
>> Table([{a:1}], If(1/0<2,{a:2}), {a:3}, [{a:4}])
Table({a:1},Error({Kind:ErrorKind.Div0}),{a:3},{a:4})

// The second argument is a table. With an error table is passed in, the result is an error
>> Table([{a:1}], If(1/0<2,[{a:2}]), {a:3}, [{a:4}])
Error({Kind:ErrorKind.Div0})

>> Table([{a:1}], {a:Error({Kind:ErrorKind.Custom})}, {a:3}, [{a:4}])
Table({a:1},{a:Error({Kind:ErrorKind.Custom})},{a:3},{a:4})
Original file line number Diff line number Diff line change
@@ -1,54 +1,54 @@
{
"Signatures": [
{
"Label": "Table(record, ...)",
"Documentation": "Creates a table from the specified records, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)",
"Label": "Table(record/table, ...)",
"Documentation": "Creates a table from the specified records and tables, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)",
"Parameters": [
{
"Label": "record",
"Documentation": "A record that will become a row in the resulting table."
"Label": "record/table",
"Documentation": "A record/table that will be part of the the resulting table."
}
]
},
{
"Label": "Table(record, record, ...)",
"Documentation": "Creates a table from the specified records, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)",
"Label": "Table(record/table, record/table, ...)",
"Documentation": "Creates a table from the specified records and tables, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)",
"Parameters": [
{
"Label": "record",
"Documentation": "A record that will become a row in the resulting table."
"Label": "record/table",
"Documentation": "A record/table that will be part of the the resulting table."
},
{
"Label": "record",
"Documentation": "A record that will become a row in the resulting table."
"Label": "record/table",
"Documentation": "A record/table that will be part of the the resulting table."
}
]
},
{
"Label": "Table(record, record, record, ...)",
"Documentation": "Creates a table from the specified records, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)",
"Label": "Table(record/table, record/table, record/table, ...)",
"Documentation": "Creates a table from the specified records and tables, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)",
"Parameters": [
{
"Label": "record",
"Documentation": "A record that will become a row in the resulting table."
"Label": "record/table",
"Documentation": "A record/table that will be part of the the resulting table."
},
{
"Label": "record",
"Documentation": "A record that will become a row in the resulting table."
"Label": "record/table",
"Documentation": "A record/table that will be part of the the resulting table."
},
{
"Label": "record",
"Documentation": "A record that will become a row in the resulting table."
"Label": "record/table",
"Documentation": "A record/table that will be part of the the resulting table."
}
]
},
{
"Label": "Table(record)",
"Documentation": "Creates a table from the specified records, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)",
"Label": "Table(record/table)",
"Documentation": "Creates a table from the specified records and tables, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)",
"Parameters": [
{
"Label": "record",
"Documentation": "A record that will become a row in the resulting table."
"Label": "record/table",
"Documentation": "A record/table that will be part of the the resulting table."
}
]
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
{
"Signatures": [
{
"Label": "Table(record, record, record, ...)",
"Documentation": "Creates a table from the specified records, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)",
"Label": "Table(record/table, record/table, record/table, ...)",
"Documentation": "Creates a table from the specified records and tables, with as many columns as there are unique record fields. For example: Table({key1: val1, key2: val2, ...}, ...)",
"Parameters": [
{
"Label": "record",
"Documentation": "A record that will become a row in the resulting table."
"Label": "record/table",
"Documentation": "A record/table that will be part of the the resulting table."
},
{
"Label": "record",
"Documentation": "A record that will become a row in the resulting table."
"Label": "record/table",
"Documentation": "A record/table that will be part of the the resulting table."
},
{
"Label": "record",
"Documentation": "A record that will become a row in the resulting table."
"Label": "record/table",
"Documentation": "A record/table that will be part of the the resulting table."
}
]
}
Expand Down
7 changes: 5 additions & 2 deletions src/tests/Microsoft.PowerFx.Core.Tests/TexlTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4144,7 +4144,9 @@ public void TestBlobFunction(string expression, string expectedType)
[InlineData("Table(Table(T1, T2), T3)", "*[a:n, b:n, c:n, d:n]")]
[InlineData("Table(T1, Table(T2, T4))", "*[a:n, b:n, c:n]")]
[InlineData("Table(Table(T1, T2), T4)", "*[a:n, b:n, c:n]")]
[InlineData("Table([1,2], If(1/0<2,[3,4]), [5,6])", "*[Value: n]")]
[InlineData("Table([1,2], If(1/0<2,[3,4]), [5,6])", "*[Value: n]")]
[InlineData("Table(Sequence(20000))", "*[Value: n]")]
[InlineData("Table(Filter(T1, b = 5))", "*[a:n, b:n, c:n]")]
public void TexlFunctionTypeSemanticsTableConcatenate(string script, string expectedType)
{
var symbol = new SymbolTable();
Expand All @@ -4162,7 +4164,8 @@ public void TexlFunctionTypeSemanticsTableConcatenate(string script, string expe

[Theory]
[InlineData("Table(T1, T2)", "*[V: n]")]
[InlineData("Table([{a:Date(2024,1,1)}], [{a:GUID(\"some-guid-value-1234\")}])", "*[a: D]")]
[InlineData("Table([{a:Date(2024,1,1)}], [{a:GUID(\"some-guid-value-1234\")}])", "*[a: D]")]
[InlineData("Table([{a:1}], 1/0, {a:3}, [{a:4}])", "*[a: n]")]
public void TexlFunctionTypeSemanticsTableConcatenate_Negative(string script, string expectedType)
{
var symbol = new SymbolTable();
Expand Down

0 comments on commit ecec364

Please sign in to comment.