Mailing-List: contact commits-help@quickstep.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@quickstep.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: jianqiao@apache.org
To: commits@quickstep.incubator.apache.org
Date: Mon, 11 Sep 2017 21:15:53 -0000
Message-Id: <6fce72384d934201bd7d3994f957970a@git.apache.org>
In-Reply-To: <b792a56c29dd41abae80cd2c53235f17@git.apache.org>
References: <b792a56c29dd41abae80cd2c53235f17@git.apache.org>
Subject: [2/2] incubator-quickstep git commit: Redirect stdout and stderr in
 network mode.
archived-at: Mon, 11 Sep 2017 21:16:02 -0000

Redirect stdout and stderr in network mode.


Project: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/commit/b815b388
Tree: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/tree/b815b388
Diff: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/diff/b815b388

Branch: refs/heads/redirect-stream
Commit: b815b388d4fb501c4454a1b4e9560a92e4fbb469
Parents: 4578c63
Author: Jianqiao Zhu <jianqiao@cs.wisc.edu>
Authored: Fri Sep 1 12:07:13 2017 -0500
Committer: Jianqiao Zhu <jianqiao@cs.wisc.edu>
Committed: Mon Sep 11 16:15:38 2017 -0500

----------------------------------------------------------------------
 CMakeLists.txt                                  |   1 +
 cli/Flags.cpp                                   |   3 +
 cli/Flags.hpp                                   |   2 +
 cli/IOInterface.hpp                             |  11 +-
 cli/NetworkCliClientMain.cpp                    |   1 -
 cli/QuickstepCli.cpp                            |  15 +-
 .../tests/ExecutionGeneratorTestRunner.cpp      |   4 +-
 utility/CMakeLists.txt                          |  13 ++
 utility/ScopedReassignment.hpp                  |  81 ++++++++
 utility/tests/ScopedReassignment_unittest.cpp   | 183 +++++++++++++++++++
 10 files changed, 304 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b815b388/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/CMakeLists.txt b/CMakeLists.txt
index ed80fab..e0d020b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -751,6 +751,7 @@ target_link_libraries(quickstep_cli_shell
                       quickstep_utility_ExecutionDAGVisualizer
                       quickstep_utility_Macros
                       quickstep_utility_PtrVector
+                      quickstep_utility_ScopedReassignment
                       quickstep_utility_SqlError
                       quickstep_utility_StringUtil)
 if (ENABLE_HDFS)

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b815b388/cli/Flags.cpp
----------------------------------------------------------------------
diff --git a/cli/Flags.cpp b/cli/Flags.cpp
index 362eac3..279c503 100644
--- a/cli/Flags.cpp
+++ b/cli/Flags.cpp
@@ -36,6 +36,9 @@ DEFINE_bool(print_query, false,
             "Print each input query statement. This is useful when running a "
             "large number of queries in a batch.");
 
