You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@quickstep.apache.org by zu...@apache.org on 2017/05/26 01:50:35 UTC

incubator-quickstep git commit: Avoided crashes using PartitionAwareInsertDest in Inner HashJoins.

Repository: incubator-quickstep
Updated Branches:
  refs/heads/improve-insert-dest [created] f8181b526


Avoided crashes using PartitionAwareInsertDest in Inner HashJoins.


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

Branch: refs/heads/improve-insert-dest
Commit: f8181b52607df16833f7cb89b6e4271732cc52f6
Parents: 45e691e
Author: Zuyu Zhang <zu...@apache.org>
Authored: Thu May 25 18:38:15 2017 -0700
Committer: Zuyu Zhang <zu...@apache.org>
Committed: Thu May 25 18:38:15 2017 -0700

----------------------------------------------------------------------
 relational_operators/HashJoinOperator.cpp |  4 +++-
 storage/InsertDestination.cpp             | 13 +++++++-----
 storage/InsertDestination.hpp             | 29 ++++++++++++++++++++------
 3 files changed, 34 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/f8181b52/relational_operators/HashJoinOperator.cpp
----------------------------------------------------------------------
diff --git a/relational_operators/HashJoinOperator.cpp b/relational_operators/HashJoinOperator.cpp
index ea90828..f8acd74 100644
--- a/relational_operators/HashJoinOperator.cpp
+++ b/relational_operators/HashJoinOperator.cpp
@@ -471,7 +471,9 @@ void HashInnerJoinWorkOrder::execute() {
         base_accessor->createSharedTupleIdSequenceAdapterVirtual(*existence_map));
   }
 
