Dandandan commented on PR #23202:
URL: https://github.com/apache/datafusion/pull/23202#issuecomment-4815578955
> LGTM Thanks @Dandandan
>
> Curious about the self.expr.len() == 1 gate — is the cliff actually at 1,
or somewhere higher? Row-format compare is a single memcmp per merge step
regardless of column count, so the fan-in reduction should still cut cursor
overhead for 2-3-column sorts even if the per-compare cost grows slightly with
encoded-key width. Common queries like ORDER BY a, b LIMIT N would benefit if
the crossover is higher.
>
> If you've already measured 2-column and it doesn't win, would be great to
include those numbers in the PR description so the gate threshold is grounded.
Otherwise filing as a follow-up to extend the gate (or making it config-driven,
e.g. sort_coalesce_max_columns) seems worth it.
Yeah at 2 it alreadt is slower (at least for string types), probably as
`lexsort_to_indices` is slow on multiple columns than using merge (which uses
row format). In benchmarks it is faster to use keep the higher fan in rather
than concatenating in this case.
Ill include some bench numbers!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
