From 99c3c1506e39d8a46b36b26d40a1268dc937205a Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Thu, 5 Nov 2026 14:19:18 +1300 Subject: [PATCH 01/13] Use the segment_encoding option when writing a directory-partitioned dataset The segment_encoding option was already respected when reading a directory-partitioned dataset, but ignored when writing. This change now URI encodes path segments if the encoding mode is set to Uri. In addition, invalid path segments now raise an error if they are invalid. --- cpp/src/arrow/dataset/partition.cc | 29 ++++++- cpp/src/arrow/dataset/partition.h | 5 ++ cpp/src/arrow/dataset/partition_test.cc | 109 ++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/dataset/partition.cc b/cpp/src/arrow/dataset/partition.cc index 4d5c6e8ce7a7..a9d9abb15565 100644 --- a/cpp/src/arrow/dataset/partition.cc +++ b/cpp/src/arrow/dataset/partition.cc @@ -321,7 +321,34 @@ Result> KeyValuePartitioning::FormatPartitionSegments( for (int i = 0; i < schema_->num_fields(); ++i) { if (values[i] != nullptr && values[i]->is_valid) { - segments[i] = values[i]->ToString(); + std::string value = values[i]->ToString(); + switch (options_.segment_encoding) { + case SegmentEncoding::None: { + // Without encoding, it's an error to write values that are not a valid path + // segment + if (value == "." || value == ".." || + value.find(fs::internal::kSep) != std::string::npos) { + return Status::Invalid("Partition value for ", schema_->field(i)->name(), + " is not a valid path segment: '", value, + "'. Use SegmentEncoding::Uri to write this value."); + } + segments[i] = std::move(value); + break; + } + case SegmentEncoding::Uri: { + segments[i] = ::arrow::util::UriEscape(value); + // A "." isn't normally required to be escaped, but "." and ".." are not + // valid directory names, and dataset discovery skips directories starting + // with ".". + if (!segments[i].empty() && segments[i][0] == '.') { + segments[i] = "%2E" + segments[i].substr(1); + } + break; + } + default: + return Status::NotImplemented("Unknown segment encoding: ", + options_.segment_encoding); + } continue; } diff --git a/cpp/src/arrow/dataset/partition.h b/cpp/src/arrow/dataset/partition.h index 315a3d384d28..6d1be0f3efa0 100644 --- a/cpp/src/arrow/dataset/partition.h +++ b/cpp/src/arrow/dataset/partition.h @@ -118,6 +118,11 @@ std::ostream& operator<<(std::ostream& os, SegmentEncoding segment_encoding); struct ARROW_DS_EXPORT KeyValuePartitioningOptions { /// After splitting a path into components, decode the path components /// before parsing according to this scheme. + /// + /// For directory and filename partitioning, partition values are also encoded + /// according to this scheme when formatting paths for writing. With + /// SegmentEncoding::None, values that are not valid path segments (containing + /// a path separator, or equal to "." or "..") raise errors. SegmentEncoding segment_encoding = SegmentEncoding::Uri; }; diff --git a/cpp/src/arrow/dataset/partition_test.cc b/cpp/src/arrow/dataset/partition_test.cc index 9f0bd7c0be04..3fd3abb4bcbc 100644 --- a/cpp/src/arrow/dataset/partition_test.cc +++ b/cpp/src/arrow/dataset/partition_test.cc @@ -30,6 +30,7 @@ #include "arrow/compute/api_vector.h" #include "arrow/compute/cast.h" #include "arrow/dataset/dataset.h" +#include "arrow/dataset/discovery.h" #include "arrow/dataset/file_ipc.h" #include "arrow/dataset/test_util_internal.h" #include "arrow/filesystem/path_util.h" @@ -245,6 +246,114 @@ TEST_F(TestPartitioning, DirectoryPartitioningEquals) { EXPECT_TRUE(another->Equals(*some_other)); } +TEST_F(TestPartitioning, DirectoryPartitioningEscapesValues) { + partitioning_ = + std::make_shared(schema({field("part", utf8())})); + written_schema_ = partitioning_->schema(); + + AssertFormat(equal(field_ref("part"), literal("plain")), "plain"); + AssertFormat(equal(field_ref("part"), literal("a b")), "a%20b"); + AssertFormat(equal(field_ref("part"), literal("100%")), "100%25"); + AssertFormat(equal(field_ref("part"), literal("a/b")), "a%2Fb"); + AssertFormat(equal(field_ref("part"), literal("ab\\cd")), "ab%5Ccd"); + AssertFormat(equal(field_ref("part"), literal("")), "%3Ctest%3E"); + AssertFormat(equal(field_ref("part"), literal(":|*")), "%3A%7C%2A"); + // "." is only escaped when it is the first character + AssertFormat(equal(field_ref("part"), literal("a.b")), "a.b"); + AssertFormat(equal(field_ref("part"), literal(".")), "%2E"); + AssertFormat(equal(field_ref("part"), literal("..")), "%2E."); + AssertFormat(equal(field_ref("part"), literal(".hidden")), "%2Ehidden"); + AssertFormat(equal(field_ref("part"), literal("../escaped")), "%2E.%2Fescaped"); +} + +TEST_F(TestPartitioning, DirectoryPartitioningEscapedValuesRoundTrip) { + // Write and scan a dataset, to check that escaped directories are created + // inside the base directory and aren't skipped by discovery when starting + // with a ".". + fs::TimePoint mock_now = std::chrono::system_clock::now(); + ASSERT_OK_AND_ASSIGN(std::shared_ptr filesystem, + fs::internal::MockFileSystem::Make(mock_now, {})); + auto base_path = "dataset"; + ASSERT_OK(filesystem->CreateDir(base_path)); + auto schema = arrow::schema( + {arrow::field("part", arrow::utf8()), arrow::field("x", arrow::int32())}); + + auto table = TableFromJSON(schema, { + R"([ + ["../escaped", 0], + [".", 1], + ["..", 2] + ])", + }); + + auto dataset = std::make_shared(table); + ASSERT_OK_AND_ASSIGN(auto scanner_builder, dataset->NewScan()); + ASSERT_OK_AND_ASSIGN(auto scanner, scanner_builder->Finish()); + + auto partition_schema = arrow::schema({arrow::field("part", arrow::utf8())}); + auto partitioning = std::make_shared(partition_schema); + auto ipc_format = std::make_shared(); + dataset::FileSystemDatasetWriteOptions write_options; + write_options.file_write_options = ipc_format->DefaultWriteOptions(); + write_options.filesystem = filesystem; + write_options.base_dir = base_path; + write_options.partitioning = partitioning; + write_options.basename_template = "part{i}.arrow"; + ASSERT_OK(dataset::FileSystemDataset::Write(write_options, scanner)); + + auto mockfs = + arrow::internal::checked_pointer_cast(filesystem); + std::vector all_dirs; + for (const auto& dir : mockfs->AllDirs()) { + all_dirs.push_back(dir.full_path); + } + std::sort(all_dirs.begin(), all_dirs.end()); + std::vector expected_dirs = {"dataset", "dataset/%2E", "dataset/%2E.", + "dataset/%2E.%2Fescaped"}; + ASSERT_EQ(all_dirs, expected_dirs); + + // Read dataset back to verify partition values round-trip correctly + fs::FileSelector selector; + selector.base_dir = base_path; + selector.recursive = true; + FileSystemFactoryOptions factory_options; + factory_options.partitioning = partitioning; + factory_options.partition_base_dir = base_path; + ASSERT_OK_AND_ASSIGN( + auto factory, + FileSystemDatasetFactory::Make(filesystem, selector, ipc_format, factory_options)); + ASSERT_OK_AND_ASSIGN(auto read_dataset, factory->Finish()); + ASSERT_OK_AND_ASSIGN(auto read_scanner_builder, read_dataset->NewScan()); + ASSERT_OK(read_scanner_builder->Project({"part", "x"})); + ASSERT_OK_AND_ASSIGN(auto read_scanner, read_scanner_builder->Finish()); + ASSERT_OK_AND_ASSIGN(auto read_table, read_scanner->ToTable()); + + ASSERT_OK_AND_ASSIGN( + auto sort_indices, + compute::SortIndices(read_table, compute::SortOptions({compute::SortKey("x")}))); + ASSERT_OK_AND_ASSIGN(auto sorted, compute::Take(read_table, sort_indices)); + AssertTablesEqual(*table, *sorted.table(), /*same_chunk_layout=*/false); +} + +TEST_F(TestPartitioning, DirectoryPartitioningNoSegmentEncoding) { + KeyValuePartitioningOptions options; + options.segment_encoding = SegmentEncoding::None; + partitioning_ = std::make_shared(schema({field("part", utf8())}), + ArrayVector(), options); + written_schema_ = partitioning_->schema(); + + // Values are written unescaped and round-trip without decoding + AssertFormat(equal(field_ref("part"), literal("a b")), "a b"); + AssertFormat(equal(field_ref("part"), literal("a%20b")), "a%20b"); + AssertFormat(equal(field_ref("part"), literal("")), ""); + + // Values that aren't a single valid path segment are rejected + AssertFormatError(equal(field_ref("part"), literal("../escaped"))); + AssertFormatError(equal(field_ref("part"), literal("a/b"))); + AssertFormatError(equal(field_ref("part"), literal("."))); + AssertFormatError(equal(field_ref("part"), literal(".."))); +} + TEST_F(TestPartitioning, FilenamePartitioning) { partitioning_ = std::make_shared( schema({field("alpha", int32()), field("beta", utf8())})); From 2cd88cf62c70c23aeda58c69d439fe6e42663e9d Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Thu, 5 Nov 2026 15:42:47 +1300 Subject: [PATCH 02/13] Add test cases for filename partitioning --- cpp/src/arrow/dataset/partition_test.cc | 49 +++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/cpp/src/arrow/dataset/partition_test.cc b/cpp/src/arrow/dataset/partition_test.cc index 3fd3abb4bcbc..faf1ef91cb0f 100644 --- a/cpp/src/arrow/dataset/partition_test.cc +++ b/cpp/src/arrow/dataset/partition_test.cc @@ -676,6 +676,55 @@ TEST_F(TestPartitioning, FilenamePartitioningFormat) { AssertFormat(equal(field_ref("alpha"), literal(0)), "", "0_"); } +TEST_F(TestPartitioning, FilenamePartitioningEscapesValues) { + partitioning_ = std::make_shared( + schema({field("alpha", utf8()), field("beta", utf8())})); + written_schema_ = partitioning_->schema(); + + AssertFormat(equal(field_ref("alpha"), literal("plain")), "", "plain_"); + AssertFormat(equal(field_ref("alpha"), literal("a b")), "", "a%20b_"); + AssertFormat(equal(field_ref("alpha"), literal("100%")), "", "100%25_"); + AssertFormat(equal(field_ref("alpha"), literal("a/b")), "", "a%2Fb_"); + AssertFormat(equal(field_ref("alpha"), literal("ab\\cd")), "", "ab%5Ccd_"); + AssertFormat(equal(field_ref("alpha"), literal("")), "", "%3Ctest%3E_"); + AssertFormat(equal(field_ref("alpha"), literal(":|*")), "", "%3A%7C%2A_"); + // "." is only escaped when it is the first character of a segment + AssertFormat(equal(field_ref("alpha"), literal("a.b")), "", "a.b_"); + AssertFormat(equal(field_ref("alpha"), literal(".")), "", "%2E_"); + AssertFormat(equal(field_ref("alpha"), literal("..")), "", "%2E._"); + AssertFormat(equal(field_ref("alpha"), literal(".hidden")), "", "%2Ehidden_"); + AssertFormat(equal(field_ref("alpha"), literal("../escaped")), "", "%2E.%2Fescaped_"); + // Each segment is escaped independently + AssertFormat(and_(equal(field_ref("alpha"), literal("a/b")), + equal(field_ref("beta"), literal(".hidden"))), + "", "a%2Fb_%2Ehidden_"); +} + +TEST_F(TestPartitioning, FilenamePartitioningNoSegmentEncoding) { + KeyValuePartitioningOptions options; + options.segment_encoding = SegmentEncoding::None; + partitioning_ = std::make_shared( + schema({field("alpha", utf8()), field("beta", utf8())}), ArrayVector(), options); + written_schema_ = partitioning_->schema(); + + // Values are written unescaped and round-trip without decoding + AssertFormat(equal(field_ref("alpha"), literal("a b")), "", "a b_"); + AssertFormat(equal(field_ref("alpha"), literal("a%20b")), "", "a%20b_"); + AssertFormat(equal(field_ref("alpha"), literal("")), "", "_"); + AssertFormat(equal(field_ref("alpha"), literal(".hidden")), "", ".hidden_"); + AssertFormat(and_(equal(field_ref("alpha"), literal("a b")), + equal(field_ref("beta"), literal("c.d"))), + "", "a b_c.d_"); + + // Values that aren't a single valid path segment are rejected + AssertFormatError(equal(field_ref("alpha"), literal("../escaped"))); + AssertFormatError(equal(field_ref("alpha"), literal("a/b"))); + AssertFormatError(equal(field_ref("alpha"), literal("."))); + AssertFormatError(equal(field_ref("alpha"), literal(".."))); + AssertFormatError(and_(equal(field_ref("alpha"), literal("ok")), + equal(field_ref("beta"), literal("a/b")))); +} + TEST_F(TestPartitioning, DiscoverHiveSchema) { auto options = HivePartitioningFactoryOptions(); options.null_fallback = "xyz"; From c5145e2779532c9767b5cfafe69ad306f171f656 Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Thu, 5 Nov 2026 16:08:20 +1300 Subject: [PATCH 03/13] Handle values containing "_" with filename partitioning --- cpp/src/arrow/dataset/partition.cc | 40 ++++++++++++++++++++----- cpp/src/arrow/dataset/partition.h | 12 +++++++- cpp/src/arrow/dataset/partition_test.cc | 25 ++++++++++++++-- 3 files changed, 66 insertions(+), 11 deletions(-) diff --git a/cpp/src/arrow/dataset/partition.cc b/cpp/src/arrow/dataset/partition.cc index a9d9abb15565..d7c009d1e719 100644 --- a/cpp/src/arrow/dataset/partition.cc +++ b/cpp/src/arrow/dataset/partition.cc @@ -37,6 +37,7 @@ #include "arrow/scalar.h" #include "arrow/util/int_util_overflow.h" #include "arrow/util/logging.h" +#include "arrow/util/string.h" #include "arrow/util/uri.h" #include "arrow/util/utf8.h" @@ -61,6 +62,9 @@ Result SafeUriUnescape(std::string_view encoded) { return decoded; } +/// Percent-encode a single character, eg. '.' becomes "%2E". +std::string PercentEncode(char c) { return "%" + ::arrow::HexEncode(&c, 1); } + // Remove parts of the path after the last filename partition // separator. For example, 12_a_part-0.parquet will become 12_a_ std::string StripNonPrefix(const std::string& path) { @@ -316,7 +320,7 @@ inline std::optional NextValid(const ScalarVector& values, int first_null) } Result> KeyValuePartitioning::FormatPartitionSegments( - const ScalarVector& values) const { + const ScalarVector& values, std::optional segment_separator) const { std::vector segments(static_cast(schema_->num_fields())); for (int i = 0; i < schema_->num_fields(); ++i) { @@ -332,17 +336,37 @@ Result> KeyValuePartitioning::FormatPartitionSegments( " is not a valid path segment: '", value, "'. Use SegmentEncoding::Uri to write this value."); } + // The value can't contain the separator between segments, or it would + // not be parsed back correctly + if (segment_separator.has_value() && + value.find(*segment_separator) != std::string::npos) { + return Status::Invalid("Partition value for '", schema_->field(i)->name(), + "' contains the partition separator '", + *segment_separator, "': '", value, + "'. Use SegmentEncoding::Uri to write this value."); + } segments[i] = std::move(value); break; } case SegmentEncoding::Uri: { - segments[i] = ::arrow::util::UriEscape(value); - // A "." isn't normally required to be escaped, but "." and ".." are not - // valid directory names, and dataset discovery skips directories starting - // with ".". - if (!segments[i].empty() && segments[i][0] == '.') { - segments[i] = "%2E" + segments[i].substr(1); + std::string escaped = ::arrow::util::UriEscape(value); + if (segment_separator.has_value()) { + size_t pos = escaped.find(*segment_separator); + if (pos != std::string::npos) { + const std::string encoded_separator = PercentEncode(*segment_separator); + do { + escaped.replace(pos, 1, encoded_separator); + pos = escaped.find(*segment_separator, pos + encoded_separator.size()); + } while (pos != std::string::npos); + } + } + // A leading "." or "_" isn't normally required to be escaped, but "." and + // ".." are not valid directory names, and dataset discovery skips paths + // starting with "." or "_" by default. + if (!escaped.empty() && (escaped[0] == '.' || escaped[0] == '_')) { + escaped.replace(0, 1, PercentEncode(escaped[0])); } + segments[i] = std::move(escaped); break; } default: @@ -437,7 +461,7 @@ Result DirectoryPartitioning::FormatValues( Result FilenamePartitioning::FormatValues( const ScalarVector& values) const { std::vector segments; - ARROW_ASSIGN_OR_RAISE(segments, FormatPartitionSegments(values)); + ARROW_ASSIGN_OR_RAISE(segments, FormatPartitionSegments(values, kFilenamePartitionSep)); return PartitionPathFormat{ "", fs::internal::JoinAbstractPath(std::move(segments), kFilenamePartitionSep) + kFilenamePartitionSep}; diff --git a/cpp/src/arrow/dataset/partition.h b/cpp/src/arrow/dataset/partition.h index 6d1be0f3efa0..02a30bd5b5dd 100644 --- a/cpp/src/arrow/dataset/partition.h +++ b/cpp/src/arrow/dataset/partition.h @@ -214,8 +214,18 @@ class ARROW_DS_EXPORT KeyValuePartitioning : public Partitioning { /// Convert a Key to a full expression. Result ConvertKey(const Key& key) const; + /// Format partition values as path segments, encoding them according to + /// options_.segment_encoding. + /// + /// \param[in] values the partition values for each schema field + /// \param[in] segment_separator if provided, the character that separates + /// segments within a single path component (eg. for filename partitioning). + /// It is escaped when using SegmentEncoding::Uri, and values containing it + /// result in an error when using SegmentEncoding::None. Result> FormatPartitionSegments( - const ScalarVector& values) const; + const ScalarVector& values, + std::optional segment_separator = std::nullopt) const; + Result> ParsePartitionSegments( const std::vector& segments) const; diff --git a/cpp/src/arrow/dataset/partition_test.cc b/cpp/src/arrow/dataset/partition_test.cc index faf1ef91cb0f..8dd86b7c2579 100644 --- a/cpp/src/arrow/dataset/partition_test.cc +++ b/cpp/src/arrow/dataset/partition_test.cc @@ -264,6 +264,10 @@ TEST_F(TestPartitioning, DirectoryPartitioningEscapesValues) { AssertFormat(equal(field_ref("part"), literal("..")), "%2E."); AssertFormat(equal(field_ref("part"), literal(".hidden")), "%2Ehidden"); AssertFormat(equal(field_ref("part"), literal("../escaped")), "%2E.%2Fescaped"); + // "_" is only escaped when it is the first character + AssertFormat(equal(field_ref("part"), literal("a_b")), "a_b"); + AssertFormat(equal(field_ref("part"), literal("_unknown")), "%5Funknown"); + AssertFormat(equal(field_ref("part"), literal("__x_")), "%5F_x_"); } TEST_F(TestPartitioning, DirectoryPartitioningEscapedValuesRoundTrip) { @@ -282,7 +286,8 @@ TEST_F(TestPartitioning, DirectoryPartitioningEscapedValuesRoundTrip) { R"([ ["../escaped", 0], [".", 1], - ["..", 2] + ["..", 2], + ["_unknown", 3] ])", }); @@ -309,7 +314,8 @@ TEST_F(TestPartitioning, DirectoryPartitioningEscapedValuesRoundTrip) { } std::sort(all_dirs.begin(), all_dirs.end()); std::vector expected_dirs = {"dataset", "dataset/%2E", "dataset/%2E.", - "dataset/%2E.%2Fescaped"}; + "dataset/%2E.%2Fescaped", + "dataset/%5Funknown"}; ASSERT_EQ(all_dirs, expected_dirs); // Read dataset back to verify partition values round-trip correctly @@ -346,6 +352,8 @@ TEST_F(TestPartitioning, DirectoryPartitioningNoSegmentEncoding) { AssertFormat(equal(field_ref("part"), literal("a b")), "a b"); AssertFormat(equal(field_ref("part"), literal("a%20b")), "a%20b"); AssertFormat(equal(field_ref("part"), literal("")), ""); + AssertFormat(equal(field_ref("part"), literal("a_b")), "a_b"); + AssertFormat(equal(field_ref("part"), literal("_unknown")), "_unknown"); // Values that aren't a single valid path segment are rejected AssertFormatError(equal(field_ref("part"), literal("../escaped"))); @@ -694,10 +702,17 @@ TEST_F(TestPartitioning, FilenamePartitioningEscapesValues) { AssertFormat(equal(field_ref("alpha"), literal("..")), "", "%2E._"); AssertFormat(equal(field_ref("alpha"), literal(".hidden")), "", "%2Ehidden_"); AssertFormat(equal(field_ref("alpha"), literal("../escaped")), "", "%2E.%2Fescaped_"); + // "_" is the segment separator, so it is always escaped + AssertFormat(equal(field_ref("alpha"), literal("a_b")), "", "a%5Fb_"); + AssertFormat(equal(field_ref("alpha"), literal("_unknown")), "", "%5Funknown_"); + AssertFormat(equal(field_ref("alpha"), literal("__x_")), "", "%5F%5Fx%5F_"); // Each segment is escaped independently AssertFormat(and_(equal(field_ref("alpha"), literal("a/b")), equal(field_ref("beta"), literal(".hidden"))), "", "a%2Fb_%2Ehidden_"); + AssertFormat(and_(equal(field_ref("alpha"), literal("a_b")), + equal(field_ref("beta"), literal("c_d"))), + "", "a%5Fb_c%5Fd_"); } TEST_F(TestPartitioning, FilenamePartitioningNoSegmentEncoding) { @@ -723,6 +738,12 @@ TEST_F(TestPartitioning, FilenamePartitioningNoSegmentEncoding) { AssertFormatError(equal(field_ref("alpha"), literal(".."))); AssertFormatError(and_(equal(field_ref("alpha"), literal("ok")), equal(field_ref("beta"), literal("a/b")))); + + // Values containing the "_" segment separator are rejected + AssertFormatError(equal(field_ref("alpha"), literal("a_b"))); + AssertFormatError(equal(field_ref("alpha"), literal("_unknown"))); + AssertFormatError(and_(equal(field_ref("alpha"), literal("ok")), + equal(field_ref("beta"), literal("c_d")))); } TEST_F(TestPartitioning, DiscoverHiveSchema) { From ab2113fcb844f410e53b4b6c69049650628c84ab Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Thu, 5 Nov 2026 21:37:35 +1300 Subject: [PATCH 04/13] Reject paths with backslash too, and with a trailing "." --- cpp/src/arrow/dataset/partition.cc | 13 ++++++++++--- cpp/src/arrow/dataset/partition.h | 5 +++-- cpp/src/arrow/dataset/partition_test.cc | 26 ++++++++++++++++++++----- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/cpp/src/arrow/dataset/partition.cc b/cpp/src/arrow/dataset/partition.cc index d7c009d1e719..12a034cebe52 100644 --- a/cpp/src/arrow/dataset/partition.cc +++ b/cpp/src/arrow/dataset/partition.cc @@ -329,9 +329,11 @@ Result> KeyValuePartitioning::FormatPartitionSegments( switch (options_.segment_encoding) { case SegmentEncoding::None: { // Without encoding, it's an error to write values that are not a valid path - // segment - if (value == "." || value == ".." || - value.find(fs::internal::kSep) != std::string::npos) { + // segment. Backslashes are rejected on all platforms as they are a path + // separator on Windows. Windows also strips trailing "." and " " characters + // from path names, and this also covers the "." and ".." special names. + if (value.find_first_of("/\\") != std::string::npos || + (!value.empty() && (value.back() == '.' || value.back() == ' '))) { return Status::Invalid("Partition value for ", schema_->field(i)->name(), " is not a valid path segment: '", value, "'. Use SegmentEncoding::Uri to write this value."); @@ -366,6 +368,11 @@ Result> KeyValuePartitioning::FormatPartitionSegments( if (!escaped.empty() && (escaped[0] == '.' || escaped[0] == '_')) { escaped.replace(0, 1, PercentEncode(escaped[0])); } + // Windows strips trailing "." characters from path names, so escape a + // trailing "." too. (A trailing space is already escaped by UriEscape.) + if (!escaped.empty() && escaped.back() == '.') { + escaped.replace(escaped.size() - 1, 1, PercentEncode('.')); + } segments[i] = std::move(escaped); break; } diff --git a/cpp/src/arrow/dataset/partition.h b/cpp/src/arrow/dataset/partition.h index 02a30bd5b5dd..2d0f2e6da96f 100644 --- a/cpp/src/arrow/dataset/partition.h +++ b/cpp/src/arrow/dataset/partition.h @@ -121,8 +121,9 @@ struct ARROW_DS_EXPORT KeyValuePartitioningOptions { /// /// For directory and filename partitioning, partition values are also encoded /// according to this scheme when formatting paths for writing. With - /// SegmentEncoding::None, values that are not valid path segments (containing - /// a path separator, or equal to "." or "..") raise errors. + /// SegmentEncoding::None, values that are not valid path segments raise errors. + /// This includes values containing "/" or "\", or ending in "." or " " + /// (including "." and ".."), as these are not portable to all platforms. SegmentEncoding segment_encoding = SegmentEncoding::Uri; }; diff --git a/cpp/src/arrow/dataset/partition_test.cc b/cpp/src/arrow/dataset/partition_test.cc index 8dd86b7c2579..35c4991a3efa 100644 --- a/cpp/src/arrow/dataset/partition_test.cc +++ b/cpp/src/arrow/dataset/partition_test.cc @@ -258,12 +258,16 @@ TEST_F(TestPartitioning, DirectoryPartitioningEscapesValues) { AssertFormat(equal(field_ref("part"), literal("ab\\cd")), "ab%5Ccd"); AssertFormat(equal(field_ref("part"), literal("")), "%3Ctest%3E"); AssertFormat(equal(field_ref("part"), literal(":|*")), "%3A%7C%2A"); - // "." is only escaped when it is the first character + // "." is only escaped when it is the first or last character AssertFormat(equal(field_ref("part"), literal("a.b")), "a.b"); AssertFormat(equal(field_ref("part"), literal(".")), "%2E"); - AssertFormat(equal(field_ref("part"), literal("..")), "%2E."); + AssertFormat(equal(field_ref("part"), literal("..")), "%2E%2E"); + AssertFormat(equal(field_ref("part"), literal("...")), "%2E.%2E"); + AssertFormat(equal(field_ref("part"), literal("a.")), "a%2E"); + AssertFormat(equal(field_ref("part"), literal("a ")), "a%20"); AssertFormat(equal(field_ref("part"), literal(".hidden")), "%2Ehidden"); AssertFormat(equal(field_ref("part"), literal("../escaped")), "%2E.%2Fescaped"); + AssertFormat(equal(field_ref("part"), literal("..\\escaped")), "%2E.%5Cescaped"); // "_" is only escaped when it is the first character AssertFormat(equal(field_ref("part"), literal("a_b")), "a_b"); AssertFormat(equal(field_ref("part"), literal("_unknown")), "%5Funknown"); @@ -313,7 +317,7 @@ TEST_F(TestPartitioning, DirectoryPartitioningEscapedValuesRoundTrip) { all_dirs.push_back(dir.full_path); } std::sort(all_dirs.begin(), all_dirs.end()); - std::vector expected_dirs = {"dataset", "dataset/%2E", "dataset/%2E.", + std::vector expected_dirs = {"dataset", "dataset/%2E", "dataset/%2E%2E", "dataset/%2E.%2Fescaped", "dataset/%5Funknown"}; ASSERT_EQ(all_dirs, expected_dirs); @@ -360,6 +364,12 @@ TEST_F(TestPartitioning, DirectoryPartitioningNoSegmentEncoding) { AssertFormatError(equal(field_ref("part"), literal("a/b"))); AssertFormatError(equal(field_ref("part"), literal("."))); AssertFormatError(equal(field_ref("part"), literal(".."))); + // Backslash is a path separator on Windows, so is rejected on all platforms + AssertFormatError(equal(field_ref("part"), literal("..\\escaped"))); + AssertFormatError(equal(field_ref("part"), literal("a\\b"))); + // Windows strips trailing "." and " " from path names + AssertFormatError(equal(field_ref("part"), literal("a."))); + AssertFormatError(equal(field_ref("part"), literal("a "))); } TEST_F(TestPartitioning, FilenamePartitioning) { @@ -696,12 +706,14 @@ TEST_F(TestPartitioning, FilenamePartitioningEscapesValues) { AssertFormat(equal(field_ref("alpha"), literal("ab\\cd")), "", "ab%5Ccd_"); AssertFormat(equal(field_ref("alpha"), literal("")), "", "%3Ctest%3E_"); AssertFormat(equal(field_ref("alpha"), literal(":|*")), "", "%3A%7C%2A_"); - // "." is only escaped when it is the first character of a segment + // "." is only escaped when it is the first or last character of a segment AssertFormat(equal(field_ref("alpha"), literal("a.b")), "", "a.b_"); AssertFormat(equal(field_ref("alpha"), literal(".")), "", "%2E_"); - AssertFormat(equal(field_ref("alpha"), literal("..")), "", "%2E._"); + AssertFormat(equal(field_ref("alpha"), literal("..")), "", "%2E%2E_"); + AssertFormat(equal(field_ref("alpha"), literal("a.")), "", "a%2E_"); AssertFormat(equal(field_ref("alpha"), literal(".hidden")), "", "%2Ehidden_"); AssertFormat(equal(field_ref("alpha"), literal("../escaped")), "", "%2E.%2Fescaped_"); + AssertFormat(equal(field_ref("alpha"), literal("..\\escaped")), "", "%2E.%5Cescaped_"); // "_" is the segment separator, so it is always escaped AssertFormat(equal(field_ref("alpha"), literal("a_b")), "", "a%5Fb_"); AssertFormat(equal(field_ref("alpha"), literal("_unknown")), "", "%5Funknown_"); @@ -736,6 +748,10 @@ TEST_F(TestPartitioning, FilenamePartitioningNoSegmentEncoding) { AssertFormatError(equal(field_ref("alpha"), literal("a/b"))); AssertFormatError(equal(field_ref("alpha"), literal("."))); AssertFormatError(equal(field_ref("alpha"), literal(".."))); + AssertFormatError(equal(field_ref("alpha"), literal("..\\escaped"))); + AssertFormatError(equal(field_ref("alpha"), literal("a\\b"))); + AssertFormatError(equal(field_ref("alpha"), literal("a."))); + AssertFormatError(equal(field_ref("alpha"), literal("a "))); AssertFormatError(and_(equal(field_ref("alpha"), literal("ok")), equal(field_ref("beta"), literal("a/b")))); From c4c896616fc29325621fdd487aa0d4bc8c26d904 Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Fri, 6 Nov 2026 15:46:08 +1300 Subject: [PATCH 05/13] Fix inconsistent error messages --- cpp/src/arrow/dataset/partition.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/dataset/partition.cc b/cpp/src/arrow/dataset/partition.cc index 12a034cebe52..cc69463d49cf 100644 --- a/cpp/src/arrow/dataset/partition.cc +++ b/cpp/src/arrow/dataset/partition.cc @@ -334,8 +334,8 @@ Result> KeyValuePartitioning::FormatPartitionSegments( // from path names, and this also covers the "." and ".." special names. if (value.find_first_of("/\\") != std::string::npos || (!value.empty() && (value.back() == '.' || value.back() == ' '))) { - return Status::Invalid("Partition value for ", schema_->field(i)->name(), - " is not a valid path segment: '", value, + return Status::Invalid("Partition value for '", schema_->field(i)->name(), + "' is not a valid path segment: '", value, "'. Use SegmentEncoding::Uri to write this value."); } // The value can't contain the separator between segments, or it would From a676fbf03cd098457d4f020c6a8a0363d3be6aa9 Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Fri, 6 Nov 2026 15:56:28 +1300 Subject: [PATCH 06/13] Simplify handling of the filename separator --- cpp/src/arrow/dataset/partition.cc | 31 +++++++++++++++++------------- cpp/src/arrow/dataset/partition.h | 11 +++++------ 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/cpp/src/arrow/dataset/partition.cc b/cpp/src/arrow/dataset/partition.cc index cc69463d49cf..0951a895184d 100644 --- a/cpp/src/arrow/dataset/partition.cc +++ b/cpp/src/arrow/dataset/partition.cc @@ -320,7 +320,7 @@ inline std::optional NextValid(const ScalarVector& values, int first_null) } Result> KeyValuePartitioning::FormatPartitionSegments( - const ScalarVector& values, std::optional segment_separator) const { + const ScalarVector& values, bool escape_filename_separator) const { std::vector segments(static_cast(schema_->num_fields())); for (int i = 0; i < schema_->num_fields(); ++i) { @@ -340,11 +340,11 @@ Result> KeyValuePartitioning::FormatPartitionSegments( } // The value can't contain the separator between segments, or it would // not be parsed back correctly - if (segment_separator.has_value() && - value.find(*segment_separator) != std::string::npos) { + if (escape_filename_separator && + value.find(kFilenamePartitionSep) != std::string::npos) { return Status::Invalid("Partition value for '", schema_->field(i)->name(), "' contains the partition separator '", - *segment_separator, "': '", value, + kFilenamePartitionSep, "': '", value, "'. Use SegmentEncoding::Uri to write this value."); } segments[i] = std::move(value); @@ -352,15 +352,19 @@ Result> KeyValuePartitioning::FormatPartitionSegments( } case SegmentEncoding::Uri: { std::string escaped = ::arrow::util::UriEscape(value); - if (segment_separator.has_value()) { - size_t pos = escaped.find(*segment_separator); - if (pos != std::string::npos) { - const std::string encoded_separator = PercentEncode(*segment_separator); - do { - escaped.replace(pos, 1, encoded_separator); - pos = escaped.find(*segment_separator, pos + encoded_separator.size()); - } while (pos != std::string::npos); + if (escape_filename_separator && + escaped.find(kFilenamePartitionSep) != std::string::npos) { + const std::string encoded_separator = PercentEncode(kFilenamePartitionSep); + std::string with_separator; + with_separator.reserve(escaped.size() + 2 * encoded_separator.size()); + for (char c : escaped) { + if (c == kFilenamePartitionSep) { + with_separator += encoded_separator; + } else { + with_separator += c; + } } + escaped = std::move(with_separator); } // A leading "." or "_" isn't normally required to be escaped, but "." and // ".." are not valid directory names, and dataset discovery skips paths @@ -468,7 +472,8 @@ Result DirectoryPartitioning::FormatValues( Result FilenamePartitioning::FormatValues( const ScalarVector& values) const { std::vector segments; - ARROW_ASSIGN_OR_RAISE(segments, FormatPartitionSegments(values, kFilenamePartitionSep)); + ARROW_ASSIGN_OR_RAISE( + segments, FormatPartitionSegments(values, /*escape_filename_separator=*/true)); return PartitionPathFormat{ "", fs::internal::JoinAbstractPath(std::move(segments), kFilenamePartitionSep) + kFilenamePartitionSep}; diff --git a/cpp/src/arrow/dataset/partition.h b/cpp/src/arrow/dataset/partition.h index 2d0f2e6da96f..d5c132baca91 100644 --- a/cpp/src/arrow/dataset/partition.h +++ b/cpp/src/arrow/dataset/partition.h @@ -219,13 +219,12 @@ class ARROW_DS_EXPORT KeyValuePartitioning : public Partitioning { /// options_.segment_encoding. /// /// \param[in] values the partition values for each schema field - /// \param[in] segment_separator if provided, the character that separates - /// segments within a single path component (eg. for filename partitioning). - /// It is escaped when using SegmentEncoding::Uri, and values containing it - /// result in an error when using SegmentEncoding::None. + /// \param[in] escape_filename_separator whether values must not contain the + /// filename partitioning separator ("_"). It is escaped when using + /// SegmentEncoding::Uri, and values containing it result in an error when + /// using SegmentEncoding::None. Result> FormatPartitionSegments( - const ScalarVector& values, - std::optional segment_separator = std::nullopt) const; + const ScalarVector& values, bool escape_filename_separator = false) const; Result> ParsePartitionSegments( const std::vector& segments) const; From c85f7e7a2705577d922d4b6cdab7c672094dfa0c Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Fri, 6 Nov 2026 16:06:17 +1300 Subject: [PATCH 07/13] Allow trailing space or period --- cpp/src/arrow/dataset/partition.cc | 8 ++++---- cpp/src/arrow/dataset/partition.h | 7 ++++--- cpp/src/arrow/dataset/partition_test.cc | 10 +++++----- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/cpp/src/arrow/dataset/partition.cc b/cpp/src/arrow/dataset/partition.cc index 0951a895184d..ba6b4cdd6cad 100644 --- a/cpp/src/arrow/dataset/partition.cc +++ b/cpp/src/arrow/dataset/partition.cc @@ -330,10 +330,10 @@ Result> KeyValuePartitioning::FormatPartitionSegments( case SegmentEncoding::None: { // Without encoding, it's an error to write values that are not a valid path // segment. Backslashes are rejected on all platforms as they are a path - // separator on Windows. Windows also strips trailing "." and " " characters - // from path names, and this also covers the "." and ".." special names. - if (value.find_first_of("/\\") != std::string::npos || - (!value.empty() && (value.back() == '.' || value.back() == ' '))) { + // separator on Windows. Other values that are only invalid on some + // platforms (eg. ending in "." on Windows) are not rejected. + if (value.find_first_of("/\\") != std::string::npos || value == "." || + value == "..") { return Status::Invalid("Partition value for '", schema_->field(i)->name(), "' is not a valid path segment: '", value, "'. Use SegmentEncoding::Uri to write this value."); diff --git a/cpp/src/arrow/dataset/partition.h b/cpp/src/arrow/dataset/partition.h index d5c132baca91..8c7769231f07 100644 --- a/cpp/src/arrow/dataset/partition.h +++ b/cpp/src/arrow/dataset/partition.h @@ -121,9 +121,10 @@ struct ARROW_DS_EXPORT KeyValuePartitioningOptions { /// /// For directory and filename partitioning, partition values are also encoded /// according to this scheme when formatting paths for writing. With - /// SegmentEncoding::None, values that are not valid path segments raise errors. - /// This includes values containing "/" or "\", or ending in "." or " " - /// (including "." and ".."), as these are not portable to all platforms. + /// SegmentEncoding::None, values are written as-is, and values containing "/" + /// or "\", or equal to "." or "..", raise errors. Other values may not be valid + /// path segments on all platforms, eg. on Windows, values ending in "." or " " + /// are not written correctly. SegmentEncoding segment_encoding = SegmentEncoding::Uri; }; diff --git a/cpp/src/arrow/dataset/partition_test.cc b/cpp/src/arrow/dataset/partition_test.cc index 35c4991a3efa..755663a59405 100644 --- a/cpp/src/arrow/dataset/partition_test.cc +++ b/cpp/src/arrow/dataset/partition_test.cc @@ -358,6 +358,9 @@ TEST_F(TestPartitioning, DirectoryPartitioningNoSegmentEncoding) { AssertFormat(equal(field_ref("part"), literal("")), ""); AssertFormat(equal(field_ref("part"), literal("a_b")), "a_b"); AssertFormat(equal(field_ref("part"), literal("_unknown")), "_unknown"); + // Values that aren't portable to all platforms are allowed + AssertFormat(equal(field_ref("part"), literal("a.")), "a."); + AssertFormat(equal(field_ref("part"), literal("a ")), "a "); // Values that aren't a single valid path segment are rejected AssertFormatError(equal(field_ref("part"), literal("../escaped"))); @@ -367,9 +370,6 @@ TEST_F(TestPartitioning, DirectoryPartitioningNoSegmentEncoding) { // Backslash is a path separator on Windows, so is rejected on all platforms AssertFormatError(equal(field_ref("part"), literal("..\\escaped"))); AssertFormatError(equal(field_ref("part"), literal("a\\b"))); - // Windows strips trailing "." and " " from path names - AssertFormatError(equal(field_ref("part"), literal("a."))); - AssertFormatError(equal(field_ref("part"), literal("a "))); } TEST_F(TestPartitioning, FilenamePartitioning) { @@ -742,6 +742,8 @@ TEST_F(TestPartitioning, FilenamePartitioningNoSegmentEncoding) { AssertFormat(and_(equal(field_ref("alpha"), literal("a b")), equal(field_ref("beta"), literal("c.d"))), "", "a b_c.d_"); + // Values that aren't portable to all platforms are allowed + AssertFormat(equal(field_ref("alpha"), literal("a.")), "", "a._"); // Values that aren't a single valid path segment are rejected AssertFormatError(equal(field_ref("alpha"), literal("../escaped"))); @@ -750,8 +752,6 @@ TEST_F(TestPartitioning, FilenamePartitioningNoSegmentEncoding) { AssertFormatError(equal(field_ref("alpha"), literal(".."))); AssertFormatError(equal(field_ref("alpha"), literal("..\\escaped"))); AssertFormatError(equal(field_ref("alpha"), literal("a\\b"))); - AssertFormatError(equal(field_ref("alpha"), literal("a."))); - AssertFormatError(equal(field_ref("alpha"), literal("a "))); AssertFormatError(and_(equal(field_ref("alpha"), literal("ok")), equal(field_ref("beta"), literal("a/b")))); From db4e2e848db3b11a8a993e9bb6b3968de9988a7c Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Fri, 6 Nov 2026 16:20:46 +1300 Subject: [PATCH 08/13] Disallow empty values with directory and filename partitioning --- cpp/src/arrow/dataset/partition.cc | 7 +++++++ cpp/src/arrow/dataset/partition.h | 3 ++- cpp/src/arrow/dataset/partition_test.cc | 27 +++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/dataset/partition.cc b/cpp/src/arrow/dataset/partition.cc index ba6b4cdd6cad..aa8c2740649d 100644 --- a/cpp/src/arrow/dataset/partition.cc +++ b/cpp/src/arrow/dataset/partition.cc @@ -326,6 +326,13 @@ Result> KeyValuePartitioning::FormatPartitionSegments( for (int i = 0; i < schema_->num_fields(); ++i) { if (values[i] != nullptr && values[i]->is_valid) { std::string value = values[i]->ToString(); + // Empty segments are dropped when joining path segments, so an empty value + // would not be parsed back correctly + if (value.empty()) { + return Status::Invalid("Partition value for '", schema_->field(i)->name(), + "' is empty, which is not supported with ", type_name(), + " partitioning"); + } switch (options_.segment_encoding) { case SegmentEncoding::None: { // Without encoding, it's an error to write values that are not a valid path diff --git a/cpp/src/arrow/dataset/partition.h b/cpp/src/arrow/dataset/partition.h index 8c7769231f07..91b5a9d09605 100644 --- a/cpp/src/arrow/dataset/partition.h +++ b/cpp/src/arrow/dataset/partition.h @@ -124,7 +124,8 @@ struct ARROW_DS_EXPORT KeyValuePartitioningOptions { /// SegmentEncoding::None, values are written as-is, and values containing "/" /// or "\", or equal to "." or "..", raise errors. Other values may not be valid /// path segments on all platforms, eg. on Windows, values ending in "." or " " - /// are not written correctly. + /// are not written correctly. For directory and filename partitioning, empty + /// values raise errors with either encoding. SegmentEncoding segment_encoding = SegmentEncoding::Uri; }; diff --git a/cpp/src/arrow/dataset/partition_test.cc b/cpp/src/arrow/dataset/partition_test.cc index 755663a59405..f03f21d7709d 100644 --- a/cpp/src/arrow/dataset/partition_test.cc +++ b/cpp/src/arrow/dataset/partition_test.cc @@ -272,6 +272,8 @@ TEST_F(TestPartitioning, DirectoryPartitioningEscapesValues) { AssertFormat(equal(field_ref("part"), literal("a_b")), "a_b"); AssertFormat(equal(field_ref("part"), literal("_unknown")), "%5Funknown"); AssertFormat(equal(field_ref("part"), literal("__x_")), "%5F_x_"); + // Empty values would be dropped from the path, so are rejected + AssertFormatError(equal(field_ref("part"), literal(""))); } TEST_F(TestPartitioning, DirectoryPartitioningEscapedValuesRoundTrip) { @@ -370,6 +372,8 @@ TEST_F(TestPartitioning, DirectoryPartitioningNoSegmentEncoding) { // Backslash is a path separator on Windows, so is rejected on all platforms AssertFormatError(equal(field_ref("part"), literal("..\\escaped"))); AssertFormatError(equal(field_ref("part"), literal("a\\b"))); + // Empty values would be dropped from the path, so are rejected + AssertFormatError(equal(field_ref("part"), literal(""))); } TEST_F(TestPartitioning, FilenamePartitioning) { @@ -680,6 +684,18 @@ TEST_F(TestPartitioning, HivePartitioningFormat) { partitioning_ = std::make_shared(schema({field("x", large_utf8())})); AssertFormat(equal(field_ref("x"), literal("hello")), "x=hello"); + + // Empty string values round-trip + partitioning_ = std::make_shared( + schema({field("alpha", utf8()), field("beta", utf8())})); + written_schema_ = partitioning_->schema(); + AssertFormat(equal(field_ref("alpha"), literal("")), "alpha="); + AssertFormat(and_(equal(field_ref("alpha"), literal("")), + equal(field_ref("beta"), literal("x"))), + "alpha=/beta=x"); + AssertFormat(and_(equal(field_ref("alpha"), literal("x")), + equal(field_ref("beta"), literal(""))), + "alpha=x/beta="); } TEST_F(TestPartitioning, FilenamePartitioningFormat) { @@ -725,6 +741,12 @@ TEST_F(TestPartitioning, FilenamePartitioningEscapesValues) { AssertFormat(and_(equal(field_ref("alpha"), literal("a_b")), equal(field_ref("beta"), literal("c_d"))), "", "a%5Fb_c%5Fd_"); + // Empty values would be dropped from the filename, so are rejected + AssertFormatError(equal(field_ref("alpha"), literal(""))); + AssertFormatError(and_(equal(field_ref("alpha"), literal("")), + equal(field_ref("beta"), literal("x")))); + AssertFormatError(and_(equal(field_ref("alpha"), literal("x")), + equal(field_ref("beta"), literal("")))); } TEST_F(TestPartitioning, FilenamePartitioningNoSegmentEncoding) { @@ -760,6 +782,11 @@ TEST_F(TestPartitioning, FilenamePartitioningNoSegmentEncoding) { AssertFormatError(equal(field_ref("alpha"), literal("_unknown"))); AssertFormatError(and_(equal(field_ref("alpha"), literal("ok")), equal(field_ref("beta"), literal("c_d")))); + + // Empty values would be dropped from the filename, so are rejected + AssertFormatError(equal(field_ref("alpha"), literal(""))); + AssertFormatError(and_(equal(field_ref("alpha"), literal("x")), + equal(field_ref("beta"), literal("")))); } TEST_F(TestPartitioning, DiscoverHiveSchema) { From 5ff121f75fb7969c5bec8ae93cf39d7e5a087869 Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Fri, 6 Nov 2026 16:26:27 +1300 Subject: [PATCH 09/13] Document and test handling of values starting with "." or "_" --- cpp/src/arrow/dataset/partition.h | 10 ++++++---- cpp/src/arrow/dataset/partition_test.cc | 10 ++++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/dataset/partition.h b/cpp/src/arrow/dataset/partition.h index 91b5a9d09605..2748c70e3c71 100644 --- a/cpp/src/arrow/dataset/partition.h +++ b/cpp/src/arrow/dataset/partition.h @@ -122,10 +122,12 @@ struct ARROW_DS_EXPORT KeyValuePartitioningOptions { /// For directory and filename partitioning, partition values are also encoded /// according to this scheme when formatting paths for writing. With /// SegmentEncoding::None, values are written as-is, and values containing "/" - /// or "\", or equal to "." or "..", raise errors. Other values may not be valid - /// path segments on all platforms, eg. on Windows, values ending in "." or " " - /// are not written correctly. For directory and filename partitioning, empty - /// values raise errors with either encoding. + /// or "\", or equal to "." or "..", raise errors. Values starting with "." or + /// "_" are written, but are skipped by dataset discovery unless + /// selector_ignore_prefixes is changed. Other values may not be valid path + /// segments on all platforms, eg. on Windows, values ending in "." or " " are + /// not written correctly. For directory and filename partitioning, empty values + /// raise errors with either encoding. SegmentEncoding segment_encoding = SegmentEncoding::Uri; }; diff --git a/cpp/src/arrow/dataset/partition_test.cc b/cpp/src/arrow/dataset/partition_test.cc index f03f21d7709d..12a7345df69a 100644 --- a/cpp/src/arrow/dataset/partition_test.cc +++ b/cpp/src/arrow/dataset/partition_test.cc @@ -359,10 +359,14 @@ TEST_F(TestPartitioning, DirectoryPartitioningNoSegmentEncoding) { AssertFormat(equal(field_ref("part"), literal("a%20b")), "a%20b"); AssertFormat(equal(field_ref("part"), literal("")), ""); AssertFormat(equal(field_ref("part"), literal("a_b")), "a_b"); - AssertFormat(equal(field_ref("part"), literal("_unknown")), "_unknown"); + AssertFormat(equal(field_ref("part"), literal("a.b")), "a.b"); // Values that aren't portable to all platforms are allowed AssertFormat(equal(field_ref("part"), literal("a.")), "a."); AssertFormat(equal(field_ref("part"), literal("a ")), "a "); + // Values starting with "." or "_" are allowed, although they are skipped by + // dataset discovery with the default selector_ignore_prefixes + AssertFormat(equal(field_ref("part"), literal(".hidden")), ".hidden"); + AssertFormat(equal(field_ref("part"), literal("_unknown")), "_unknown"); // Values that aren't a single valid path segment are rejected AssertFormatError(equal(field_ref("part"), literal("../escaped"))); @@ -760,12 +764,14 @@ TEST_F(TestPartitioning, FilenamePartitioningNoSegmentEncoding) { AssertFormat(equal(field_ref("alpha"), literal("a b")), "", "a b_"); AssertFormat(equal(field_ref("alpha"), literal("a%20b")), "", "a%20b_"); AssertFormat(equal(field_ref("alpha"), literal("")), "", "_"); - AssertFormat(equal(field_ref("alpha"), literal(".hidden")), "", ".hidden_"); AssertFormat(and_(equal(field_ref("alpha"), literal("a b")), equal(field_ref("beta"), literal("c.d"))), "", "a b_c.d_"); // Values that aren't portable to all platforms are allowed AssertFormat(equal(field_ref("alpha"), literal("a.")), "", "a._"); + // Values starting with "." are allowed, although they are skipped by dataset + // discovery with the default selector_ignore_prefixes + AssertFormat(equal(field_ref("alpha"), literal(".hidden")), "", ".hidden_"); // Values that aren't a single valid path segment are rejected AssertFormatError(equal(field_ref("alpha"), literal("../escaped"))); From 749c7ef54c2605b33057373a82cc32c2a003e7c7 Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Fri, 6 Nov 2026 16:35:24 +1300 Subject: [PATCH 10/13] Update expected results in Python tests --- python/pyarrow/tests/test_dataset.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/pyarrow/tests/test_dataset.py b/python/pyarrow/tests/test_dataset.py index b50441a1ae05..3b575e24d5e1 100644 --- a/python/pyarrow/tests/test_dataset.py +++ b/python/pyarrow/tests/test_dataset.py @@ -853,8 +853,8 @@ def test_partitioning_pickling(pickle_module): "flavor, expected_defined_partition, expected_undefined_partition", [ ("HivePartitioning", (r"foo=A/bar=ant%20bee", ""), ("", "")), - ("DirectoryPartitioning", (r"A/ant bee", ""), ("", "")), - ("FilenamePartitioning", ("", r"A_ant bee_"), ("", "_")), + ("DirectoryPartitioning", (r"A/ant%20bee", ""), ("", "")), + ("FilenamePartitioning", ("", r"A_ant%20bee_"), ("", "_")), ], ) def test_dataset_partitioning_format( From c3959a6aeace0f19e4c83275f4eba65d9e5a088e Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Mon, 9 Nov 2026 14:24:08 +1300 Subject: [PATCH 11/13] Update Python docs on dataset and filename partitioning --- python/pyarrow/_dataset.pyx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/python/pyarrow/_dataset.pyx b/python/pyarrow/_dataset.pyx index d40614a61fc0..8889090801c8 100644 --- a/python/pyarrow/_dataset.pyx +++ b/python/pyarrow/_dataset.pyx @@ -2758,8 +2758,8 @@ cdef class DirectoryPartitioning(KeyValuePartitioning): every value which may be taken by the corresponding column or an error will be raised in parsing. segment_encoding : str, default "uri" - After splitting paths into segments, decode the segments. Valid - values are "uri" (URI-decode segments) and "none" (leave as-is). + Method to use to encode and decode path segments. Valid values are + "uri" (URI-encode and decode segments) and "none" (leave as-is). Returns ------- @@ -3037,8 +3037,8 @@ cdef class FilenamePartitioning(KeyValuePartitioning): every value which may be taken by the corresponding column or an error will be raised in parsing. segment_encoding : str, default "uri" - After splitting paths into segments, decode the segments. Valid - values are "uri" (URI-decode segments) and "none" (leave as-is). + Method to use to encode and decode path segments. Valid values are + "uri" (URI-encode and decode segments) and "none" (leave as-is). Returns ------- From a14ba14101bd31e73c94ac781b39ab56c3a543ff Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Tue, 10 Nov 2026 10:49:57 +1300 Subject: [PATCH 12/13] Remove confusing string reserve --- cpp/src/arrow/dataset/partition.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/src/arrow/dataset/partition.cc b/cpp/src/arrow/dataset/partition.cc index aa8c2740649d..a2aa58565e80 100644 --- a/cpp/src/arrow/dataset/partition.cc +++ b/cpp/src/arrow/dataset/partition.cc @@ -363,7 +363,6 @@ Result> KeyValuePartitioning::FormatPartitionSegments( escaped.find(kFilenamePartitionSep) != std::string::npos) { const std::string encoded_separator = PercentEncode(kFilenamePartitionSep); std::string with_separator; - with_separator.reserve(escaped.size() + 2 * encoded_separator.size()); for (char c : escaped) { if (c == kFilenamePartitionSep) { with_separator += encoded_separator; From f1d84048110f075bc8159775f43ffc3c85c69248 Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Tue, 10 Nov 2026 10:56:33 +1300 Subject: [PATCH 13/13] Improve doc comment wording for escape_filename_separator --- cpp/src/arrow/dataset/partition.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/dataset/partition.h b/cpp/src/arrow/dataset/partition.h index 2748c70e3c71..0c36fdbbf5e8 100644 --- a/cpp/src/arrow/dataset/partition.h +++ b/cpp/src/arrow/dataset/partition.h @@ -223,10 +223,9 @@ class ARROW_DS_EXPORT KeyValuePartitioning : public Partitioning { /// options_.segment_encoding. /// /// \param[in] values the partition values for each schema field - /// \param[in] escape_filename_separator whether values must not contain the - /// filename partitioning separator ("_"). It is escaped when using - /// SegmentEncoding::Uri, and values containing it result in an error when - /// using SegmentEncoding::None. + /// \param[in] escape_filename_separator whether to escape the + /// filename partitioning separator ("_"). If no encoding is used + /// (SegmentEncoding::None), values containing the separator result in an error. Result> FormatPartitionSegments( const ScalarVector& values, bool escape_filename_separator = false) const;