-  if (probe_accessor->getImplementationType() == ValueAccessor::Implementation::kSplitRowStore) {
+  if (probe_accessor->getImplementationType() == ValueAccessor::Implementation::kSplitRowStore &&
+      output_destination_->getInsertDestinationType() ==
+          InsertDestination::InsertDestinationType::kBlockPoolInsertDestination) {
     executeWithCopyElision(probe_accessor.get());
   } else {
     executeWithoutCopyElision(probe_accessor.get());

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/f8181b52/storage/InsertDestination.cpp
----------------------------------------------------------------------
diff --git a/storage/InsertDestination.cpp b/storage/InsertDestination.cpp
index 891b5a1..aeef08a 100644
--- a/storage/InsertDestination.cpp
+++ b/storage/InsertDestination.cpp
@@ -57,14 +57,16 @@ using std::vector;
 
 namespace quickstep {
 
-InsertDestination::InsertDestination(const CatalogRelationSchema &relation,
+InsertDestination::InsertDestination(const InsertDestinationType insert_dest_type,
+                                     const CatalogRelationSchema &relation,
                                      const StorageBlockLayout *layout,
                                      StorageManager *storage_manager,
                                      const std::size_t relational_op_index,
                                      const std::size_t query_id,
                                      const tmb::client_id scheduler_client_id,
                                      tmb::MessageBus *bus)
-    : thread_id_map_(*ClientIDMap::Instance()),
+    : insert_dest_type_(insert_dest_type),
+      thread_id_map_(*ClientIDMap::Instance()),
       storage_manager_(storage_manager),
       relation_(relation),
       layout_(layout),
@@ -289,7 +291,7 @@ void InsertDestination::bulkInsertTuplesFromValueAccessors(
     // same tuples from the other ValueAccessors.
     for (auto &p : reduced_accessor_attribute_map) {
       ValueAccessor *accessor = p.first;
-      std::vector<attribute_id> attribute_map = p.second;
+      const std::vector<attribute_id> &attribute_map = p.second;
 
       InvokeOnAnyValueAccessor(
           accessor,
@@ -308,7 +310,7 @@ void InsertDestination::bulkInsertTuplesFromValueAccessors(
         // Since the bulk insertion of the first ValueAccessor should already
         // have reserved the space for all the other ValueAccessors' columns,
         // we must have been able to insert all the tuples we asked to insert.
-        DCHECK(num_tuples_inserted == num_tuples_to_insert);
+        DCHECK_EQ(num_tuples_inserted, num_tuples_to_insert);
       }
     }
 
@@ -491,7 +493,8 @@ PartitionAwareInsertDestination::PartitionAwareInsertDestination(
     const std::size_t query_id,
     const tmb::client_id scheduler_client_id,
     tmb::MessageBus *bus)
-    : InsertDestination(relation,
+    : InsertDestination(InsertDestinationType::kPartitionAwareInsertDestination,
+                        relation,
                         layout,
                         storage_manager,
                         relational_op_index,

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/f8181b52/storage/InsertDestination.hpp
----------------------------------------------------------------------
diff --git a/storage/InsertDestination.hpp b/storage/InsertDestination.hpp
index 66c67d7..db58116 100644
--- a/storage/InsertDestination.hpp
+++ b/storage/InsertDestination.hpp
@@ -71,9 +71,16 @@ namespace serialization { class InsertDestination; }
  **/
 class InsertDestination : public InsertDestinationInterface {
  public:
+  enum class InsertDestinationType {
+    kAlwaysCreateBlockInsertDestination = 0,
+    kBlockPoolInsertDestination,
+    kPartitionAwareInsertDestination,
+  };
+
   /**
    * @brief Constructor.
    *
+   * @param insert_dest_type The implementation type.
    * @param relation The relation to insert tuples into.
    * @param layout The layout to use for any newly-created blocks. If NULL,
    *        defaults to relation's default layout.
@@ -84,7 +91,8 @@ class InsertDestination : public InsertDestinationInterface {
    * @param scheduler_client_id The TMB client ID of the scheduler thread.
    * @param bus A pointer to the TMB.
    **/
-  InsertDestination(const CatalogRelationSchema &relation,
+  InsertDestination(const InsertDestinationType insert_dest_type,
+                    const CatalogRelationSchema &relation,
                     const StorageBlockLayout *layout,
                     StorageManager *storage_manager,
                     const std::size_t relational_op_index,
@@ -159,6 +167,10 @@ class InsertDestination : public InsertDestinationInterface {
   void insertTuplesFromVector(std::vector<Tuple>::const_iterator begin,
                               std::vector<Tuple>::const_iterator end) override;
 
+  InsertDestinationType getInsertDestinationType() const {
+    return insert_dest_type_;
+  }
+
   /**
    * @brief Get the set of blocks that were used by clients of this
    *        InsertDestination for insertion.
@@ -271,6 +283,8 @@ class InsertDestination : public InsertDestinationInterface {
     return query_id_;
   }
 
+  const InsertDestinationType insert_dest_type_;
+
   const ClientIDMap &thread_id_map_;
 
   StorageManager *storage_manager_;
@@ -310,7 +324,8 @@ class AlwaysCreateBlockInsertDestination : public InsertDestination {
                                      const std::size_t query_id,
                                      const tmb::client_id scheduler_client_id,
                                      tmb::MessageBus *bus)
-      : InsertDestination(relation,
+      : InsertDestination(InsertDestinationType::kAlwaysCreateBlockInsertDestination,
+                          relation,
                           layout,
                           storage_manager,
                           relational_op_index,
@@ -323,7 +338,7 @@ class AlwaysCreateBlockInsertDestination : public InsertDestination {
 
   void bulkInsertTuplesFromValueAccessors(
       const std::vector<std::pair<ValueAccessor *, std::vector<attribute_id>>> &accessor_attribute_map,
-      bool always_mark_full = false) override  {
+      bool always_mark_full = false) override {
     LOG(FATAL) << "bulkInsertTuplesFromValueAccessors is not implemented for AlwaysCreateBlockInsertDestination";
   }
 
@@ -376,7 +391,8 @@ class BlockPoolInsertDestination : public InsertDestination {
                              const std::size_t query_id,
                              const tmb::client_id scheduler_client_id,
                              tmb::MessageBus *bus)
-      : InsertDestination(relation,
+      : InsertDestination(InsertDestinationType::kBlockPoolInsertDestination,
+                          relation,
                           layout,
                           storage_manager,
                           relational_op_index,
@@ -405,7 +421,8 @@ class BlockPoolInsertDestination : public InsertDestination {
                              const std::size_t query_id,
                              const tmb::client_id scheduler_client_id,
                              tmb::MessageBus *bus)
-      : InsertDestination(relation,
+      : InsertDestination(InsertDestinationType::kBlockPoolInsertDestination,
+                          relation,
                           layout,
                           storage_manager,
                           relational_op_index,
@@ -517,7 +534,7 @@ class PartitionAwareInsertDestination : public InsertDestination {
 
   void bulkInsertTuplesFromValueAccessors(
       const std::vector<std::pair<ValueAccessor *, std::vector<attribute_id>>> &accessor_attribute_map,
-      bool always_mark_full = false) override  {
+      bool always_mark_full = false) override {
     LOG(FATAL) << "bulkInsertTuplesFromValueAccessors is not implemented for PartitionAwareInsertDestination";
   }