You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by uw...@apache.org on 2018/05/02 05:02:35 UTC
[arrow] branch master updated: ARROW-2466: [C++] Fix "append" flag
to FileOutputStream
This is an automated email from the ASF dual-hosted git repository.
uwe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new 4cc9157 ARROW-2466: [C++] Fix "append" flag to FileOutputStream
4cc9157 is described below
commit 4cc9157b9742d31fc31f77ed912b8680f859f6c2
Author: Antoine Pitrou <an...@python.org>
AuthorDate: Wed May 2 07:02:28 2018 +0200
ARROW-2466: [C++] Fix "append" flag to FileOutputStream
The "append" flag only meant "don't truncate", and would write at the start of the file.
Author: Antoine Pitrou <an...@python.org>
Closes #1978 from pitrou/ARROW-2466-file-output-stream-append and squashes the following commits:
4147bb1d <Antoine Pitrou> ARROW-2466: Fix "append" flag to FileOutputStream
---
cpp/src/arrow/io/file.cc | 16 ++++++++++------
cpp/src/arrow/io/file.h | 4 ++--
cpp/src/arrow/io/io-file-test.cc | 41 +++++++++++++++++++++++++++++++++++++---
cpp/src/arrow/util/io-util.cc | 8 +++++++-
cpp/src/arrow/util/io-util.h | 2 +-
5 files changed, 58 insertions(+), 13 deletions(-)
diff --git a/cpp/src/arrow/io/file.cc b/cpp/src/arrow/io/file.cc
index e3d6f84..8113482 100644
--- a/cpp/src/arrow/io/file.cc
+++ b/cpp/src/arrow/io/file.cc
@@ -61,14 +61,16 @@ class OSFile {
// Note: only one of the Open* methods below may be called on a given instance
- Status OpenWriteable(const std::string& path, bool append, bool write_only) {
+ Status OpenWriteable(const std::string& path, bool truncate, bool append,
+ bool write_only) {
RETURN_NOT_OK(SetFileName(path));
- RETURN_NOT_OK(internal::FileOpenWriteable(file_name_, write_only, !append, &fd_));
+ RETURN_NOT_OK(
+ internal::FileOpenWriteable(file_name_, write_only, truncate, append, &fd_));
is_open_ = true;
mode_ = write_only ? FileMode::WRITE : FileMode::READWRITE;
- if (append) {
+ if (!truncate) {
RETURN_NOT_OK(internal::FileGetSize(fd_, &size_));
} else {
size_ = 0;
@@ -284,7 +286,8 @@ int ReadableFile::file_descriptor() const { return impl_->fd(); }
class FileOutputStream::FileOutputStreamImpl : public OSFile {
public:
Status Open(const std::string& path, bool append) {
- return OpenWriteable(path, append, true /* write_only */);
+ const bool truncate = !append;
+ return OpenWriteable(path, truncate, append, true /* write_only */);
}
Status Open(int fd) { return OpenWriteable(fd); }
};
@@ -363,9 +366,10 @@ class MemoryMappedFile::MemoryMap : public MutableBuffer {
// Memory mapping has permission failures if PROT_READ not set
prot_flags = PROT_READ | PROT_WRITE;
map_mode = MAP_SHARED;
- constexpr bool append = true;
+ constexpr bool append = false;
+ constexpr bool truncate = false;
constexpr bool write_only = false;
- RETURN_NOT_OK(file_->OpenWriteable(path, append, write_only));
+ RETURN_NOT_OK(file_->OpenWriteable(path, truncate, append, write_only));
is_mutable_ = true;
} else {
diff --git a/cpp/src/arrow/io/file.h b/cpp/src/arrow/io/file.h
index c2572da..7020cab 100644
--- a/cpp/src/arrow/io/file.h
+++ b/cpp/src/arrow/io/file.h
@@ -44,7 +44,7 @@ class ARROW_EXPORT FileOutputStream : public OutputStream {
/// \param[out] out a base interface OutputStream instance
///
/// When opening a new file, any existing file with the indicated path is
- /// truncated to 0 bytes, deleting any existing memory
+ /// truncated to 0 bytes, deleting any existing data
static Status Open(const std::string& path, std::shared_ptr<OutputStream>* out);
/// \brief Open a local file for writing
@@ -68,7 +68,7 @@ class ARROW_EXPORT FileOutputStream : public OutputStream {
/// \param[out] file a FileOutputStream instance
///
/// When opening a new file, any existing file with the indicated path is
- /// truncated to 0 bytes, deleting any existing memory
+ /// truncated to 0 bytes, deleting any existing data
static Status Open(const std::string& path, std::shared_ptr<FileOutputStream>* file);
/// \brief Open a local file for writing
diff --git a/cpp/src/arrow/io/io-file-test.cc b/cpp/src/arrow/io/io-file-test.cc
index 40e24aa..4d9120f 100644
--- a/cpp/src/arrow/io/io-file-test.cc
+++ b/cpp/src/arrow/io/io-file-test.cc
@@ -74,10 +74,12 @@ class TestFileOutputStream : public FileTestFixture {
ASSERT_OK(internal::FileNameFromString(path_, &file_name));
int fd_file, fd_stream;
ASSERT_OK(internal::FileOpenWriteable(file_name, true /* write_only */,
- false /* truncate */, &fd_file));
+ false /* truncate */, false /* append */,
+ &fd_file));
ASSERT_OK(FileOutputStream::Open(fd_file, &file_));
ASSERT_OK(internal::FileOpenWriteable(file_name, true /* write_only */,
- false /* truncate */, &fd_stream));
+ false /* truncate */, false /* append */,
+ &fd_stream));
ASSERT_OK(FileOutputStream::Open(fd_stream, &stream_));
}
@@ -168,7 +170,7 @@ TEST_F(TestFileOutputStream, FromFileDescriptor) {
internal::PlatformFilename file_name;
ASSERT_OK(internal::FileNameFromString(path_, &file_name));
ASSERT_OK(internal::FileOpenWriteable(file_name, true /* write_only */,
- false /* truncate */, &fd));
+ false /* truncate */, false /* append */, &fd));
ASSERT_OK(internal::FileSeek(fd, 0, SEEK_END));
ASSERT_OK(FileOutputStream::Open(fd, &stream_));
@@ -220,6 +222,7 @@ TEST_F(TestFileOutputStream, TruncatesNewFile) {
AssertFileContents(path_, "");
+ // Same with stream-returning API
ASSERT_OK(FileOutputStream::Open(path_, &stream_));
ASSERT_OK(stream_->Write(data, strlen(data)));
@@ -231,6 +234,38 @@ TEST_F(TestFileOutputStream, TruncatesNewFile) {
AssertFileContents(path_, "");
}
+TEST_F(TestFileOutputStream, Append) {
+ ASSERT_OK(FileOutputStream::Open(path_, &file_));
+ {
+ const char* data = "test";
+ ASSERT_OK(file_->Write(data, strlen(data)));
+ }
+ ASSERT_OK(file_->Close());
+ ASSERT_OK(FileOutputStream::Open(path_, true /* append */, &file_));
+ {
+ const char* data = "data";
+ ASSERT_OK(file_->Write(data, strlen(data)));
+ }
+ ASSERT_OK(file_->Close());
+ AssertFileContents(path_, "testdata");
+
+ // Same with stream-returning API
+ ASSERT_OK(FileOutputStream::Open(path_, &stream_));
+ {
+ const char* data = "test";
+ ASSERT_OK(stream_->Write(data, strlen(data)));
+ }
+ ASSERT_OK(stream_->Close());
+
+ ASSERT_OK(FileOutputStream::Open(path_, true /* append */, &stream_));
+ {
+ const char* data = "data";
+ ASSERT_OK(stream_->Write(data, strlen(data)));
+ }
+ ASSERT_OK(stream_->Close());
+ AssertFileContents(path_, "testdata");
+}
+
// ----------------------------------------------------------------------
// File input tests
diff --git a/cpp/src/arrow/util/io-util.cc b/cpp/src/arrow/util/io-util.cc
index 03edc18..ce989f4 100644
--- a/cpp/src/arrow/util/io-util.cc
+++ b/cpp/src/arrow/util/io-util.cc
@@ -149,7 +149,7 @@ Status FileOpenReadable(const PlatformFilename& file_name, int* fd) {
}
Status FileOpenWriteable(const PlatformFilename& file_name, bool write_only,
- bool truncate, int* fd) {
+ bool truncate, bool append, int* fd) {
int ret, errno_actual;
#if defined(_MSC_VER)
@@ -162,6 +162,9 @@ Status FileOpenWriteable(const PlatformFilename& file_name, bool write_only,
if (truncate) {
oflag |= _O_TRUNC;
}
+ if (append) {
+ oflag |= _O_APPEND;
+ }
if (write_only) {
oflag |= _O_WRONLY;
@@ -178,6 +181,9 @@ Status FileOpenWriteable(const PlatformFilename& file_name, bool write_only,
if (truncate) {
oflag |= O_TRUNC;
}
+ if (append) {
+ oflag |= O_APPEND;
+ }
if (write_only) {
oflag |= O_WRONLY;
diff --git a/cpp/src/arrow/util/io-util.h b/cpp/src/arrow/util/io-util.h
index e857490..a0c66e9 100644
--- a/cpp/src/arrow/util/io-util.h
+++ b/cpp/src/arrow/util/io-util.h
@@ -146,7 +146,7 @@ Status FileNameFromString(const std::string& file_name, PlatformFilename* out);
Status FileOpenReadable(const PlatformFilename& file_name, int* fd);
Status FileOpenWriteable(const PlatformFilename& file_name, bool write_only,
- bool truncate, int* fd);
+ bool truncate, bool append, int* fd);
Status FileRead(int fd, uint8_t* buffer, const int64_t nbytes, int64_t* bytes_read);
Status FileReadAt(int fd, uint8_t* buffer, int64_t position, int64_t nbytes,
--
To stop receiving notification emails like this one, please contact
uwe@apache.org.