String optimize

[pixelherodev]

Slight speed-up for a few string conversions.

Strings are immutable. As such, string([]byte(foo)), given a mutable buffer, must copy the string, so that changes to the underlying slice do not break the language's requirements.

unsafe.String(foo, length), instead, crafts a string header pointing to the existing buffer. So long as that buffer is never modified for the lifetime of the string, this can be totally safe, and is faster.

Since the flatbufs will never be modified, it's totally safe to craft strings that point directly into its innards. The string pointer is sufficient to prevent GC, and we never modify that flatbuf data on read paths.

Tests pass and there's a small but measurable performance improvement in profiles I was looking at.

---

Important

🛠️ Maintainer triage note for @pixelherodev · by @zeroshade · 2026-08-27 20:05 UTC

Helpful heads-up from the maintainers — please address these Pull Request quality criteria items before this PR can be reviewed:

• ❌ Static checks — failing: Lint. See docs.
• ❌ Failing CI checks — failing: AMD64 Debian 12 Go 1.24, ARM64 Debian 12 Go 1.24, Verify (macos-latest), AMD64 Debian 12 Go 1.24 - CGO, Verify (ubuntu-latest) (+5 more). See docs.

The ball is in your court — you've been assigned to this PR. Fix the above, then mark it Ready for review.

_{Automated triage — may be imperfect; a maintainer takes the next look.}

[zeroshade]

do we see any measurable performance difference in the existing benchmarks?

[pixelherodev]

I don't think so.

For the profiles I was looking at, the rate of timestamps was sufficient that this minor savings was happening millions of times per second, and was still pretty small, I think. I can double check, but this one might be worth skipping, yeah.

[zeroshade]

@pixelherodev can you fix the lint issue any the failures here?

[zeroshade]

@pixelherodev A few things need addressing before review — see our Pull Request quality criteria.

• ❌ Lint / pre-commit — failing: Lint. See docs.
• ❌ Platform tests — failing: AMD64 Debian 12 Go 1.24, ARM64 Debian 12 Go 1.24, Verify (macos-latest), AMD64 Debian 12 Go 1.24 - CGO, Verify (ubuntu-latest) (+5 more). See docs.

Note: Your branch is 432 commits behind main. Please rebase and push again to get up-to-date CI results.

No rush.

---

Note: This comment was drafted by an AI-assisted triage tool run by a maintainer, and may contain mistakes. Once you have addressed the points above, an Apache Arrow Go maintainer — a real person — will take the next look at your PR. If anything here looks wrong, say so on the PR and a maintainer will follow up. See CONTRIBUTING.md for the project's contribution conventions.

[zeroshade]

Found two blocking correctness issues in the zero-copy FlatBuffer string conversions: legal empty values panic, and decoded schema strings alias caller-owned or memory-mapped input storage. The file also needs gofmt, and the branch needs current CI after rebasing.

---

This review was drafted by an AI-assisted tool and confirmed by an Apache Arrow Go maintainer. The maintainer requesting these changes has read the findings and signed off. If something feels off, please reply on the PR and a maintainer will follow up.

More on how Apache Arrow Go handles maintainer review:
CONTRIBUTING.md.

[zeroshade]

Blocking: Arrow permits empty field names, so &name[0] panics when len(name) == 0. More broadly, unsafe.String makes the decoded schema borrow FlatBuffer storage; with NewMappedFileReader, that storage can be caller-owned or memory-mapped, and mutating it changes the already-decoded field name. Please retain the owned conversion:

o.Name = string(field.Name())

unsafe.SliceData would only address empty input, not ownership.

[zeroshade]

Blocking: Unzoned Arrow timestamps have an empty timezone, so this panics on &tz[0]. The resulting string also borrows FlatBuffer storage. Please retain an owned conversion:

tz := string(data.Timezone())
return &arrow.TimestampType{Unit: unit, TimeZone: tz}, nil