fallintoplace commented on code in PR #1198:
URL: https://github.com/apache/iceberg-go/pull/1198#discussion_r3439143159
##########
transforms.go:
##########
@@ -477,6 +484,18 @@ func (t TruncateTransform) Transformer(src Type)
(func(any) any, error) {
ErrInvalidArgument, src)
}
+func truncateString(s string, width int) string {
+ for idx := range s {
+ if width == 0 {
+ return s[:idx]
Review Comment:
I checked the Go spec again, and I think the small reason to keep this
version is that range over a string already gives us the byte index for each
UTF-8 code point boundary:
https://go.dev/ref/spec#For_statements_with_range_clause
So this only walks until it finds the cutoff point and then slices the
original string there.
With the []rune version we’d need a bounds check to avoid panicking when
width is larger than the number of runes, and it also converts the whole string
to a rune slice first.
That said, I agree the []rune version is easier to understand at a glance.
I’m happy to either keep this and add a short comment, or switch to the clearer
version with the bounds check if you prefer.
--
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]
