-
Notifications
You must be signed in to change notification settings - Fork 354
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?