adamreeve commented on code in PR #55423:
URL: https://github.com/apache/arrow/pull/55423#discussion_r4518548626
##########
cpp/src/arrow/dataset/partition.cc:
##########
@@ -316,12 +320,77 @@ inline std::optional<int> NextValid(const ScalarVector&
values, int first_null)
}
Result<std::vector<std::string>> KeyValuePartitioning::FormatPartitionSegments(
- const ScalarVector& values) const {
+ const ScalarVector& values, bool escape_filename_separator) const {
std::vector<std::string>
segments(static_cast<size_t>(schema_->num_fields()));
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();
+ // 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
+ // segment. Backslashes are rejected on all platforms as they are a
path
+ // 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.");
+ }
+ // The value can't contain the separator between segments, or it
would
+ // not be parsed back correctly
+ if (escape_filename_separator &&
+ value.find(kFilenamePartitionSep) != std::string::npos) {
+ return Status::Invalid("Partition value for '",
schema_->field(i)->name(),
+ "' contains the partition separator '",
+ kFilenamePartitionSep, "': '", value,
+ "'. Use SegmentEncoding::Uri to write this
value.");
+ }
+ segments[i] = std::move(value);
+ break;
+ }
+ case SegmentEncoding::Uri: {
+ std::string escaped = ::arrow::util::UriEscape(value);
+ 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());
Review Comment:
I've removed this line in a14ba14101bd31e73c94ac781b39ab56c3a543ff
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
