Skip to content

[arrow-array] Use consistent value_length name in FixedSizeBinaryArray#9905

Merged
alamb merged 1 commit into
apache:mainfrom
alamb:alamb/rename_fsb
May 5, 2026
Merged

[arrow-array] Use consistent value_length name in FixedSizeBinaryArray#9905
alamb merged 1 commit into
apache:mainfrom
alamb:alamb/rename_fsb

Conversation

@alamb
Copy link
Copy Markdown
Contributor

@alamb alamb commented May 5, 2026

Which issue does this PR close?

Rationale for this change

While trying to avoid overflows due to using i32 arithmetic in FixedSizeBinaryArray, I found the use of the term size in parameters to be confusing when the field name is called value_length

What changes are included in this PR?

Change several parameter / variable names to value_length to keep the code consistent

Are these changes tested?

By CI

Are there any user-facing changes?

No this is an internal code refactor

@github-actions github-actions Bot added the arrow Changes to the arrow crate label May 5, 2026
@alamb alamb force-pushed the alamb/rename_fsb branch 2 times, most recently from ed2ff20 to 633b5c4 Compare May 5, 2026 13:34
@alamb alamb force-pushed the alamb/rename_fsb branch from 633b5c4 to 6b10c3f Compare May 5, 2026 13:46
@alamb alamb changed the title [arrow-array] rename size parameters to value_length [arrow-array] Use consistent value_length name in FixedSizeBinaryArray May 5, 2026
data_type,
value_data: values,
value_length: size,
value_length,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

here is an example of inconsistent terminology size and value_length used for the same concept

After this PR, the same concept has the same name in both parameters and fields

Copy link
Copy Markdown
Contributor

@etseidl etseidl left a comment

Choose a reason for hiding this comment

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

Thanks @alamb, the naming now makes more sense.

@alamb
Copy link
Copy Markdown
Contributor Author

alamb commented May 5, 2026

Since this is a name only hange and I have some stacked work on it, I will merge it in

@alamb alamb merged commit 8091f3f into apache:main May 5, 2026
28 checks passed
@alamb
Copy link
Copy Markdown
Contributor Author

alamb commented May 5, 2026

Thank you @etseidl

@alamb alamb deleted the alamb/rename_fsb branch May 5, 2026 19:08
Rich-T-kid pushed a commit to Rich-T-kid/arrow-rs that referenced this pull request Jun 2, 2026
…ray (apache#9905)

# Which issue does this PR close?
- Part of apache#9906
- First follow on to apache#9872

# Rationale for this change

While trying to avoid overflows due to using i32 arithmetic in
FixedSizeBinaryArray, I found the use of the term `size` in parameters
to be confusing when the field name is called `value_length`

# What changes are included in this PR?

Change several parameter / variable names to `value_length` to keep the
code consistent

# Are these changes tested?

By CI

# Are there any user-facing changes?

No this is an internal code refactor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants