duckdb-r
duckdb-r copied to clipboard
rel_order accepts sort order as an argument
Related to duckdblabs/duckplyr#92
Changes:
-
rapi_rel_order
now accepts a vector of boolean values that describe whether each expression should be sorted in ascending order -
rel_order
checks that the number of expressions and boolean values match or defaults to TRUE if a null-value is passed - tests for the relation api test the
rel_order()
, notrapi_rel_order()
. The tests break without this change as the default value is handled in rel_order.
I will mark this as a draft due to these notes:
- This does not work for nested expressions. For example, assume someone writes
arrange(someotherfunction(desc(column)))
in duckplyr. I don't know if this is an issue as the interpretation of such an expression in terms of sort order is a bit ambiguous(?).- note: this problem exists also without changes proposed here?
- I was unsure what function should to check that
length(orders) == length(ascending)
. It is nowrel_order()
,rapi_rel_order()
. As a result, using rapi_rel_order directly might result indexing out of array bounds. Perhaps the check should be in rapi_rel_order as the vector is indexed there? - Related to the previous note. Should we aim to test
rel_order()
orrapi_rel_order
? There are examples of both for other relation expressions