Skip to content

GH-43170: [Swift] Add StructArray support to ArrowWriter#43439

Merged
kou merged 1 commit intoapache:mainfrom
abandy:GH-43170
May 28, 2025
Merged

GH-43170: [Swift] Add StructArray support to ArrowWriter#43439
kou merged 1 commit intoapache:mainfrom
abandy:GH-43170

Conversation

@abandy
Copy link
Contributor

@abandy abandy commented Jul 26, 2024

Rationale for this change

Support for Struct type has been added to the Swift ArrayReader. This change adds this support to the ArrowWriter.

What changes are included in this PR?

Updates to the ArrowWriter to support the Struct type.

Are these changes tested?

Yes, test included in PR

@abandy abandy requested a review from kou as a code owner July 26, 2024 22:40
@github-actions
Copy link

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

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

Comment on lines 94 to 95
Copy link
Member

Choose a reason for hiding this comment

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

childrenOffset?

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Jul 29, 2024
@kou
Copy link
Member

kou commented May 23, 2025

@abandy Sorry. I forgot this PR.

Could you rebase on main? Let's merge this before we move swift/ to apache/arrow-swift.

Cc: @raulcd

@raulcd
Copy link
Member

raulcd commented May 23, 2025

Thanks for the ping @kou

@raulcd raulcd requested a review from kou May 26, 2025 08:47
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
switch arrowType.id {
switch typeId {

@kou kou merged commit b5b6707 into apache:main May 28, 2025
5 of 6 checks passed
@kou kou removed the awaiting merge Awaiting merge label May 28, 2025
@kou
Copy link
Member

kou commented May 28, 2025

@raulcd We're ready to move swift/!

@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit b5b6707.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 22 possible false positives for unstable benchmarks that are known to sometimes produce them.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants