Skip to content

feat: Support int32 to uint64 in reverseTransformArray#728

Merged
kodiakhq[bot] merged 2 commits intomainfrom
claude/stoic-roentgen
Mar 23, 2026
Merged

feat: Support int32 to uint64 in reverseTransformArray#728
kodiakhq[bot] merged 2 commits intomainfrom
claude/stoic-roentgen

Conversation

@erezrokah
Copy link
Member

Summary

  • Add reverseTransformFromInt32 to handle int32 to uint64 array widening in parquet reads
  • Add corresponding case in reverseTransformArray switch for *array.Int32
  • Mirrors the existing reverseTransformFromUint32 implementation

Test plan

  • TestReverseTransformArray_Int32ToUint64 — basic values (0, 42, null, max int32)
  • TestReverseTransformArray_Int32ToUint64_Empty — empty array
  • TestReverseTransformArray_Int32ToUint64_ListOf — list of int32 → list of uint64

Add reverseTransformFromInt32 to handle int32 to uint64 widening,
mirroring the existing uint32 to uint64 support.
Copilot AI review requested due to automatic review settings March 23, 2026 17:37
@erezrokah erezrokah requested a review from a team as a code owner March 23, 2026 17:37
@erezrokah erezrokah requested review from murarustefaan and removed request for a team March 23, 2026 17:37
@erezrokah erezrokah changed the title feat(parquet): Support int32 to uint64 in reverseTransformArray feat: Support int32 to uint64 in reverseTransformArray Mar 23, 2026
@erezrokah erezrokah added the automerge Add to automerge PRs once requirements are met label Mar 23, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for widening int32 arrays to uint64 during Parquet reads, aligning reverse-transformation behavior with existing uint32uint64 handling in the Arrow conversion layer.

Changes:

  • Add reverseTransformFromInt32 to convert *array.Int32 to uint64 arrays.
  • Add *array.Int32 handling in reverseTransformArray dispatch.
  • Add unit tests covering scalar, empty, and list-of conversions for int32uint64.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
parquet/read.go Adds a new reverse-transform path for *array.Int32 to support uint64 schema expectations.
parquet/read_test.go Adds test coverage for int32uint64 conversions including list arrays.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +110 to +118
builder := array.NewUint64Builder(memory.DefaultAllocator)
for i := 0; i < arr.Len(); i++ {
if arr.IsNull(i) {
builder.AppendNull()
continue
}
v := arr.Value(i)
if v < 0 {
panic(fmt.Errorf("negative int32 value %d at index %d cannot be converted to uint64", v, i))
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverseTransformFromInt32 allocates a Uint64Builder but never releases it. In Arrow Go, builders are reference-counted and should be Release()d to avoid leaking allocator-backed memory; you can defer builder.Release() (safe because builder.NewArray() returns a separate array) or release after creating the array.

Copilot uses AI. Check for mistakes.
@kodiakhq kodiakhq bot merged commit 5ed6ca8 into main Mar 23, 2026
7 checks passed
@kodiakhq kodiakhq bot deleted the claude/stoic-roentgen branch March 23, 2026 17:42
kodiakhq bot pushed a commit that referenced this pull request Mar 23, 2026
🤖 I have created a release *beep* *boop*
---


## [4.7.0](v4.6.16...v4.7.0) (2026-03-23)


### Features

* Support int32 to uint64 in reverseTransformArray ([#728](#728)) ([5ed6ca8](5ed6ca8))


### Bug Fixes

* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.94.7 ([#726](#726)) ([ccf6a31](ccf6a31))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Add to automerge PRs once requirements are met

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants