Repository navigation
Conversation
…ive partitions HivePartitioning::FormatValues always URI-encoded partition values, so setting segment_encoding to none only affected parsing. Honor the option when formatting, keeping uri as the default behavior.
Niltra
force-pushed
the
GH-34905-hive-segment-encoding
branch
from
October 30, 2026 19:38
22ea0b7 to
737ba0a
Compare
adamreeve
reviewed
Nov 9, 2026
| std::string value = values[i]->ToString(); | ||
| switch (hive_options_.segment_encoding) { | ||
| case SegmentEncoding::None: | ||
| break; |
Contributor
There was a problem hiding this comment.
Hi @Niltra, thanks for working on this. I have a PR open for the opposite scenario in directory and filename partitioning, which currently do no segment encoding when writing: #55423
As part of my PR, I've also made some changes to the no-encoding path so that it raises errors on invalid path segments.
I think we should wait until my PR is merged, and then re-use the validation here for Hive partitioning.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
HivePartitioning::FormatValuesalways URI-encoded partition values, even when the partitioning was created withsegment_encoding="none". The option therefore only affected parsing: writingMy Fancy Productstill producedpart=My%20Fancy%20Product, as reported in GH-34905 and GH-41618.Closes #34905.
What changes are included in this PR?
HivePartitioning::FormatValuesnow honourssegment_encoding:Urikeeps the current behaviour,Nonewrites values verbatim.%in the partition value.ds.write_datasetwith both encodings, including read-back.segment_encodingdocumentation in the C++ header and the Python docstring.The default remains
"uri", so existing output is unchanged.Are these changes tested?
arrow-dataset-partition-test: 38/38 pass, includingHivePartitioningFormatand the newWriteHiveWithoutUriEncoding.test_write_dataset_hive_segment_encoding: 2/2 pass against a local pyarrow built from this branch and the patched Arrow C++. Against pyarrow 26.0.0 the"none"case fails as expected, which reproduces the issue.clang-format18.1.8,cpplint1.6.1 andflake8pass on the touched files.Are there any user-facing changes?
Yes, a bug fix. This now works, without changing the default:
part=My Fancy Productis written as-is; the default"uri"still writespart=My%20Fancy%20Product. With"none"the caller is responsible for using values that are valid path segments.Was AI used for this PR?
PR code and description written by:
Reviewed before submission by:
This PR was human-directed and AI-assisted. The author selected the issue, set the requirements and quality bar (minimal change, no regressions, pass Arrow's checks), discussed the approach with the assistant, reviewed the test evidence, and authorised submission. The implementation, tests and description were produced with AI assistance and verified locally as described above. The change is limited to the existing
segment_encodingoption.