jorisvandenbossche commented on a change in pull request #10955:
URL: https://github.com/apache/arrow/pull/10955#discussion_r718478927
##########
File path: cpp/src/arrow/dataset/file_base.h
##########
@@ -343,6 +343,18 @@ class ARROW_DS_EXPORT FileWriter {
fs::FileLocator destination_locator_;
};
+/// \brief Controls what happens if files exist in an output directory during
a dataset
+/// write
+enum ExistingDataBehavior : int8_t {
+ /// Deletes all files in a directory the first time that directory is
encountered
+ kDeleteMatchingPartitions,
+ /// Ignores existing files, overwriting any that happen to have the same
name as an
+ /// output file
+ kOverwriteOrIgnore,
+ /// Returns an error if there are any files or subdirectories in the output
directory
+ kError,
Review comment:
I think it could be useful behaviour? So then yes I can create a JIRA
for it.
> If we check for subdirectories we either have to check on the fly or do
two passes.
> If we check on the fly then it could lead to case where data is partially
written when an error occurs.
What I don't fully understand is why it is not possible to check that
efficiently on the fly in this case, while the `kDeleteMatchingPartitions` also
needs to check if a directory already exists to delete its content if necessary
(and also needs to do it only the first time when writing, and not for
subsequent files to that directory)
##########
File path: cpp/src/arrow/dataset/file_base.h
##########
@@ -343,6 +343,18 @@ class ARROW_DS_EXPORT FileWriter {
fs::FileLocator destination_locator_;
};
+/// \brief Controls what happens if files exist in an output directory during
a dataset
+/// write
+enum ExistingDataBehavior : int8_t {
+ /// Deletes all files in a directory the first time that directory is
encountered
+ kDeleteMatchingPartitions,
+ /// Ignores existing files, overwriting any that happen to have the same
name as an
+ /// output file
+ kOverwriteOrIgnore,
+ /// Returns an error if there are any files or subdirectories in the output
directory
+ kError,
Review comment:
Ah, because `kDeleteMatchingPartitions` deletes data and doesn't abort
the operation. While for erroring in this case, you could have already written
a file to a partition that didn't exist yet, before writing a file to an
existing partition. In which case you indeed expect to roll back the part that
already was written.
##########
File path: cpp/src/arrow/dataset/file_base.h
##########
@@ -364,6 +376,18 @@ struct ARROW_DS_EXPORT FileSystemDatasetWriteOptions {
/// {i} will be replaced by an auto incremented integer.
std::string basename_template;
+ /// If greater than 0 then this will limit the maximum number of files that
can be left
+ /// open. If an attempt is made to open too many files then the least
recently used file
+ /// will be closed. If this setting is set too low you may end up
fragmenting your data
+ /// into many small files.
+ uint32_t max_open_files = 1024;
+
+ /// If greater than 0 then this will limit how many rows are placed in any
single file.
Review comment:
Thanks for the update. That's clear now.
(I am wondering a bit whether this is actually a good default, or if we
shouldn't put some default max file size. Although that probably depends a lot
on your use case / whether the file format can easily handle large files, in
which case it might be better to explicitly leave it up to the user)
##########
File path: cpp/src/arrow/dataset/file_base.h
##########
@@ -343,6 +343,18 @@ class ARROW_DS_EXPORT FileWriter {
fs::FileLocator destination_locator_;
};
+/// \brief Controls what happens if files exist in an output directory during
a dataset
+/// write
+enum ExistingDataBehavior : int8_t {
+ /// Deletes all files in a directory the first time that directory is
encountered
+ kDeleteMatchingPartitions,
+ /// Ignores existing files, overwriting any that happen to have the same
name as an
+ /// output file
+ kOverwriteOrIgnore,
+ /// Returns an error if there are any files or subdirectories in the output
directory
+ kError,
Review comment:
(and you actually also already clearly explained that in August ..
https://github.com/apache/arrow/pull/10955#discussion_r694423159 Sorry! :))
--
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]