+DEFINE_bool(display_timing, true,
+            "Whether to display execution time of each statement.");
+
 DEFINE_bool(initialize_db, false, "If true, initialize a database.");
 
 static bool ValidateNumWorkers(const char *flagname, int value) {

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b815b388/cli/Flags.hpp
----------------------------------------------------------------------
diff --git a/cli/Flags.hpp b/cli/Flags.hpp
index 1ae37c4..614cca5 100644
--- a/cli/Flags.hpp
+++ b/cli/Flags.hpp
@@ -45,6 +45,8 @@ DECLARE_string(storage_path);
 
 DECLARE_bool(preload_buffer_pool);
 
+DECLARE_bool(display_timing);
+
 /** @} */
 
 }  // namespace quickstep

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b815b388/cli/IOInterface.hpp
----------------------------------------------------------------------
diff --git a/cli/IOInterface.hpp b/cli/IOInterface.hpp
index 815596e..dc0d5b2 100644
--- a/cli/IOInterface.hpp
+++ b/cli/IOInterface.hpp
@@ -28,7 +28,7 @@
 namespace quickstep {
 
 /**
- * An individual IO interaction with Quickstep.
+ * @brief An individual IO interaction with Quickstep.
  */
 class IOHandle {
  public:
@@ -56,18 +56,21 @@ class IOHandle {
 };
 
 /**
- * Virtual base defines a generic, file-based interface around IO. One IO interaction (eg a SQL query) will be assigned
- * an IOHandle. On destruction of the IOHandle, the IO interaction has finished.
+ * @brief Virtual base defines a generic, file-based interface around IO.
+ *        One IO interaction (eg a SQL query) will be assigned an IOHandle.
+ *        On destruction of the IOHandle, the IO interaction has finished.
  */
 class IOInterface {
  public:
   /**
-   * @note Destructing the IOInterface should close any outstanding IO state (eg an open port).
+   * @note Destructing the IOInterface should close any outstanding IO state
+   *       (e.g. an open port).
    */
   virtual ~IOInterface() {}
 
   /**
    * @brief Retrieves the next IOHandle. Blocks if no IO ready.
+   *
    * @return An IOHandle.
    */
   virtual IOHandle* getNextIOHandle() = 0;

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b815b388/cli/NetworkCliClientMain.cpp
----------------------------------------------------------------------
diff --git a/cli/NetworkCliClientMain.cpp b/cli/NetworkCliClientMain.cpp
index 862941c..c55819b 100644
--- a/cli/NetworkCliClientMain.cpp
+++ b/cli/NetworkCliClientMain.cpp
@@ -53,7 +53,6 @@ int main(int argc, char **argv) {
   while (!linereader.bufferEmpty()) {
     std::string query = linereader.getNextCommand();
     if (!query.empty()) {
-      std::cout << query << std::endl;
       std::cout << qs_client.Query(query) << std::endl;
     }
   }

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b815b388/cli/QuickstepCli.cpp
----------------------------------------------------------------------
diff --git a/cli/QuickstepCli.cpp b/cli/QuickstepCli.cpp
index c55bdd7..e8ca56c 100644
--- a/cli/QuickstepCli.cpp
+++ b/cli/QuickstepCli.cpp
@@ -69,6 +69,7 @@
 #include "utility/ExecutionDAGVisualizer.hpp"
 #include "utility/Macros.hpp"
 #include "utility/PtrVector.hpp"
+#include "utility/ScopedReassignment.hpp"
 #include "utility/SqlError.hpp"
 #include "utility/StringUtil.hpp"
 
@@ -105,6 +106,7 @@ using quickstep::PtrVector;
 using quickstep::QueryExecutionUtil;
 using quickstep::QueryHandle;
 using quickstep::QueryProcessor;
+using quickstep::ScopedReassignment;
 using quickstep::SqlParserWrapper;
 using quickstep::Worker;
 using quickstep::WorkerDirectory;
@@ -303,6 +305,9 @@ int main(int argc, char* argv[]) {
   for (;;) {
     string *command_string = new string();
     std::unique_ptr<quickstep::IOHandle> io_handle(io->getNextIOHandle());
+    ScopedReassignment<FILE*> reassign_stdout(&stdout, io_handle->out());
+    ScopedReassignment<FILE*> reassign_stderr(&stderr, io_handle->err());
+
     *command_string = io_handle->getCommand();
     LOG(INFO) << "Command received: " << *command_string;
     if (command_string->size() == 0) {
@@ -397,10 +402,12 @@ int main(int argc, char* argv[]) {
           }
 
           query_processor->saveCatalog();
-          std::chrono::duration<double, std::milli> time_ms = end - start;
-          fprintf(io_handle->out(), "Time: %s ms\n",
-                 quickstep::DoubleToStringWithSignificantDigits(
-                     time_ms.count(), 3).c_str());
+          if (quickstep::FLAGS_display_timing) {
+            std::chrono::duration<double, std::milli> time_ms = end - start;
+            fprintf(io_handle->out(), "Time: %s ms\n",
+                   quickstep::DoubleToStringWithSignificantDigits(
+                       time_ms.count(), 3).c_str());
+          }
           if (quickstep::FLAGS_profile_and_report_workorder_perf) {
             // TODO(harshad) - Allow user specified file instead of stdout.
             foreman.printWorkOrderProfilingResults(query_id, stdout);

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b815b388/query_optimizer/tests/ExecutionGeneratorTestRunner.cpp
----------------------------------------------------------------------
diff --git a/query_optimizer/tests/ExecutionGeneratorTestRunner.cpp b/query_optimizer/tests/ExecutionGeneratorTestRunner.cpp
index 050ef0d..a854589 100644
--- a/query_optimizer/tests/ExecutionGeneratorTestRunner.cpp
+++ b/query_optimizer/tests/ExecutionGeneratorTestRunner.cpp
@@ -63,7 +63,9 @@ void ExecutionGeneratorTestRunner::runTestCase(
   sql_parser_.feedNextBuffer(new std::string(input));
 
   // Redirect stderr to output_stream.
-  stderr = output_stream.file();
+  if (!FLAGS_logtostderr) {
+    stderr = output_stream.file();
+  }
 
   while (true) {
     ParseResult result = sql_parser_.getNextStatement();

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b815b388/utility/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/utility/CMakeLists.txt b/utility/CMakeLists.txt
index a832404..c5c9dd8 100644
--- a/utility/CMakeLists.txt
+++ b/utility/CMakeLists.txt
@@ -196,6 +196,7 @@ add_library(quickstep_utility_PtrMap ../empty_src.cpp PtrMap.hpp)
 add_library(quickstep_utility_PtrVector ../empty_src.cpp PtrVector.hpp)
 add_library(quickstep_utility_ScopedBuffer ../empty_src.cpp ScopedBuffer.hpp)
 add_library(quickstep_utility_ScopedDeleter ../empty_src.cpp ScopedDeleter.hpp)
+add_library(quickstep_utility_ScopedReassignment ../empty_src.cpp ScopedReassignment.hpp)
 add_library(quickstep_utility_ShardedLockManager ../empty_src.cpp ShardedLockManager.hpp)
 add_library(quickstep_utility_SortConfiguration SortConfiguration.cpp SortConfiguration.hpp)
 add_library(quickstep_utility_SortConfiguration_proto
@@ -313,6 +314,8 @@ target_link_libraries(quickstep_utility_ScopedBuffer
                       quickstep_utility_Macros)
 target_link_libraries(quickstep_utility_ScopedDeleter
                       quickstep_utility_Macros)
+target_link_libraries(quickstep_utility_ScopedReassignment
+                      quickstep_utility_Macros)
 target_link_libraries(quickstep_utility_SqlError
                       glog)
 target_link_libraries(quickstep_utility_SortConfiguration
@@ -379,6 +382,7 @@ target_link_libraries(quickstep_utility
                       quickstep_utility_PtrVector
                       quickstep_utility_ScopedBuffer
                       quickstep_utility_ScopedDeleter
+                      quickstep_utility_ScopedReassignment
                       quickstep_utility_ShardedLockManager
                       quickstep_utility_SortConfiguration
                       quickstep_utility_SortConfiguration_proto
@@ -462,6 +466,15 @@ target_link_libraries(ScopedDeleter_unittest
                       ${LIBS})
 add_test(ScopedDeleter_unittest ScopedDeleter_unittest)
 
+add_executable(ScopedReassignment_unittest "${CMAKE_CURRENT_SOURCE_DIR}/tests/ScopedReassignment_unittest.cpp")
+target_link_libraries(ScopedReassignment_unittest
+                      gtest
+                      gtest_main
+                      quickstep_utility_Macros
+                      quickstep_utility_ScopedReassignment
+                      ${LIBS})
+add_test(ScopedReassignment_unittest ScopedReassignment_unittest)
+
 add_executable(SqlError_unittest "${CMAKE_CURRENT_SOURCE_DIR}/tests/SqlError_unittest.cpp")
 target_link_libraries(SqlError_unittest
                       glog

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b815b388/utility/ScopedReassignment.hpp
----------------------------------------------------------------------
diff --git a/utility/ScopedReassignment.hpp b/utility/ScopedReassignment.hpp
new file mode 100644
index 0000000..bd0bcb2
--- /dev/null
+++ b/utility/ScopedReassignment.hpp
@@ -0,0 +1,81 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ **/
+
+#ifndef QUICKSTEP_UTILITY_SCOPED_REASSIGNMENT_HPP_
+#define QUICKSTEP_UTILITY_SCOPED_REASSIGNMENT_HPP_
+
+#include <utility>
+#include <type_traits>
+
+#include "utility/Macros.hpp"
+
+namespace quickstep {
+
+/** \addtogroup Utility
+ *  @{
+ */
+
+/**
+ * @brief RAII helper object that assigns a new value to a variable and restores
+ *        the old value when the helper object goes out of scope.
+ **/
+template <typename T>
+class ScopedReassignment {
+ public:
+  /**
+   * @brief Constructor.
+   *
+   * @param var The variable.
+   * @param new_value The new value to be assigned to \p var.
+   **/
+  template <typename U>
+  ScopedReassignment(T *var, U &&new_value)
+      : var_(*var),
+        old_value_(std::move(*var)) {
+    *var = std::forward<U>(new_value);
+  }
+
+  /**
+   * @brief Destructor. Restores the old value to \p var_.
+   **/
+  ~ScopedReassignment() {
+    var_ = std::move(old_value_);
+  }
+
+  /**
+   * @brief Get the old value.
+   *
+   * @return A const reference to the old value.
+   */
+  inline const T& old_value() const {
+    return old_value_;
+  }
+
+ private:
+  T &var_;
+  T old_value_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedReassignment);
+};
+
+/** @} */
+
+}  // namespace quickstep
+
+#endif  // QUICKSTEP_UTILITY_SCOPED_REASSIGNMENT_HPP_

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b815b388/utility/tests/ScopedReassignment_unittest.cpp
----------------------------------------------------------------------
diff --git a/utility/tests/ScopedReassignment_unittest.cpp b/utility/tests/ScopedReassignment_unittest.cpp
new file mode 100644
index 0000000..b7da017
--- /dev/null
+++ b/utility/tests/ScopedReassignment_unittest.cpp
@@ -0,0 +1,183 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ **/
+
+#include "utility/ScopedReassignment.hpp"
+
+#include "utility/Macros.hpp"
+
+#include "gtest/gtest.h"
+
+namespace quickstep {
+
+namespace {
+const int kOldValue = 10;
+const int kNewValue = 20;
+}  // namespace
+
+/**
+ * @brief A test class that is movable but not copyable.
+ */
+class NonCopyable {
+ public:
+  explicit NonCopyable(const int value)
+      : value_(value) {}
+
+  explicit NonCopyable(NonCopyable &&other)
+      : value_(other.value_) {}
+
+  NonCopyable& operator=(NonCopyable &&other) {
+    value_ = other.value_;
+    return *this;
+  }
+
+  int value() const {
+    return value_;
+  }
+
+ private:
+  int value_;
+
+  DISALLOW_COPY_AND_ASSIGN(NonCopyable);
+};
+
+/**
+ * @brief A test class that is copyable but not movable.
+ */
+class NonMovable {
+ public:
+  explicit NonMovable(const int value)
+      : value_(value) {}
+
+  explicit NonMovable(const NonMovable &other)
+      : value_(other.value_) {}
+
+  NonMovable& operator=(const NonMovable &other) {
+    value_ = other.value_;
+    return *this;
+  }
+
+  int value() const {
+    return value_;
+  }
+
+ private:
+  int value_;
+};
+
+/**
+ * @brief A test class that is copyable and movable.
+ */
+class CopyableMovable {
+ public:
+  explicit CopyableMovable(const int value)
+      : value_(value) {}
+
+  explicit CopyableMovable(const CopyableMovable &other)
+      : copy_constructed_(true),
+        value_(other.value_) {}
+
+  explicit CopyableMovable(CopyableMovable &&other)
+      : value_(other.value_) {}
+
+  CopyableMovable& operator=(const CopyableMovable &other) {
+    value_ = other.value_;
+    copy_assigned_ = true;
+    return *this;
+  }
+
+  CopyableMovable& operator=(CopyableMovable &&other) {
+    value_ = other.value_;
+    copy_assigned_ = false;
+    return *this;
+  }
+
+  int value() const {
+    return value_;
+  }
+
+  bool copy_constructed() const {
+    return copy_constructed_;
+  }
+
+  bool copy_assigned() const {
+    return copy_assigned_;
+  }
+
+ private:
+  const bool copy_constructed_ = false;
+  int value_;
+  bool copy_assigned_ = false;
+};
+
+TEST(ScopedReassignment, NonCopyableTest) {
+  NonCopyable non_copyable(kOldValue);
+  {
+    NonCopyable other(kNewValue);
+    ScopedReassignment<NonCopyable> reassign(&non_copyable, std::move(other));
+    EXPECT_EQ(kNewValue, non_copyable.value());
+  }
+  EXPECT_EQ(kOldValue, non_copyable.value());
+
+  {
+    ScopedReassignment<NonCopyable> reassign(&non_copyable, NonCopyable(kNewValue));
+    EXPECT_EQ(kNewValue, non_copyable.value());
+  }
+  EXPECT_EQ(kOldValue, non_copyable.value());
+}
+
+TEST(ScopedReassignment, NonMovableTest) {
+  NonMovable non_movable(kOldValue);
+  {
+    NonMovable other(kNewValue);
+    ScopedReassignment<NonMovable> reassign(&non_movable, other);
+    EXPECT_EQ(kNewValue, non_movable.value());
+  }
+  EXPECT_EQ(kOldValue, non_movable.value());
+
+  {
+    ScopedReassignment<NonMovable> reassign(&non_movable, NonMovable(kNewValue));
+    EXPECT_EQ(kNewValue, non_movable.value());
+  }
+  EXPECT_EQ(kOldValue, non_movable.value());
+}
+
+TEST(ScopedReassignment, CopyableMovableTest) {
+  CopyableMovable copyable_movable(kOldValue);
+  {
+    CopyableMovable other(kNewValue);
+    ScopedReassignment<CopyableMovable> reassign(&copyable_movable, other);
+    EXPECT_EQ(kNewValue, copyable_movable.value());
+    EXPECT_FALSE(reassign.old_value().copy_constructed());
+    EXPECT_TRUE(copyable_movable.copy_assigned());
+  }
+  EXPECT_EQ(kOldValue, copyable_movable.value());
+  EXPECT_FALSE(copyable_movable.copy_assigned());
+
+  {
+    CopyableMovable other(kNewValue);
+    ScopedReassignment<CopyableMovable> reassign(&copyable_movable, std::move(other));
+    EXPECT_EQ(kNewValue, copyable_movable.value());
+    EXPECT_FALSE(reassign.old_value().copy_constructed());
+    EXPECT_FALSE(copyable_movable.copy_assigned());
+  }
+  EXPECT_EQ(kOldValue, copyable_movable.value());
+  EXPECT_FALSE(copyable_movable.copy_assigned());
+}
+
+}  // namespace quickstep