Brijesh-Thakkar commented on PR #2988:
URL:
https://github.com/apache/datafusion-comet/pull/2988#issuecomment-3693305625
@coderfender Thank you for the feedback! You're absolutely right.
I've updated the implementation to use a simpler approach with Rust's
built-in `trim()`, `trim_start()`, and `trim_end()` methods on Arrow string
arrays. This avoids reimplementing logic that already exists.
**Current implementation:**
- Uses standard Rust string trimming methods
- Handles both array and scalar inputs
- Supports 1-argument form (whitespace trimming)
- Added basic unit tests
**Regarding 2-argument support:**
The current implementation returns `NotImplemented` error for the 2-argument
form (custom trim characters). I can add this if needed, but wanted to first
confirm:
1. Is 2-argument trim (`TRIM('chars', col)`) actually used in the benchmark?
2. Should I prioritize getting the basic 1-argument case working first?
**Next steps:**
I'll add the Spark-side tests you mentioned to ensure correctness. Could you
point me to which specific test file I should add them to?
Let me know if this approach looks better or if you'd prefer I investigate
using DataFusion's internal trim functions instead (though I wasn't able to
find them exported in the available crates).
--
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]
