microsoft / microsoft/Power-Fx
SortByColumns is confused by missing fields from UDF
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 3.4k
- Forks
- 358
- Avg merge
- 10h 34m
- Merged PRs (30d)
- 3
Description
Note the runtime error when the Age column is omitted.
>> Person := Type( { Name: Text, Age: Number } );
>> People := Type( [ Person ] );
>> SortedPeople(list:People):People = SortByColumns(list, "Name", SortOrder.Ascending, "Age", SortOrder.Ascending);
>> SortedPeople([{Name:"John", Age:30},{Name:"Jane", Age:40}])
Age Name
===== ======
40 Jane
30 John
>> SortedPeople([{Name:"John"},{Name:"Jane"}]) // BUG!
<Error: The specified column 'Age' does not exist or is an invalid sort column type.>
>> SortedPeople([{Name:1},{Name:2}])
Age Name
===== ======
1
2
>> IR(SortedPeople([{Name:"John"},{Name:"Jane"}]))
SortedPeople:*[Age:n, Name:s](Table:*[Name:s]({Name: "John":s}, {Name: "Jane":s}))
>> IR(SortedPeople([{Name:1},{Name:2}])) // numbers coerce to strings
SortedPeople:*[Age:n, Name:s](AggregateCoercion(TableToTable, Name <- DecimalToText:s(ScopeAccess(Scope 1, Name))))
If you look in the interpreter for the bug case, the type of Arg0 to SortByColumns doesn't include Age. The argument is passed through unmodified through the UDF parameter. By using numbers in the second example, a coercion is required, which brings in the Age column.
What is the expectation here? Should an AggregateCoercion have been inserted in the bug case since the type of the tables wasn't exactly the same? In general, are missing columns acceptable throughout the system?
Or should SortByColumns have assumed that some columns referenced may not be there and treat them as Blank? This can't be done directly as it is impossible to differentiate a missing column name from an invalid column name, but can be done roundabout by using the column names of the result from SortByColumns in the IRContext instead of the first argument.
It is tempting just to patch SortByColumns, but that is just the case I found. Is this a more general problem?
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in the interpreter around SortByColumns and inspect how Arg0 types, UDF parameters, AggregateCoercion, and IRContext handle missing columns. Reproduce the examples in the issue, especially the omitted Age field, then determine whether coercion or Blank handling is the intended general behavior. Done means the behavior is decided and the regression is covered for the reported cases.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100