Skip to content

Navigation Menu

Sign in
Sign up

GH-50657: [C++][Dev] Implement BinaryView support in gdb_arrow.py - #50662

Open
KHARSHAVARDHAN-eng wants to merge 2 commits into
apache:main from
KHARSHAVARDHAN-eng:gh-50657-binaryview-gdb
Open

GH-50657: [C++][Dev] Implement BinaryView support in gdb_arrow.py #50662
KHARSHAVARDHAN-eng wants to merge 2 commits into
apache:main from
KHARSHAVARDHAN-eng:gh-50657-binaryview-gdb

Conversation

@KHARSHAVARDHAN-eng

@KHARSHAVARDHAN-eng KHARSHAVARDHAN-eng commented Jul 27, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown

Why are these changes needed?

gdb_arrow.py currently does not support pretty-printing for BinaryView and StringView data types.

This PR adds debugger support for these types while reusing the existing pretty-printer infrastructure.

What changes are included?

  • Add BinaryView and StringView type registration.
  • Reuse the existing pretty-printer implementation where appropriate.
  • Add debugger fixtures.
  • Add GDB test coverage for the new types.

Are these changes tested?

Yes.

  • Added GDB tests.
  • Ran the relevant debugger tests locally.
  • Verified Python syntax.
  • Verified pre-commit hooks.
  • Verified formatting.

Are there any user-facing changes?

No.

This change only improves the developer debugging experience.

Copy link
Copy Markdown

⚠️ GitHub issue #50657 has been automatically assigned in GitHub to PR creator.

fenfeng9 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

@KHARSHAVARDHAN-eng Hi! Are you still planning to continue this PR? It overlaps with GH-50680, which was merged into main and already added STRING_VIEW and BINARY_VIEW to _type_ids in gdb_arrow.py

Comment thread cpp/gdb_arrow.py Outdated
Comment on lines +50 to +51
_type_id_tuples = [(name, i) for i, name in enumerate(_type_ids)]
_type_id_tuples.extend([('STRING_VIEW', 39), ('BINARY_VIEW', 40)])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why is this necessary?

Copy link
Copy Markdown
Author

Hi @raulcd,

Thanks for the review.

Originally, this PR included the manual _type_id_tuples extension because STRING_VIEW and BINARY_VIEW were not yet present in gdb_arrow.py.

Since GH-50680 has now been merged, I've rebased this PR onto the latest main and removed that redundant workaround.

This PR now focuses only on adding the pretty-printer support and corresponding tests for StringView and BinaryView.

Please let me know if you see anything else that should be adjusted. Thanks!

Copy link
Copy Markdown
Author

Hi @fenfeng9,

Thanks for pointing that out.

I've now rebased this PR onto the latest main and removed the duplicated enum registration that was added by GH-50680.

The remaining changes are limited to the pretty-printer implementation and its associated tests for StringView and BinaryView.

Thanks for the review!

Comment thread cpp/gdb_arrow.py Outdated
Comment on lines +2064 to +2065
Type.STRING_VIEW: DataTypeTraits(BaseBinaryTypeClass, 'StringViewType'),
Type.BINARY_VIEW: DataTypeTraits(BaseBinaryTypeClass, 'BinaryViewType'),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this doesn't seem correct. This will use the BinaryArrayDataPrinter for Binary View layout Arrays. You should add tests for arrays:

# Binary-like
check_heap_repr(
gdb_arrow, "heap_fixed_size_binary_array",
(r'arrow::FixedSizeBinaryArray of type arrow::fixed_size_binary(3), '
r'length 3, offset 0, null count 1 = {'
r'[0] = null, [1] = "abc", [2] = "000円037円377円"}'))
check_heap_repr(
gdb_arrow, "heap_fixed_size_binary_array_zero_width",
(r'arrow::FixedSizeBinaryArray of type arrow::fixed_size_binary(0), '
r'length 2, offset 0, null count 1 = {[0] = null, [1] = ""}'))
check_heap_repr(
gdb_arrow, "heap_fixed_size_binary_array_sliced",
(r'arrow::FixedSizeBinaryArray of type arrow::fixed_size_binary(3), '
r'length 1, offset 1, unknown null count = {[0] = "abc"}'))
check_heap_repr(
gdb_arrow, "heap_binary_array",
(r'arrow::BinaryArray of length 3, offset 0, null count 1 = {'
r'[0] = null, [1] = "abcd", [2] = "000円037円377円"}'))
check_heap_repr(
gdb_arrow, "heap_large_binary_array",
(r'arrow::LargeBinaryArray of length 3, offset 0, null count 1 = {'
r'[0] = null, [1] = "abcd", [2] = "000円037円377円"}'))
check_heap_repr(
gdb_arrow, "heap_string_array",
(r'arrow::StringArray of length 3, offset 0, null count 1 = {'
r'[0] = null, [1] = "héhé", [2] = "invalid \\xff char"}'))
check_heap_repr(
gdb_arrow, "heap_large_string_array",
(r'arrow::LargeStringArray of length 3, offset 0, null count 1 = {'
r'[0] = null, [1] = "héhé", [2] = "invalid \\xff char"}'))
check_heap_repr(
gdb_arrow, "heap_binary_array_sliced",
(r'arrow::BinaryArray of length 1, offset 1, unknown null count = '
r'{[0] = "abcd"}'))

Copy link
Copy Markdown
Author

@raulcd , in process of correcting my old pull request..

Thanks for the review! I’ve updated the implementation to use a dedicated BinaryViewArrayDataPrinter and added array coverage for StringView/BinaryView, including sliced arrays.

Copy link
Copy Markdown
Author

Hi @raulcd @pitrou @AlenkaF @rok, just following up on this PR. The requested changes and tests have been addressed. When you get a chance, could you please take another look? Thanks!

1 similar comment

Copy link
Copy Markdown
Author

Hi @raulcd @pitrou @AlenkaF @rok, just following up on this PR. The requested changes and tests have been addressed. When you get a chance, could you please take another look? Thanks!

Copy link
Copy Markdown
Author

Hi @pitrou @AlenkaF @rok, just following up on this PR. The requested changes and tests have been addressed. When you get a chance, could you please take another look?

Thanks!

Copy link
Copy Markdown
Author

Hi @pitrou @AlenkaF @rok, just following up on this PR. The requested changes and tests have been addressed. When you get a chance, could you please take another look?

Thanks!

pitrou commented Sep 10, 2026

Copy link
Copy Markdown
Member

The failures in Conda-based builds will be fixed by #51266

pitrou commented Sep 10, 2026

Copy link
Copy Markdown
Member

Actually, @KHARSHAVARDHAN-eng , can you rebase / merge from git main?
KHARSHAVARDHAN-eng reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@raulcd raulcd raulcd left review comments
@pitrou pitrou Awaiting requested review from pitrou pitrou is a code owner
@AlenkaF AlenkaF Awaiting requested review from AlenkaF AlenkaF is a code owner
@rok rok Awaiting requested review from rok rok is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /