shazilkhan commented on PR #26367: URL: https://github.com/apache/datafusion/pull/26367#issuecomment-6297829617
Thanks for the review and for running the benchmarks @neilconway! - **Tests:** moved them all to SLT in `string_literal.slt`, next to the existing lpad/rpad tests, and removed the unit tests from `pad.rs`. Pushed in f41e6ea. - **Splitting PRs:** fair point. Next time I'll send the refactor on its own and keep perf and test changes separate. - **`rpad utf8 unicode no fill [size=4096]` (1.20x):** I couldn't find a cause in the code. That path writes the spaces the same way as before, and `lpad` shares it and is flat in your run, as is the 1024-row case. Locally both lpad and rpad move about ±10% on this case between runs, so I think it's mostly noise. Happy to look closer if you think it's real. -- 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]
