Skip to content

fix: Concatenate extension arrays and keep concatenated results valid after inputs are disposed - #444

Merged
CurtHagenlocher merged 1 commit into
apache:mainfrom
CurtHagenlocher:fix-concatenate-extension-and-views
Sep 27, 2026
Merged

CurtHagenlocher merged 1 commit into
apache:mainfrom
CurtHagenlocher:fix-concatenate-extension-and-views

Conversation

@CurtHagenlocher

Copy link
Copy Markdown
Contributor

What's Changed

Fixes the three ArrowArrayConcatenator problems reported in #443, plus a related lifetime issue found while fixing them.

  • Extension arrays are concatenated. ExtensionType.Accept passes the visitor through to the storage type, so the storage visitor then rejected the Extension type ID. ArrayDataConcatenator.Concatenate now checks for an extension type first. It requires every input to have the same extension name and metadata, concatenates the storage arrays, and puts the extension type back on the result. This path also applies to child arrays, so e.g. a list of GuidArrays works too.
  • View results own their data buffers. ConcatenateBinaryViewArrayData now calls Retain() on each variadic data buffer it takes from an input. The result stays valid after the inputs (including SliceShared inputs) are disposed.
  • Empty view inputs with data buffers no longer throw. The loop that copies data buffers now skips zero-length inputs, just as ConcatenateViewBuffer already did when counting them. The buffer count and the view buffer indices now match.
  • Other results that aliased an input now retain it. The result used to share an input's memory without holding a reference in three cases: a single input to either Concatenate method, a nested child where only one input contributed values (for example, lists where only one has values), and the zero-length child or values array used when every input is empty. These now use Retain() / SliceShared(0, 0).

Note: a single-input Concatenate call now returns a new array that holds its own references, not the input object itself. The caller should dispose it like any other concatenation result.

Tests

Eight new tests in ArrowArrayConcatenatorTests cover extension arrays (plain, nested in a list, mismatched types), view results that outlive their inputs (plain and SliceShared), empty view inputs with data buffers, and results that outlive a single input, a list with one non-empty input, and a run-end encoded array with one non-empty input. All eight fail without the fix. The full Apache.Arrow.Tests suite passes on net8.0, net462 and net472.

Closes #443.

🤖 Generated with Claude Code

… after inputs are disposed

- Concatenate extension arrays by concatenating their storage and
  re-applying the extension type.
- Retain the variadic data buffers a concatenated view array shares
  with its inputs, and skip empty inputs consistently when counting
  and copying those buffers.
- Retain the input when the result would otherwise alias it: a single
  input, a single non-empty nested child, or the empty child used when
  every input is empty.

Closes apache#443.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The reviewed changes are covered by regression tests and have no unresolved blocking issues.

Review effort: Lite
Findings: None

What changed in this PR

Fixes ArrowArrayConcatenator support for extension arrays and preserves result validity after input disposal.

Changes:

  • Adds extension-array concatenation and validation.
  • Retains view and aliased buffers safely.
  • Adds regression tests for lifetime and empty-input cases.
File Description
test/​Apache.Arrow.Tests/​ArrowArrayConcatenatorTests.cs Adds extension, view, and lifetime regression tests.
src/​Apache.Arrow/​Arrays/​ArrowArrayConcatenator.cs Retains data for single-input results.
src/​Apache.Arrow/​Arrays/​ArrayDataConcatenator.cs Implements extension concatenation and buffer ownership fixes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@CurtHagenlocher
CurtHagenlocher merged commit 931db0e into apache:main Sep 27, 2026
14 checks passed
@CurtHagenlocher
CurtHagenlocher deleted the fix-concatenate-extension-and-views branch September 27, 2026 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ArrowArrayConcatenator: extension arrays are refused, and view arrays share unretained data buffers and fail on empty inputs

2 participants