parquet-cpp git commit: PARQUET-537: Ensure that LocalFileSource is properly closed.

[ju...@apache.org]

Repository: parquet-cpp
Updated Branches:
  refs/heads/master eab9a7342 -> 46e40e20e


PARQUET-537: Ensure that LocalFileSource is properly closed.

Author: Aliaksei Sandryhaila <al...@hp.com>

Closes #68 from asandryh/PARQUET-537 and squashes the following commits:

18dca87 [Aliaksei Sandryhaila] Added a unit test.
dfb7a0b [Aliaksei Sandryhaila] PARQUET-537: Ensure that LocalFileSource is properly closed.


Project: http://git-wip-us.apache.org/repos/asf/parquet-cpp/repo
Commit: http://git-wip-us.apache.org/repos/asf/parquet-cpp/commit/46e40e20
Tree: http://git-wip-us.apache.org/repos/asf/parquet-cpp/tree/46e40e20
Diff: http://git-wip-us.apache.org/repos/asf/parquet-cpp/diff/46e40e20

Branch: refs/heads/master
Commit: 46e40e20e7afa1fe493ea927d5a9f689be71dbf8
Parents: eab9a73
Author: Aliaksei Sandryhaila <al...@hp.com>
Authored: Tue Mar 1 17:33:51 2016 -0800
Committer: Julien Le Dem <ju...@dremio.com>
Committed: Tue Mar 1 17:33:51 2016 -0800

----------------------------------------------------------------------
 src/parquet/file/reader-internal.cc |  4 ++++
 src/parquet/file/reader-internal.h  |  1 +
 src/parquet/file/reader.cc          |  8 ++++++--
 src/parquet/reader-test.cc          | 35 +++++++++++++++++++++++++++++++-
 4 files changed, 45 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/46e40e20/src/parquet/file/reader-internal.cc
----------------------------------------------------------------------
diff --git a/src/parquet/file/reader-internal.cc b/src/parquet/file/reader-internal.cc
index 3d8c373..c571c72 100644
--- a/src/parquet/file/reader-internal.cc
+++ b/src/parquet/file/reader-internal.cc
@@ -215,6 +215,10 @@ void SerializedFile::Close() {
   source_->Close();
 }
 
+SerializedFile::~SerializedFile() {
+  Close();
+}
+
 std::shared_ptr<RowGroupReader> SerializedFile::GetRowGroup(int i) {
   std::unique_ptr<SerializedRowGroup> contents(new SerializedRowGroup(source_.get(),
           &metadata_.row_groups[i]));

http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/46e40e20/src/parquet/file/reader-internal.h
----------------------------------------------------------------------
diff --git a/src/parquet/file/reader-internal.h b/src/parquet/file/reader-internal.h
index 08d4607..a398cb3 100644
--- a/src/parquet/file/reader-internal.h
+++ b/src/parquet/file/reader-internal.h
@@ -100,6 +100,7 @@ class SerializedFile : public ParquetFileReader::Contents {
   virtual int64_t num_rows() const;
   virtual int num_columns() const;
   virtual int num_row_groups() const;
+  virtual ~SerializedFile();
 
  private:
   // This class takes ownership of the provided data source

http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/46e40e20/src/parquet/file/reader.cc
----------------------------------------------------------------------
diff --git a/src/parquet/file/reader.cc b/src/parquet/file/reader.cc
index fcbe453..8f639e9 100644
--- a/src/parquet/file/reader.cc
+++ b/src/parquet/file/reader.cc
@@ -65,7 +65,9 @@ RowGroupStatistics RowGroupReader::GetColumnStats(int i) const {
 // ParquetFileReader public API
 
 ParquetFileReader::ParquetFileReader() : schema_(nullptr) {}
-ParquetFileReader::~ParquetFileReader() {}
+ParquetFileReader::~ParquetFileReader() {
+  Close();
+}
 
 std::unique_ptr<ParquetFileReader> ParquetFileReader::OpenFile(const std::string& path,
     bool memory_map) {
@@ -91,7 +93,9 @@ void ParquetFileReader::Open(std::unique_ptr<ParquetFileReader::Contents> conten
 }
 
 void ParquetFileReader::Close() {
-  contents_->Close();
+  if (contents_) {
+    contents_->Close();
+  }
 }
 
 int ParquetFileReader::num_row_groups() const {

http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/46e40e20/src/parquet/reader-test.cc
----------------------------------------------------------------------
diff --git a/src/parquet/reader-test.cc b/src/parquet/reader-test.cc
index 3ac1525..2c69ce1 100644
--- a/src/parquet/reader-test.cc
+++ b/src/parquet/reader-test.cc
@@ -16,6 +16,7 @@
 // under the License.
 
 #include <gtest/gtest.h>
+#include <fcntl.h>
 #include <cstdlib>
 #include <cstdint>
 #include <iostream>
@@ -23,8 +24,10 @@
 #include <string>
 
 #include "parquet/file/reader.h"
+#include "parquet/file/reader-internal.h"
 #include "parquet/column/reader.h"
 #include "parquet/column/scanner.h"
+#include "parquet/util/input.h"
 
 using std::string;
 
@@ -50,7 +53,6 @@ class TestAllTypesPlain : public ::testing::Test {
   std::unique_ptr<ParquetFileReader> reader_;
 };
 
-
 TEST_F(TestAllTypesPlain, NoopConstructDestruct) {
 }
 
@@ -121,4 +123,35 @@ TEST_F(TestAllTypesPlain, DebugPrintWorks) {
   ASSERT_GT(result.size(), 0);
 }
 
+
+class TestLocalFileSource : public ::testing::Test {
+ public:
+  void SetUp() {
+    std::string dir_string(data_dir);
+
+    std::stringstream ss;
+    ss << dir_string << "/" << "alltypes_plain.parquet";
+
+    file.reset(new LocalFileSource());
+    file->Open(ss.str());
+  }
+
+  void TearDown() {}
+
+ protected:
+  std::unique_ptr<LocalFileSource> file;
+};
+
+TEST_F(TestLocalFileSource, FileClosedOnDestruction) {
+  int file_desc = file->file_descriptor();
+  {
+    auto contents = SerializedFile::Open(std::move(file));
+    std::unique_ptr<ParquetFileReader> result(new ParquetFileReader());
+    result->Open(std::move(contents));
+  }
+  ASSERT_EQ(-1, fcntl(file_desc, F_GETFD));
+  ASSERT_EQ(EBADF, errno);
+}
+
+
 } // namespace parquet_cpp