You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@quickstep.apache.org by ra...@apache.org on 2016/08/15 11:29:09 UTC

[1/4] incubator-quickstep git commit: Fixed signed-unsigned comparison failure. Minor code cleanup.

Repository: incubator-quickstep
Updated Branches:
  refs/heads/quickstep-28-29 1595e0ba7 -> 5a80e33ef


Fixed signed-unsigned comparison failure. Minor code cleanup.


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

Branch: refs/heads/quickstep-28-29
Commit: 94767dd1a54c1b237ae34d4bd093bc38e726bf3e
Parents: 1595e0b
Author: rathijit <ra...@node-2.aggregation.quickstep-pg0.wisc.cloudlab.us>
Authored: Sun Aug 14 16:14:36 2016 -0500
Committer: rathijit <ra...@node-2.aggregation.quickstep-pg0.wisc.cloudlab.us>
Committed: Sun Aug 14 16:14:36 2016 -0500

----------------------------------------------------------------------
 storage/AggregationOperationState.cpp | 36 ++----------------------------
 storage/FastHashTable.hpp             | 20 +++++------------
 2 files changed, 8 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/94767dd1/storage/AggregationOperationState.cpp
----------------------------------------------------------------------
diff --git a/storage/AggregationOperationState.cpp b/storage/AggregationOperationState.cpp
index 14d4ea6..ab1430c 100644
--- a/storage/AggregationOperationState.cpp
+++ b/storage/AggregationOperationState.cpp
@@ -94,13 +94,6 @@ AggregationOperationState::AggregationOperationState(
     handles_.emplace_back(new AggregationHandleDistinct());
     arguments_.push_back({});
     is_distinct_.emplace_back(false);
-
- /*   group_by_hashtable_pools_.emplace_back(std::unique_ptr<HashTablePool>(
-        new HashTablePool(estimated_num_entries,
-                          hash_table_impl_type,
-                          group_by_types,
-                          handles_.back().get(),
-                          storage_manager)));*/
     group_by_hashtable_pools_.emplace_back(std::unique_ptr<HashTablePool>(
         new HashTablePool(estimated_num_entries,
                           hash_table_impl_type,
@@ -136,19 +129,12 @@ AggregationOperationState::AggregationOperationState(
       handles_.emplace_back((*agg_func_it)->createHandle(argument_types));
 
       if (!group_by_list_.empty()) {
-        // Aggregation with GROUP BY: create a HashTable pool for per-group states.
- /*       group_by_hashtable_pools_.emplace_back(std::unique_ptr<HashTablePool>(
-            new HashTablePool(estimated_num_entries,
-                              hash_table_impl_type,
-                              group_by_types,
-                              handles_.back().get(),
-                              storage_manager)));*/
+        // Aggregation with GROUP BY: combined payload is partially updated in the presence of DISTINCT.
          if (*is_distinct_it) {
             handles_.back()->BlockUpdate();
          }
          group_by_handles.emplace_back(handles_.back());
          payload_sizes.emplace_back(group_by_handles.back()->getPayloadSize());
-
       } else {
         // Aggregation without GROUP BY: create a single global state.
         single_states_.emplace_back(handles_.back()->createInitialState());
@@ -183,23 +169,13 @@ AggregationOperationState::AggregationOperationState(
         // the number of entries in the distinctify hash table. We may estimate
         // for each distinct aggregation an estimated_num_distinct_keys value during
         // query optimization, if it worths.
- /*       distinctify_hashtables_.emplace_back(
-            handles_.back()->createDistinctifyHashTable(
-                *distinctify_hash_table_impl_types_it,
-                key_types,
-                estimated_num_entries,
-                storage_manager));*/
-
-        std::vector<AggregationHandle *> local;
-        // local.emplace_back(handles_.back());
-        local.clear();
         distinctify_hashtables_.emplace_back(
         AggregationStateFastHashTableFactory::CreateResizable(
                 *distinctify_hash_table_impl_types_it,
                 key_types,
                 estimated_num_entries,
                 {0},
-                local,
+                {},
                 storage_manager));
         ++distinctify_hash_table_impl_types_it;
       } else {
@@ -455,13 +431,6 @@ void AggregationOperationState::aggregateBlockHashTable(const block_id input_blo
   DCHECK(group_by_hashtable_pools_[0] != nullptr);
   AggregationStateHashTableBase *agg_hash_table = group_by_hashtable_pools_[0]->getHashTableFast();
   DCHECK(agg_hash_table != nullptr);
- /*     block->aggregateGroupBy(*handles_[agg_idx],
-                              arguments_[agg_idx],
-                              group_by_list_,
-                              predicate_.get(),
-                              agg_hash_table,
-                              &reuse_matches,
-                              &reuse_group_by_vectors);*/
   block->aggregateGroupByFast(arguments_,
                               group_by_list_,
                               predicate_.get(),
@@ -507,7 +476,6 @@ void AggregationOperationState::finalizeHashTable(InsertDestination *output_dest
 
   // TODO(harshad) - Find heuristics for faster merge, even in a single thread.
   // e.g. Keep merging entries from smaller hash tables to larger.
-//  auto *hash_tables = group_by_hashtable_pools_[0]->getAllHashTables();
 
   auto *hash_tables = group_by_hashtable_pools_[0]->getAllHashTables();
   for (std::size_t agg_idx = 0; agg_idx < handles_.size(); ++agg_idx) {

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/94767dd1/storage/FastHashTable.hpp
----------------------------------------------------------------------
diff --git a/storage/FastHashTable.hpp b/storage/FastHashTable.hpp
index e7887ab..8d8d82b 100644
--- a/storage/FastHashTable.hpp
+++ b/storage/FastHashTable.hpp
@@ -1901,7 +1901,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
         uint8_t *value = upsertCompositeKeyInternalFast(key, init_value_ptr, variable_size);
         if (value != nullptr) {
             SpinMutex lock(value);
-            for (int k = 0; k < handles_.size(); ++k) {
+            for (unsigned int k = 0; k < handles_.size(); ++k) {
                 handles_[k]->mergeStatesFast(source_state + payload_offsets_[k], value + payload_offsets_[k]);
             }
           return true;
@@ -1915,7 +1915,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
       return false;
     } else {
       SpinMutex lock(value);
-      for (int k = 0; k < handles_.size(); ++k) {
+      for (unsigned int k = 0; k < handles_.size(); ++k) {
           handles_[k]->mergeStatesFast(source_state + payload_offsets_[k], value + payload_offsets_[k]);
       }
       return true;
@@ -2018,10 +2018,8 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
               break;
             } else {
               SpinMutex lock(value);
-              for (int k = 0; k < handles_.size(); ++k) {
+              for (unsigned int k = 0; k < handles_.size(); ++k) {
                   local.clear();
-//                std::for_each(argument_ids[k].begin(),argument_ids[k].end(),[&](attribute_id id)
-//                {local.emplace_back(accessor->getTypedValue(id));});
                   if (argument_ids[k].size()) {
                     local.emplace_back(accessor->getTypedValue(argument_ids[k].front()));
                   }
@@ -2047,10 +2045,8 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
           return false;
         } else {
           SpinMutex lock(value);
-          for (int k = 0; k < handles_.size(); ++k) {
+          for (unsigned int k = 0; k < handles_.size(); ++k) {
               local.clear();
-//            std::for_each(argument_ids[k].begin(),argument_ids[k].end(),[&](attribute_id id)
-//            {local.emplace_back(accessor->getTypedValue(id));});
               if (argument_ids[k].size()) {
                  local.emplace_back(accessor->getTypedValue(argument_ids[k].front()));
               }
@@ -2175,10 +2171,8 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
               break;
             } else {
               SpinMutex lock(value);
-              for (int k = 0; k < handles_.size(); ++k) {
+              for (unsigned int k = 0; k < handles_.size(); ++k) {
                   local.clear();
-//                std::for_each(argument_ids[k].begin(),argument_ids[k].end(),[&](attribute_id id)
-//                {local.emplace_back(accessor->getTypedValue(id));});
                   if (argument_ids[k].size()) {
                       local.emplace_back(accessor->getTypedValue(argument_ids[k].front()));
                   }
@@ -2208,10 +2202,8 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
           return false;
         } else {
           SpinMutex lock(value);
-          for (int k = 0; k < handles_.size(); ++k) {
+          for (unsigned int k = 0; k < handles_.size(); ++k) {
               local.clear();
-//            std::for_each(argument_ids[k].begin(),argument_ids[k].end(),[&](attribute_id id)
-//            {local.emplace_back(accessor->getTypedValue(id));});
               if (argument_ids[k].size()) {
                  local.emplace_back(accessor->getTypedValue(argument_ids[k].front()));
               }


[2/4] incubator-quickstep git commit: reinterpreted byte to SpinMutex before locking. This removes the need to have an additional function accepting a pointer in the SpinMutex class

Posted by ra...@apache.org.
reinterpreted byte to SpinMutex before locking. This removes the need to have an additional function accepting a pointer in the SpinMutex class


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

Branch: refs/heads/quickstep-28-29
Commit: 21d7e618f53adbd121bd4a54a70149ff04a4e282
Parents: 94767dd
Author: rathijit <ra...@node-2.aggregation.quickstep-pg0.wisc.cloudlab.us>
Authored: Sun Aug 14 19:54:50 2016 -0500
Committer: rathijit <ra...@node-2.aggregation.quickstep-pg0.wisc.cloudlab.us>
Committed: Sun Aug 14 19:54:50 2016 -0500

----------------------------------------------------------------------
 storage/FastHashTable.hpp | 12 ++++++------
 threading/SpinMutex.hpp   |  2 --
 2 files changed, 6 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/21d7e618/storage/FastHashTable.hpp
----------------------------------------------------------------------
diff --git a/storage/FastHashTable.hpp b/storage/FastHashTable.hpp
index 8d8d82b..c659a20 100644
--- a/storage/FastHashTable.hpp
+++ b/storage/FastHashTable.hpp
@@ -1900,7 +1900,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
         SpinSharedMutexSharedLock<true> resize_lock(resize_shared_mutex_);
         uint8_t *value = upsertCompositeKeyInternalFast(key, init_value_ptr, variable_size);
         if (value != nullptr) {
-            SpinMutex lock(value);
+            SpinMutexLock lock(*(reinterpret_cast<SpinMutex *>(value)));
             for (unsigned int k = 0; k < handles_.size(); ++k) {
                 handles_[k]->mergeStatesFast(source_state + payload_offsets_[k], value + payload_offsets_[k]);
             }
@@ -1914,7 +1914,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
     if (value == nullptr) {
       return false;
     } else {
-      SpinMutex lock(value);
+      SpinMutexLock lock(*(reinterpret_cast<SpinMutex *>(value)));
       for (unsigned int k = 0; k < handles_.size(); ++k) {
           handles_[k]->mergeStatesFast(source_state + payload_offsets_[k], value + payload_offsets_[k]);
       }
@@ -2017,7 +2017,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
               continuing = true;
               break;
             } else {
-              SpinMutex lock(value);
+              SpinMutexLock lock(*(reinterpret_cast<SpinMutex *>(value)));
               for (unsigned int k = 0; k < handles_.size(); ++k) {
                   local.clear();
                   if (argument_ids[k].size()) {
@@ -2044,7 +2044,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
         if (value == nullptr) {
           return false;
         } else {
-          SpinMutex lock(value);
+          SpinMutexLock lock(*(reinterpret_cast<SpinMutex *>(value)));
           for (unsigned int k = 0; k < handles_.size(); ++k) {
               local.clear();
               if (argument_ids[k].size()) {
@@ -2170,7 +2170,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
               continuing = true;
               break;
             } else {
-              SpinMutex lock(value);
+              SpinMutexLock lock(*(reinterpret_cast<SpinMutex *>(value)));
               for (unsigned int k = 0; k < handles_.size(); ++k) {
                   local.clear();
                   if (argument_ids[k].size()) {
@@ -2201,7 +2201,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
         if (value == nullptr) {
           return false;
         } else {
-          SpinMutex lock(value);
+          SpinMutexLock lock(*(reinterpret_cast<SpinMutex *>(value)));
           for (unsigned int k = 0; k < handles_.size(); ++k) {
               local.clear();
               if (argument_ids[k].size()) {

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/21d7e618/threading/SpinMutex.hpp
----------------------------------------------------------------------
diff --git a/threading/SpinMutex.hpp b/threading/SpinMutex.hpp
index c9838e4..b64a6ca 100644
--- a/threading/SpinMutex.hpp
+++ b/threading/SpinMutex.hpp
@@ -42,8 +42,6 @@ class SpinMutex {
   SpinMutex() : locked_(false) {
   }
 
-  explicit SpinMutex(uint8_t *ptr): locked_(*ptr) {}
-
   /**
    * @note This call does NOT yield when contended. SpinMutex is intended
    *       mainly for cases where locks are held briefly and it is better to


[4/4] incubator-quickstep git commit: Removed some dead code and made minor updates.

Posted by ra...@apache.org.
Removed some dead code and made minor updates.


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

Branch: refs/heads/quickstep-28-29
Commit: 5a80e33efad6b60786661f777fca6a0c3af47274
Parents: 21d7e61
Author: rathijit <ra...@node-2.aggregation.quickstep-pg0.wisc.cloudlab.us>
Authored: Mon Aug 15 06:28:36 2016 -0500
Committer: rathijit <ra...@node-2.aggregation.quickstep-pg0.wisc.cloudlab.us>
Committed: Mon Aug 15 06:28:36 2016 -0500

----------------------------------------------------------------------
 .../aggregation/AggregationConcreteHandle.hpp   | 418 +------------------
 expressions/aggregation/AggregationHandle.hpp   |  24 +-
 .../aggregation/AggregationHandleAvg.cpp        |  30 +-
 .../aggregation/AggregationHandleAvg.hpp        |  11 +-
 .../aggregation/AggregationHandleCount.cpp      |  30 +-
 .../aggregation/AggregationHandleCount.hpp      |  13 +-
 .../aggregation/AggregationHandleDistinct.cpp   |   5 +-
 .../aggregation/AggregationHandleDistinct.hpp   |  13 +-
 .../aggregation/AggregationHandleMax.cpp        |  24 +-
 .../aggregation/AggregationHandleMax.hpp        |  11 +-
 .../aggregation/AggregationHandleMin.cpp        |  24 +-
 .../aggregation/AggregationHandleMin.hpp        |  11 +-
 .../aggregation/AggregationHandleSum.cpp        |  21 +-
 .../aggregation/AggregationHandleSum.hpp        |  14 +-
 expressions/aggregation/CMakeLists.txt          |  82 ++--
 storage/AggregationOperationState.cpp           |  21 +-
 storage/FastHashTable.hpp                       | 293 ++-----------
 storage/FastSeparateChainingHashTable.hpp       | 213 +---------
 18 files changed, 136 insertions(+), 1122 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationConcreteHandle.hpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationConcreteHandle.hpp b/expressions/aggregation/AggregationConcreteHandle.hpp
index df02721..a5b77b5 100644
--- a/expressions/aggregation/AggregationConcreteHandle.hpp
+++ b/expressions/aggregation/AggregationConcreteHandle.hpp
@@ -50,37 +50,6 @@ class ValueAccessor;
  * @brief An upserter class for modifying the destination hash table while
  *        merging two group by hash tables.
  **/
-template <typename HandleT, typename StateT>
-class HashTableStateUpserter {
- public:
-  /**
-   * @brief Constructor.
-   *
-   * @param handle The aggregation handle being used.
-   * @param source_state The aggregation state in the source aggregation hash
-   *        table. The corresponding state (for the same key) in the destination
-   *        hash table will be upserted.
-   **/
-  HashTableStateUpserter(const HandleT &handle, const StateT &source_state)
-      : handle_(handle), source_state_(source_state) {}
-
-  /**
-   * @brief The operator for the functor required for the upsert.
-   *
-   * @param destination_state The aggregation state in the aggregation hash
-   *        table that is being upserted.
-   **/
-  void operator()(StateT *destination_state) {
-    handle_.mergeStates(source_state_, destination_state);
-  }
-
- private:
-  const HandleT &handle_;
-  const StateT &source_state_;
-
-  DISALLOW_COPY_AND_ASSIGN(HashTableStateUpserter);
-};
-
 template <typename HandleT>
 class HashTableStateUpserterFast {
  public:
@@ -113,103 +82,6 @@ class HashTableStateUpserterFast {
 };
 
 /**
- * @brief A class to support the functor for merging group by hash tables.
- **/
-template <typename HandleT, typename StateT, typename HashTableT>
-class HashTableMerger {
- public:
-  /**
-   * @brief Constructor
-   *
-   * @param handle The Aggregation handle being used.
-   * @param destination_hash_table The destination hash table to which other
-   *        hash tables will be merged.
-   **/
-  HashTableMerger(const HandleT &handle,
-                  AggregationStateHashTableBase *destination_hash_table)
-      : handle_(handle),
-        destination_hash_table_(
-            static_cast<HashTableT *>(destination_hash_table)) {}
-
-  /**
-   * @brief The operator for the functor.
-   *
-   * @param group_by_key The group by key being merged.
-   * @param source_state The aggregation state for the given key in the source
-   *        aggregation hash table.
-   **/
-  inline void operator()(const std::vector<TypedValue> &group_by_key,
-                         const StateT &source_state) {
-    const StateT *original_state =
-        destination_hash_table_->getSingleCompositeKey(group_by_key);
-    if (original_state != nullptr) {
-      HashTableStateUpserter<HandleT, StateT> upserter(
-          handle_, source_state);
-      // The CHECK is required as upsertCompositeKey can return false if the
-      // hash table runs out of space during the upsert process. The ideal
-      // solution will be to retry again if the upsert fails.
-      CHECK(destination_hash_table_->upsertCompositeKey(
-          group_by_key, *original_state, &upserter));
-    } else {
-      destination_hash_table_->putCompositeKey(group_by_key, source_state);
-    }
-  }
-
- private:
-  const HandleT &handle_;
-  HashTableT *destination_hash_table_;
-
-  DISALLOW_COPY_AND_ASSIGN(HashTableMerger);
-};
-
-template <typename HandleT, typename HashTableT>
-class HashTableMergerFast {
- public:
-  /**
-   * @brief Constructor
-   *
-   * @param handle The Aggregation handle being used.
-   * @param destination_hash_table The destination hash table to which other
-   *        hash tables will be merged.
-   **/
-  HashTableMergerFast(const HandleT &handle,
-                  AggregationStateHashTableBase *destination_hash_table)
-      : handle_(handle),
-        destination_hash_table_(
-            static_cast<HashTableT *>(destination_hash_table)) {}
-
-  /**
-   * @brief The operator for the functor.
-   *
-   * @param group_by_key The group by key being merged.
-   * @param source_state The aggregation state for the given key in the source
-   *        aggregation hash table.
-   **/
-  inline void operator()(const std::vector<TypedValue> &group_by_key,
-                         const uint8_t *source_state) {
-    const uint8_t *original_state =
-        destination_hash_table_->getSingleCompositeKey(group_by_key);
-    if (original_state != nullptr) {
-      HashTableStateUpserterFast<HandleT> upserter(
-          handle_, source_state);
-      // The CHECK is required as upsertCompositeKey can return false if the
-      // hash table runs out of space during the upsert process. The ideal
-      // solution will be to retry again if the upsert fails.
-      CHECK(destination_hash_table_->upsertCompositeKeyFast(
-          group_by_key, original_state, &upserter));
-    } else {
-      destination_hash_table_->putCompositeKeyFast(group_by_key, source_state);
-    }
-  }
-
- private:
-  const HandleT &handle_;
-  HashTableT *destination_hash_table_;
-
-  DISALLOW_COPY_AND_ASSIGN(HashTableMergerFast);
-};
-
-/**
  * @brief The helper intermediate subclass of AggregationHandle that provides
  *        virtual method implementations as well as helper methods that are
  *        shared among all its subclasses.
@@ -255,55 +127,16 @@ class AggregationConcreteHandle : public AggregationHandle {
   }
 
   template <typename HandleT,
-            typename StateT,
-            typename HashTableT>
-  void aggregateValueAccessorIntoHashTableNullaryHelper(
-      ValueAccessor *accessor,
-      const std::vector<attribute_id> &group_by_key_ids,
-      const StateT &default_state,
-      AggregationStateHashTableBase *hash_table) const;
-
-  template <typename HandleT,
-            typename StateT,
-            typename HashTableT>
-  void aggregateValueAccessorIntoHashTableUnaryHelper(
-      ValueAccessor *accessor,
-      const attribute_id argument_id,
-      const std::vector<attribute_id> &group_by_key_ids,
-      const StateT &default_state,
-      AggregationStateHashTableBase *hash_table) const;
-
-  template <typename HandleT,
-            typename StateT>
-  StateT* aggregateOnDistinctifyHashTableForSingleUnaryHelper(
-      const AggregationStateHashTableBase &distinctify_hash_table) const;
-
-  template <typename HandleT,
             typename StateT>
   StateT* aggregateOnDistinctifyHashTableForSingleUnaryHelperFast(
       const AggregationStateHashTableBase &distinctify_hash_table) const;
 
   template <typename HandleT,
-            typename StateT,
-            typename HashTableT>
-  void aggregateOnDistinctifyHashTableForGroupByUnaryHelper(
-      const AggregationStateHashTableBase &distinctify_hash_table,
-      const StateT &default_state,
-      AggregationStateHashTableBase *hash_table) const;
-
-  template <typename HandleT,
             typename HashTableT>
   void aggregateOnDistinctifyHashTableForGroupByUnaryHelperFast(
       const AggregationStateHashTableBase &distinctify_hash_table,
-      AggregationStateHashTableBase *hash_table, int index) const;
-
-
-  template <typename HandleT,
-            typename HashTableT>
-  ColumnVector* finalizeHashTableHelper(
-      const Type &result_type,
-      const AggregationStateHashTableBase &hash_table,
-      std::vector<std::vector<TypedValue>> *group_by_keys) const;
+      AggregationStateHashTableBase *hash_table,
+      int index) const;
 
   template <typename HandleT,
             typename HashTableT>
@@ -314,17 +147,6 @@ class AggregationConcreteHandle : public AggregationHandle {
       int index) const;
 
   template <typename HandleT, typename HashTableT>
-  inline TypedValue finalizeGroupInHashTable(
-      const AggregationStateHashTableBase &hash_table,
-      const std::vector<TypedValue> &group_key) const {
-    const AggregationState *group_state
-        = static_cast<const HashTableT&>(hash_table).getSingleCompositeKey(group_key);
-    DCHECK(group_state != nullptr)
-        << "Could not find entry for specified group_key in HashTable";
-    return static_cast<const HandleT*>(this)->finalizeHashTableEntry(*group_state);
-  }
-
-  template <typename HandleT, typename HashTableT>
   inline TypedValue finalizeGroupInHashTableFast(
       const AggregationStateHashTableBase &hash_table,
       const std::vector<TypedValue> &group_key,
@@ -336,67 +158,16 @@ class AggregationConcreteHandle : public AggregationHandle {
     return static_cast<const HandleT*>(this)->finalizeHashTableEntryFast(group_state);
   }
 
-  template <typename HandleT, typename StateT, typename HashTableT>
-  void mergeGroupByHashTablesHelper(
-      const AggregationStateHashTableBase &source_hash_table,
-      AggregationStateHashTableBase *destination_hash_table) const;
-
   template <typename HandleT, typename HashTableT>
   void mergeGroupByHashTablesHelperFast(
       const AggregationStateHashTableBase &source_hash_table,
       AggregationStateHashTableBase *destination_hash_table) const;
 
-
  private:
   DISALLOW_COPY_AND_ASSIGN(AggregationConcreteHandle);
 };
 
 /**
- * @brief Templated class to implement value-accessor-based upserter for each
- *        aggregation state payload type. This version is for nullary
- *        aggregates (those that take no arguments).
- **/
-template <typename HandleT, typename StateT>
-class NullaryAggregationStateValueAccessorUpserter {
- public:
-  explicit NullaryAggregationStateValueAccessorUpserter(const HandleT &handle)
-      : handle_(handle) {
-  }
-
-  template <typename ValueAccessorT>
-  inline void operator()(const ValueAccessorT &accessor, StateT *state) {
-    handle_.iterateNullaryInl(state);
-  }
-
- private:
-  const HandleT &handle_;
-};
-
-/**
- * @brief Templated class to implement value-accessor-based upserter for each
- *        aggregation state payload type. This version is for unary aggregates
- *        (those that take a single argument).
- **/
-template <typename HandleT, typename StateT>
-class UnaryAggregationStateValueAccessorUpserter {
- public:
-  UnaryAggregationStateValueAccessorUpserter(const HandleT &handle,
-                                             attribute_id value_id)
-    : handle_(handle),
-      value_id_(value_id) {
-  }
-
-  template <typename ValueAccessorT>
-  inline void operator()(const ValueAccessorT &accessor, StateT *state) {
-    handle_.iterateUnaryInl(state, accessor.getTypedValue(value_id_));
-  }
-
- private:
-  const HandleT &handle_;
-  const attribute_id value_id_;
-};
-
-/**
  * @brief Templated helper class used to implement
  *        AggregationHandle::finalizeHashTable() by visiting each entry (i.e.
  *        GROUP) in a HashTable, finalizing the aggregation for the GROUP, and
@@ -438,68 +209,6 @@ class HashTableAggregateFinalizer {
 // Implementations of templated methods follow:
 
 template <typename HandleT,
-          typename StateT,
-          typename HashTableT>
-void AggregationConcreteHandle::aggregateValueAccessorIntoHashTableNullaryHelper(
-    ValueAccessor *accessor,
-    const std::vector<attribute_id> &group_by_key_ids,
-    const StateT &default_state,
-    AggregationStateHashTableBase *hash_table) const {
-  NullaryAggregationStateValueAccessorUpserter<HandleT, StateT>
-      upserter(static_cast<const HandleT&>(*this));
-  static_cast<HashTableT*>(hash_table)->upsertValueAccessorCompositeKey(
-      accessor,
-      group_by_key_ids,
-      true,
-      default_state,
-      &upserter);
-}
-
-template <typename HandleT,
-          typename StateT,
-          typename HashTableT>
-void AggregationConcreteHandle::aggregateValueAccessorIntoHashTableUnaryHelper(
-    ValueAccessor *accessor,
-    const attribute_id argument_id,
-    const std::vector<attribute_id> &group_by_key_ids,
-    const StateT &default_state,
-    AggregationStateHashTableBase *hash_table) const {
-  UnaryAggregationStateValueAccessorUpserter<HandleT, StateT>
-      upserter(static_cast<const HandleT&>(*this), argument_id);
-  static_cast<HashTableT*>(hash_table)->upsertValueAccessorCompositeKey(
-      accessor,
-      group_by_key_ids,
-      true,
-      default_state,
-      &upserter);
-}
-
-template <typename HandleT,
-          typename StateT>
-StateT* AggregationConcreteHandle::aggregateOnDistinctifyHashTableForSingleUnaryHelper(
-    const AggregationStateHashTableBase &distinctify_hash_table) const {
-  const HandleT& handle = static_cast<const HandleT&>(*this);
-  StateT *state = static_cast<StateT*>(createInitialState());
-
-  // A lambda function which will be called on each key from the distinctify
-  // hash table.
-  const auto aggregate_functor = [&handle, &state](const TypedValue &key,
-                                                   const bool &dumb_placeholder) {
-    // For each (unary) key in the distinctify hash table, aggregate the key
-    // into "state".
-    handle.iterateUnaryInl(state, key);
-  };
-
-  const AggregationStateHashTable<bool> &hash_table =
-      static_cast<const AggregationStateHashTable<bool>&>(distinctify_hash_table);
-  // Invoke the lambda function "aggregate_functor" on each key from the distinctify
-  // hash table.
-  hash_table.forEach(&aggregate_functor);
-
-  return state;
-}
-
-template <typename HandleT,
           typename StateT>
 StateT* AggregationConcreteHandle::aggregateOnDistinctifyHashTableForSingleUnaryHelperFast(
     const AggregationStateHashTableBase &distinctify_hash_table) const {
@@ -525,47 +234,11 @@ StateT* AggregationConcreteHandle::aggregateOnDistinctifyHashTableForSingleUnary
 }
 
 template <typename HandleT,
-          typename StateT,
-          typename HashTableT>
-void AggregationConcreteHandle::aggregateOnDistinctifyHashTableForGroupByUnaryHelper(
-    const AggregationStateHashTableBase &distinctify_hash_table,
-    const StateT &default_state,
-    AggregationStateHashTableBase *aggregation_hash_table) const {
-  const HandleT& handle = static_cast<const HandleT&>(*this);
-  HashTableT *target_hash_table = static_cast<HashTableT*>(aggregation_hash_table);
-
-  // A lambda function which will be called on each key-value pair from the
-  // distinctify hash table.
-  const auto aggregate_functor = [&handle, &target_hash_table, &default_state](
-      std::vector<TypedValue> &key,
-      const bool &dumb_placeholder) {
-    // For each (composite) key vector in the distinctify hash table with size N.
-    // The first N-1 entries are GROUP BY columns and the last entry is the argument
-    // to be aggregated on.
-    const TypedValue argument(std::move(key.back()));
-    key.pop_back();
-
-    // An upserter as lambda function for aggregating the argument into its
-    // GROUP BY group's entry inside aggregation_hash_table.
-    const auto upserter = [&handle, &argument](StateT *state) {
-      handle.iterateUnaryInl(state, argument);
-    };
-
-    target_hash_table->upsertCompositeKey(key, default_state, &upserter);
-  };
-
-  const AggregationStateHashTable<bool> &source_hash_table =
-      static_cast<const AggregationStateHashTable<bool>&>(distinctify_hash_table);
-  // Invoke the lambda function "aggregate_functor" on each composite key vector
-  // from the distinctify hash table.
-  source_hash_table.forEachCompositeKey(&aggregate_functor);
-}
-
-template <typename HandleT,
           typename HashTableT>
 void AggregationConcreteHandle::aggregateOnDistinctifyHashTableForGroupByUnaryHelperFast(
     const AggregationStateHashTableBase &distinctify_hash_table,
-    AggregationStateHashTableBase *aggregation_hash_table, int index) const {
+    AggregationStateHashTableBase *aggregation_hash_table,
+    int index) const {
   const HandleT& handle = static_cast<const HandleT&>(*this);
   HashTableT *target_hash_table = static_cast<HashTableT*>(aggregation_hash_table);
 
@@ -596,57 +269,6 @@ void AggregationConcreteHandle::aggregateOnDistinctifyHashTableForGroupByUnaryHe
   source_hash_table.forEachCompositeKeyFast(&aggregate_functor);
 }
 
-
-template <typename HandleT,
-          typename HashTableT>
-ColumnVector* AggregationConcreteHandle::finalizeHashTableHelper(
-    const Type &result_type,
-    const AggregationStateHashTableBase &hash_table,
-    std::vector<std::vector<TypedValue>> *group_by_keys) const {
-  const HandleT &handle = static_cast<const HandleT&>(*this);
-  const HashTableT &hash_table_concrete = static_cast<const HashTableT&>(hash_table);
-
-  if (group_by_keys->empty()) {
-    if (NativeColumnVector::UsableForType(result_type)) {
-      NativeColumnVector *result = new NativeColumnVector(result_type,
-                                                          hash_table_concrete.numEntries());
-      HashTableAggregateFinalizer<HandleT, NativeColumnVector> finalizer(
-          handle,
-          group_by_keys,
-          result);
-      hash_table_concrete.forEachCompositeKey(&finalizer);
-      return result;
-    } else {
-      IndirectColumnVector *result = new IndirectColumnVector(result_type,
-                                                              hash_table_concrete.numEntries());
-      HashTableAggregateFinalizer<HandleT, IndirectColumnVector> finalizer(
-          handle,
-          group_by_keys,
-          result);
-      hash_table_concrete.forEachCompositeKey(&finalizer);
-      return result;
-    }
-  } else {
-    if (NativeColumnVector::UsableForType(result_type)) {
-      NativeColumnVector *result = new NativeColumnVector(result_type,
-                                                          group_by_keys->size());
-      for (const std::vector<TypedValue> &group_by_key : *group_by_keys) {
-        result->appendTypedValue(finalizeGroupInHashTable<HandleT, HashTableT>(hash_table,
-                                                                               group_by_key));
-      }
-      return result;
-    } else {
-      IndirectColumnVector *result = new IndirectColumnVector(result_type,
-                                                              hash_table_concrete.numEntries());
-      for (const std::vector<TypedValue> &group_by_key : *group_by_keys) {
-        result->appendTypedValue(finalizeGroupInHashTable<HandleT, HashTableT>(hash_table,
-                                                                               group_by_key));
-      }
-      return result;
-    }
-  }
-}
-
 template <typename HandleT,
           typename HashTableT>
 ColumnVector* AggregationConcreteHandle::finalizeHashTableHelperFast(
@@ -700,38 +322,6 @@ ColumnVector* AggregationConcreteHandle::finalizeHashTableHelperFast(
   }
 }
 
-template <typename HandleT,
-          typename StateT,
-          typename HashTableT>
-void AggregationConcreteHandle::mergeGroupByHashTablesHelper(
-    const AggregationStateHashTableBase &source_hash_table,
-    AggregationStateHashTableBase *destination_hash_table) const {
-  const HandleT &handle = static_cast<const HandleT &>(*this);
-  const HashTableT &source_hash_table_concrete =
-      static_cast<const HashTableT &>(source_hash_table);
-
-  HashTableMerger<HandleT, StateT, HashTableT> merger(handle,
-                                                      destination_hash_table);
-
-  source_hash_table_concrete.forEachCompositeKey(&merger);
-}
-
-template <typename HandleT,
-          typename HashTableT>
-void AggregationConcreteHandle::mergeGroupByHashTablesHelperFast(
-    const AggregationStateHashTableBase &source_hash_table,
-    AggregationStateHashTableBase *destination_hash_table) const {
-  const HandleT &handle = static_cast<const HandleT &>(*this);
-  const HashTableT &source_hash_table_concrete =
-      static_cast<const HashTableT &>(source_hash_table);
-
-  HashTableMergerFast<HandleT, HashTableT> merger(handle,
-                                              destination_hash_table);
-
-  source_hash_table_concrete.forEachCompositeKeyFast(&merger);
-}
-
-
 }  // namespace quickstep
 
 #endif  // QUICKSTEP_EXPRESSIONS_AGGREGATION_AGGREGATION_CONCRETE_HANDLE_HPP_

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandle.hpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandle.hpp b/expressions/aggregation/AggregationHandle.hpp
index 368b94e..29b0355 100644
--- a/expressions/aggregation/AggregationHandle.hpp
+++ b/expressions/aggregation/AggregationHandle.hpp
@@ -265,7 +265,8 @@ class AggregationHandle {
    **/
   virtual ColumnVector* finalizeHashTable(
       const AggregationStateHashTableBase &hash_table,
-      std::vector<std::vector<TypedValue>> *group_by_keys, int index) const = 0;
+      std::vector<std::vector<TypedValue>> *group_by_keys,
+      int index) const = 0;
 
   /**
    * @brief Create a new HashTable for the distinctify step for DISTINCT aggregation.
@@ -347,26 +348,13 @@ class AggregationHandle {
    */
   virtual void aggregateOnDistinctifyHashTableForGroupBy(
       const AggregationStateHashTableBase &distinctify_hash_table,
-      AggregationStateHashTableBase *aggregation_hash_table, int index) const = 0;
-
-  /**
-   * @brief Merge two GROUP BY hash tables in one.
-   *
-   * @note Both the hash tables should have the same structure.
-   *
-   * @param source_hash_table The hash table which will get merged.
-   * @param destination_hash_table The hash table to which we will merge the
-   *        other hash table.
-   **/
-  virtual void mergeGroupByHashTables(
-      const AggregationStateHashTableBase &source_hash_table,
-      AggregationStateHashTableBase *destination_hash_table) const = 0;
+      AggregationStateHashTableBase *aggregation_hash_table,
+      int index) const = 0;
 
   virtual size_t getPayloadSize() const {return 1;}
-  virtual void setPayloadOffset(std::size_t) {}
-  virtual void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) {}
+  virtual void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) const {}
   virtual void mergeStatesFast(const uint8_t *src, uint8_t *dst) const {}
-  virtual void initPayload(uint8_t *byte_ptr) {}
+  virtual void initPayload(uint8_t *byte_ptr) const {}
   virtual void BlockUpdate() {}
   virtual void AllowUpdate() {}
 

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleAvg.cpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleAvg.cpp b/expressions/aggregation/AggregationHandleAvg.cpp
index 93adf55..45202e3 100644
--- a/expressions/aggregation/AggregationHandleAvg.cpp
+++ b/expressions/aggregation/AggregationHandleAvg.cpp
@@ -137,22 +137,6 @@ void AggregationHandleAvg::aggregateValueAccessorIntoHashTable(
     AggregationStateHashTableBase *hash_table) const {
   DCHECK_EQ(1u, argument_ids.size())
       << "Got wrong number of arguments for AVG: " << argument_ids.size();
-/*  aggregateValueAccessorIntoHashTableUnaryHelper<
-      AggregationHandleAvg,
-      AggregationStateAvg,
-      AggregationStateHashTable<AggregationStateAvg>>(
-          accessor,
-          argument_ids.front(),
-          group_by_key_ids,
-          blank_state_,
-          hash_table); */
-
-/*     static_cast<AggregationStateFastHashTable *>(hash_table)->upsertValueAccessorCompositeKeyFast(
-      argument_ids.front(),
-      accessor,
-      group_by_key_ids,
-      true,
-      const_cast<AggregationHandleAvg *>(this));*/
 }
 
 void AggregationHandleAvg::mergeStates(
@@ -214,20 +198,14 @@ AggregationState* AggregationHandleAvg::aggregateOnDistinctifyHashTableForSingle
 
 void AggregationHandleAvg::aggregateOnDistinctifyHashTableForGroupBy(
     const AggregationStateHashTableBase &distinctify_hash_table,
-    AggregationStateHashTableBase *aggregation_hash_table, int index) const {
+    AggregationStateHashTableBase *aggregation_hash_table,
+    int index) const {
   aggregateOnDistinctifyHashTableForGroupByUnaryHelperFast<
       AggregationHandleAvg,
       AggregationStateFastHashTable>(
           distinctify_hash_table,
-          aggregation_hash_table, index);
-}
-
-void AggregationHandleAvg::mergeGroupByHashTables(
-    const AggregationStateHashTableBase &source_hash_table,
-    AggregationStateHashTableBase *destination_hash_table) const {
-  mergeGroupByHashTablesHelperFast<AggregationHandleAvg,
-                               AggregationStateFastHashTable>(
-      source_hash_table, destination_hash_table);
+          aggregation_hash_table,
+          index);
 }
 
 }  // namespace quickstep

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleAvg.hpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleAvg.hpp b/expressions/aggregation/AggregationHandleAvg.hpp
index 3de0f2b..ecb0c6c 100644
--- a/expressions/aggregation/AggregationHandleAvg.hpp
+++ b/expressions/aggregation/AggregationHandleAvg.hpp
@@ -132,7 +132,7 @@ class AggregationHandleAvg : public AggregationConcreteHandle {
     ++(*count_ptr);
   }
 
-  inline void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) override {
+  inline void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) const override {
      if (block_update) return;
      iterateUnaryInlFast(arguments.front(), byte_ptr);
   }
@@ -145,7 +145,7 @@ class AggregationHandleAvg : public AggregationConcreteHandle {
       block_update = false;
   }
 
-  void initPayload(uint8_t *byte_ptr) override {
+  void initPayload(uint8_t *byte_ptr) const override {
     TypedValue *sum_ptr = reinterpret_cast<TypedValue *>(byte_ptr + blank_state_.sum_offset);
     std::int64_t *count_ptr = reinterpret_cast<std::int64_t *>(byte_ptr + blank_state_.count_offset);
     *sum_ptr = blank_state_.sum_;
@@ -217,11 +217,8 @@ class AggregationHandleAvg : public AggregationConcreteHandle {
    */
   void aggregateOnDistinctifyHashTableForGroupBy(
       const AggregationStateHashTableBase &distinctify_hash_table,
-      AggregationStateHashTableBase *aggregation_hash_table, int index) const override;
-
-  void mergeGroupByHashTables(
-      const AggregationStateHashTableBase &source_hash_table,
-      AggregationStateHashTableBase *destination_hash_table) const override;
+      AggregationStateHashTableBase *aggregation_hash_table,
+      int index) const override;
 
   size_t getPayloadSize() const override {
       return blank_state_.getPayloadSize();

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleCount.cpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleCount.cpp b/expressions/aggregation/AggregationHandleCount.cpp
index a1d5388..872d770 100644
--- a/expressions/aggregation/AggregationHandleCount.cpp
+++ b/expressions/aggregation/AggregationHandleCount.cpp
@@ -135,26 +135,9 @@ template <bool count_star, bool nullable_type>
   if (count_star) {
     DCHECK_EQ(0u, argument_ids.size())
         << "Got wrong number of arguments for COUNT(*): " << argument_ids.size();
-/*    aggregateValueAccessorIntoHashTableNullaryHelper<
-        AggregationHandleCount<count_star, nullable_type>,
-        AggregationStateCount,
-        AggregationStateHashTable<AggregationStateCount>>(
-            accessor,
-            group_by_key_ids,
-            AggregationStateCount(),
-            hash_table);*/
   } else {
     DCHECK_EQ(1u, argument_ids.size())
         << "Got wrong number of arguments for COUNT: " << argument_ids.size();
-/*    aggregateValueAccessorIntoHashTableUnaryHelper<
-        AggregationHandleCount<count_star, nullable_type>,
-        AggregationStateCount,
-        AggregationStateHashTable<AggregationStateCount>>(
-            accessor,
-            argument_ids.front(),
-            group_by_key_ids,
-            AggregationStateCount(),
-            hash_table); */
   }
 }
 
@@ -206,7 +189,8 @@ template <bool count_star, bool nullable_type>
 void AggregationHandleCount<count_star, nullable_type>
     ::aggregateOnDistinctifyHashTableForGroupBy(
         const AggregationStateHashTableBase &distinctify_hash_table,
-        AggregationStateHashTableBase *aggregation_hash_table, int index) const {
+        AggregationStateHashTableBase *aggregation_hash_table,
+        int index) const {
   DCHECK_EQ(count_star, false);
   aggregateOnDistinctifyHashTableForGroupByUnaryHelperFast<
       AggregationHandleCount<count_star, nullable_type>,
@@ -216,16 +200,6 @@ void AggregationHandleCount<count_star, nullable_type>
           index);
 }
 
-template <bool count_star, bool nullable_type>
-void AggregationHandleCount<count_star, nullable_type>::mergeGroupByHashTables(
-    const AggregationStateHashTableBase &source_hash_table,
-    AggregationStateHashTableBase *destination_hash_table) const {
-  mergeGroupByHashTablesHelperFast<
-      AggregationHandleCount,
-      AggregationStateFastHashTable>(source_hash_table,
-                                                        destination_hash_table);
-}
-
 // Explicitly instantiate and compile in the different versions of
 // AggregationHandleCount we need. Note that we do not compile a version with
 // 'count_star == true' and 'nullable_type == true', as that combination is

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleCount.hpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleCount.hpp b/expressions/aggregation/AggregationHandleCount.hpp
index 64d03a3..6594ed2 100644
--- a/expressions/aggregation/AggregationHandleCount.hpp
+++ b/expressions/aggregation/AggregationHandleCount.hpp
@@ -113,7 +113,7 @@ class AggregationHandleCount : public AggregationConcreteHandle {
     state->count_.fetch_add(1, std::memory_order_relaxed);
   }
 
-  inline void iterateNullaryInlFast(uint8_t *byte_ptr) {
+  inline void iterateNullaryInlFast(uint8_t *byte_ptr) const {
       std::int64_t *count_ptr = reinterpret_cast<std::int64_t *>(byte_ptr);
       (*count_ptr)++;
   }
@@ -134,7 +134,7 @@ class AggregationHandleCount : public AggregationConcreteHandle {
     }
   }
 
-  inline void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) override {
+  inline void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) const override {
      if (block_update) return;
      if (arguments.size())
          iterateUnaryInlFast(arguments.front(), byte_ptr);
@@ -150,7 +150,7 @@ class AggregationHandleCount : public AggregationConcreteHandle {
      block_update = false;
   }
 
-  void initPayload(uint8_t *byte_ptr) override {
+  void initPayload(uint8_t *byte_ptr) const override {
      std::int64_t *count_ptr = reinterpret_cast<std::int64_t *>(byte_ptr);
      *count_ptr = 0;
   }
@@ -217,11 +217,8 @@ class AggregationHandleCount : public AggregationConcreteHandle {
    */
   void aggregateOnDistinctifyHashTableForGroupBy(
       const AggregationStateHashTableBase &distinctify_hash_table,
-      AggregationStateHashTableBase *aggregation_hash_table, int index) const override;
-
-  void mergeGroupByHashTables(
-      const AggregationStateHashTableBase &source_hash_table,
-      AggregationStateHashTableBase *destination_hash_table) const override;
+      AggregationStateHashTableBase *aggregation_hash_table,
+      int index) const override;
 
   size_t getPayloadSize() const override {
       return sizeof(std::int64_t);

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleDistinct.cpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleDistinct.cpp b/expressions/aggregation/AggregationHandleDistinct.cpp
index cfcedcf..6eb3f47 100644
--- a/expressions/aggregation/AggregationHandleDistinct.cpp
+++ b/expressions/aggregation/AggregationHandleDistinct.cpp
@@ -63,14 +63,15 @@ void AggregationHandleDistinct::aggregateValueAccessorIntoHashTable(
 
 ColumnVector* AggregationHandleDistinct::finalizeHashTable(
     const AggregationStateHashTableBase &hash_table,
-    std::vector<std::vector<TypedValue>> *group_by_keys, int index) const {
+    std::vector<std::vector<TypedValue>> *group_by_keys,
+    int index) const {
   DCHECK(group_by_keys->empty());
 
   const auto keys_retriever = [&group_by_keys](std::vector<TypedValue> &group_by_key,
                                                const bool &dumb_placeholder) -> void {
     group_by_keys->emplace_back(std::move(group_by_key));
   };
-  static_cast<const AggregationStateFastHashTable&>(hash_table).forEachCompositeKey(&keys_retriever);
+  static_cast<const AggregationStateFastHashTable&>(hash_table).forEachCompositeKeyFast(&keys_retriever);
 
   return nullptr;
 }

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleDistinct.hpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleDistinct.hpp b/expressions/aggregation/AggregationHandleDistinct.hpp
index 2947d12..7bdb508 100644
--- a/expressions/aggregation/AggregationHandleDistinct.hpp
+++ b/expressions/aggregation/AggregationHandleDistinct.hpp
@@ -88,7 +88,8 @@ class AggregationHandleDistinct : public AggregationConcreteHandle {
 
   void aggregateOnDistinctifyHashTableForGroupBy(
       const AggregationStateHashTableBase &distinctify_hash_table,
-      AggregationStateHashTableBase *groupby_hash_table, int index) const override {
+      AggregationStateHashTableBase *groupby_hash_table,
+      int index) const override {
     LOG(FATAL) << "AggregationHandleDistinct does not support "
                << "aggregateOnDistinctifyHashTableForGroupBy().";
   }
@@ -107,14 +108,8 @@ class AggregationHandleDistinct : public AggregationConcreteHandle {
 
   ColumnVector* finalizeHashTable(
       const AggregationStateHashTableBase &hash_table,
-      std::vector<std::vector<TypedValue>> *group_by_keys, int index) const override;
-
-  void mergeGroupByHashTables(
-      const AggregationStateHashTableBase &source_hash_table,
-      AggregationStateHashTableBase *destination_hash_table) const override {
-    LOG(FATAL)
-        << "AggregationHandleDistinct does not support mergeGroupByHashTables";
-  }
+      std::vector<std::vector<TypedValue>> *group_by_keys,
+      int index) const override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(AggregationHandleDistinct);

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleMax.cpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleMax.cpp b/expressions/aggregation/AggregationHandleMax.cpp
index ef44742..d63564a 100644
--- a/expressions/aggregation/AggregationHandleMax.cpp
+++ b/expressions/aggregation/AggregationHandleMax.cpp
@@ -86,16 +86,6 @@ void AggregationHandleMax::aggregateValueAccessorIntoHashTable(
     AggregationStateHashTableBase *hash_table) const {
   DCHECK_EQ(1u, argument_ids.size())
       << "Got wrong number of arguments for MAX: " << argument_ids.size();
-
-/*  aggregateValueAccessorIntoHashTableUnaryHelper<
-      AggregationHandleMax,
-      AggregationStateMax,
-      AggregationStateHashTable<AggregationStateMax>>(
-          accessor,
-          argument_ids.front(),
-          group_by_key_ids,
-          AggregationStateMax(type_),
-          hash_table);*/
 }
 
 void AggregationHandleMax::mergeStates(
@@ -141,20 +131,14 @@ AggregationState* AggregationHandleMax::aggregateOnDistinctifyHashTableForSingle
 
 void AggregationHandleMax::aggregateOnDistinctifyHashTableForGroupBy(
     const AggregationStateHashTableBase &distinctify_hash_table,
-    AggregationStateHashTableBase *aggregation_hash_table, int index) const {
+    AggregationStateHashTableBase *aggregation_hash_table,
+    int index) const {
   aggregateOnDistinctifyHashTableForGroupByUnaryHelperFast<
       AggregationHandleMax,
       AggregationStateFastHashTable>(
           distinctify_hash_table,
-          aggregation_hash_table, index);
-}
-
-void AggregationHandleMax::mergeGroupByHashTables(
-    const AggregationStateHashTableBase &source_hash_table,
-    AggregationStateHashTableBase *destination_hash_table) const {
-  mergeGroupByHashTablesHelperFast<AggregationHandleMax,
-                               AggregationStateFastHashTable>(
-      source_hash_table, destination_hash_table);
+          aggregation_hash_table,
+          index);
 }
 
 }  // namespace quickstep

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleMax.hpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleMax.hpp b/expressions/aggregation/AggregationHandleMax.hpp
index d82b9a8..16c7458 100644
--- a/expressions/aggregation/AggregationHandleMax.hpp
+++ b/expressions/aggregation/AggregationHandleMax.hpp
@@ -112,7 +112,7 @@ class AggregationHandleMax : public AggregationConcreteHandle {
     compareAndUpdateFast(max_ptr, value);
   }
 
-  inline void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) override {
+  inline void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) const override {
     if (block_update) return;
     iterateUnaryInlFast(arguments.front(), byte_ptr);
   }
@@ -125,7 +125,7 @@ class AggregationHandleMax : public AggregationConcreteHandle {
       block_update = false;
   }
 
-  void initPayload(uint8_t *byte_ptr) override {
+  void initPayload(uint8_t *byte_ptr) const override {
     TypedValue *max_ptr = reinterpret_cast<TypedValue *>(byte_ptr);
     TypedValue t1 = (type_.getNullableVersion().makeNullValue());
     *max_ptr = t1;
@@ -184,11 +184,8 @@ class AggregationHandleMax : public AggregationConcreteHandle {
    */
   void aggregateOnDistinctifyHashTableForGroupBy(
       const AggregationStateHashTableBase &distinctify_hash_table,
-      AggregationStateHashTableBase *aggregation_hash_table, int index) const override;
-
-  void mergeGroupByHashTables(
-      const AggregationStateHashTableBase &source_hash_table,
-      AggregationStateHashTableBase *destination_hash_table) const override;
+      AggregationStateHashTableBase *aggregation_hash_table,
+      int index) const override;
 
   size_t getPayloadSize() const override {
       return sizeof(TypedValue);

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleMin.cpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleMin.cpp b/expressions/aggregation/AggregationHandleMin.cpp
index b6e46f4..3582adc 100644
--- a/expressions/aggregation/AggregationHandleMin.cpp
+++ b/expressions/aggregation/AggregationHandleMin.cpp
@@ -88,16 +88,6 @@ void AggregationHandleMin::aggregateValueAccessorIntoHashTable(
     AggregationStateHashTableBase *hash_table) const {
   DCHECK_EQ(1u, argument_ids.size())
       << "Got wrong number of arguments for MIN: " << argument_ids.size();
-
-/*  aggregateValueAccessorIntoHashTableUnaryHelper<
-      AggregationHandleMin,
-      AggregationStateMin,
-      AggregationStateHashTable<AggregationStateMin>>(
-          accessor,
-          argument_ids.front(),
-          group_by_key_ids,
-          AggregationStateMin(type_),
-          hash_table);*/
 }
 
 void AggregationHandleMin::mergeStates(
@@ -144,20 +134,14 @@ AggregationState* AggregationHandleMin::aggregateOnDistinctifyHashTableForSingle
 
 void AggregationHandleMin::aggregateOnDistinctifyHashTableForGroupBy(
     const AggregationStateHashTableBase &distinctify_hash_table,
-    AggregationStateHashTableBase *aggregation_hash_table, int index) const {
+    AggregationStateHashTableBase *aggregation_hash_table,
+    int index) const {
   aggregateOnDistinctifyHashTableForGroupByUnaryHelperFast<
       AggregationHandleMin,
       AggregationStateFastHashTable>(
           distinctify_hash_table,
-          aggregation_hash_table, index);
-}
-
-void AggregationHandleMin::mergeGroupByHashTables(
-    const AggregationStateHashTableBase &source_hash_table,
-    AggregationStateHashTableBase *destination_hash_table) const {
-  mergeGroupByHashTablesHelperFast<AggregationHandleMin,
-                               AggregationStateFastHashTable>(
-      source_hash_table, destination_hash_table);
+          aggregation_hash_table,
+          index);
 }
 
 }  // namespace quickstep

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleMin.hpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleMin.hpp b/expressions/aggregation/AggregationHandleMin.hpp
index adf79d7..119102b 100644
--- a/expressions/aggregation/AggregationHandleMin.hpp
+++ b/expressions/aggregation/AggregationHandleMin.hpp
@@ -116,7 +116,7 @@ class AggregationHandleMin : public AggregationConcreteHandle {
       compareAndUpdateFast(min_ptr, value);
   }
 
-  inline void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) override {
+  inline void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) const override {
     if (block_update) return;
     iterateUnaryInlFast(arguments.front(), byte_ptr);
   }
@@ -129,7 +129,7 @@ class AggregationHandleMin : public AggregationConcreteHandle {
       block_update = false;
   }
 
-  void initPayload(uint8_t *byte_ptr) override {
+  void initPayload(uint8_t *byte_ptr) const override {
     TypedValue *min_ptr = reinterpret_cast<TypedValue *>(byte_ptr);
     TypedValue t1 = (type_.getNullableVersion().makeNullValue());
     *min_ptr = t1;
@@ -187,11 +187,8 @@ class AggregationHandleMin : public AggregationConcreteHandle {
    */
   void aggregateOnDistinctifyHashTableForGroupBy(
       const AggregationStateHashTableBase &distinctify_hash_table,
-      AggregationStateHashTableBase *aggregation_hash_table, int index) const override;
-
-  void mergeGroupByHashTables(
-      const AggregationStateHashTableBase &source_hash_table,
-      AggregationStateHashTableBase *destination_hash_table) const override;
+      AggregationStateHashTableBase *aggregation_hash_table,
+      int index) const override;
 
   size_t getPayloadSize() const override {
       return sizeof(TypedValue);

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleSum.cpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleSum.cpp b/expressions/aggregation/AggregationHandleSum.cpp
index 1b0e355..e625fb1 100644
--- a/expressions/aggregation/AggregationHandleSum.cpp
+++ b/expressions/aggregation/AggregationHandleSum.cpp
@@ -125,16 +125,6 @@ void AggregationHandleSum::aggregateValueAccessorIntoHashTable(
     AggregationStateHashTableBase *hash_table) const {
   DCHECK_EQ(1u, argument_ids.size())
       << "Got wrong number of arguments for SUM: " << argument_ids.size();
-
-/*  aggregateValueAccessorIntoHashTableUnaryHelper<
-      AggregationHandleSum,
-      AggregationStateSum,
-      AggregationStateHashTable<AggregationStateSum>>(
-          accessor,
-          argument_ids.front(),
-          group_by_key_ids,
-          blank_state_,
-          hash_table);*/
 }
 
 void AggregationHandleSum::mergeStates(
@@ -192,7 +182,8 @@ AggregationState* AggregationHandleSum::aggregateOnDistinctifyHashTableForSingle
 
 void AggregationHandleSum::aggregateOnDistinctifyHashTableForGroupBy(
     const AggregationStateHashTableBase &distinctify_hash_table,
-    AggregationStateHashTableBase *aggregation_hash_table, int index) const {
+    AggregationStateHashTableBase *aggregation_hash_table,
+    int index) const {
   aggregateOnDistinctifyHashTableForGroupByUnaryHelperFast<
       AggregationHandleSum,
       AggregationStateFastHashTable>(
@@ -201,12 +192,4 @@ void AggregationHandleSum::aggregateOnDistinctifyHashTableForGroupBy(
           index);
 }
 
-void AggregationHandleSum::mergeGroupByHashTables(
-    const AggregationStateHashTableBase &source_hash_table,
-    AggregationStateHashTableBase *destination_hash_table) const {
-  mergeGroupByHashTablesHelperFast<AggregationHandleSum,
-                               AggregationStateFastHashTable>(
-      source_hash_table, destination_hash_table);
-}
-
 }  // namespace quickstep

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/AggregationHandleSum.hpp
----------------------------------------------------------------------
diff --git a/expressions/aggregation/AggregationHandleSum.hpp b/expressions/aggregation/AggregationHandleSum.hpp
index 53f43ce..bf0eab0 100644
--- a/expressions/aggregation/AggregationHandleSum.hpp
+++ b/expressions/aggregation/AggregationHandleSum.hpp
@@ -127,7 +127,7 @@ class AggregationHandleSum : public AggregationConcreteHandle {
     *null_ptr = false;
   }
 
-  inline void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) override {
+  inline void iterateInlFast(const std::vector<TypedValue> &arguments, uint8_t *byte_ptr) const override {
      if (block_update) return;
      iterateUnaryInlFast(arguments.front(), byte_ptr);
   }
@@ -140,7 +140,7 @@ class AggregationHandleSum : public AggregationConcreteHandle {
       block_update = false;
   }
 
-  void initPayload(uint8_t *byte_ptr) override {
+  void initPayload(uint8_t *byte_ptr) const override {
     TypedValue *sum_ptr = reinterpret_cast<TypedValue *>(byte_ptr + blank_state_.sum_offset);
     bool *null_ptr = reinterpret_cast<bool *>(byte_ptr + blank_state_.null_offset);
     *sum_ptr = blank_state_.sum_;
@@ -182,7 +182,8 @@ class AggregationHandleSum : public AggregationConcreteHandle {
 
   ColumnVector* finalizeHashTable(
       const AggregationStateHashTableBase &hash_table,
-      std::vector<std::vector<TypedValue>> *group_by_keys, int index) const override;
+      std::vector<std::vector<TypedValue>> *group_by_keys,
+      int index) const override;
 
   /**
    * @brief Implementation of AggregationHandle::aggregateOnDistinctifyHashTableForSingle()
@@ -197,11 +198,8 @@ class AggregationHandleSum : public AggregationConcreteHandle {
    */
   void aggregateOnDistinctifyHashTableForGroupBy(
       const AggregationStateHashTableBase &distinctify_hash_table,
-      AggregationStateHashTableBase *aggregation_hash_table, int index) const override;
-
-  void mergeGroupByHashTables(
-      const AggregationStateHashTableBase &source_hash_table,
-      AggregationStateHashTableBase *destination_hash_table) const override;
+      AggregationStateHashTableBase *aggregation_hash_table,
+      int index) const override;
 
   size_t getPayloadSize() const override {
       return blank_state_.getPayloadSize();

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/expressions/aggregation/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/expressions/aggregation/CMakeLists.txt b/expressions/aggregation/CMakeLists.txt
index 9a9c1a8..33ce6e9 100644
--- a/expressions/aggregation/CMakeLists.txt
+++ b/expressions/aggregation/CMakeLists.txt
@@ -278,45 +278,45 @@ target_link_libraries(quickstep_expressions_aggregation
 # Tests:
 
 # Unified executable to ammortize cost of linking.
-add_executable(AggregationHandle_tests
-               "${CMAKE_CURRENT_SOURCE_DIR}/tests/AggregationHandleAvg_unittest.cpp"
-               "${CMAKE_CURRENT_SOURCE_DIR}/tests/AggregationHandleCount_unittest.cpp"
-               "${CMAKE_CURRENT_SOURCE_DIR}/tests/AggregationHandleMax_unittest.cpp"
-               "${CMAKE_CURRENT_SOURCE_DIR}/tests/AggregationHandleMin_unittest.cpp"
-               "${CMAKE_CURRENT_SOURCE_DIR}/tests/AggregationHandleSum_unittest.cpp")
-target_link_libraries(AggregationHandle_tests
-                      gtest
-                      gtest_main
-                      quickstep_catalog_CatalogTypedefs
-                      quickstep_expressions_aggregation_AggregateFunction
-                      quickstep_expressions_aggregation_AggregateFunctionFactory
-                      quickstep_expressions_aggregation_AggregationHandle
-                      quickstep_expressions_aggregation_AggregationHandleAvg
-                      quickstep_expressions_aggregation_AggregationHandleCount
-                      quickstep_expressions_aggregation_AggregationHandleMax
-                      quickstep_expressions_aggregation_AggregationHandleMin
-                      quickstep_expressions_aggregation_AggregationHandleSum
-                      quickstep_expressions_aggregation_AggregationID
-                      quickstep_storage_HashTableBase
-                      quickstep_storage_StorageManager
-                      quickstep_types_CharType
-                      quickstep_types_DateOperatorOverloads
-                      quickstep_types_DatetimeIntervalType
-                      quickstep_types_DatetimeType
-                      quickstep_types_DoubleType
-                      quickstep_types_FloatType
-                      quickstep_types_IntType
-                      quickstep_types_IntervalLit
-                      quickstep_types_LongType
-                      quickstep_types_Type
-                      quickstep_types_TypeFactory
-                      quickstep_types_TypeID
-                      quickstep_types_TypedValue
-                      quickstep_types_VarCharType
-                      quickstep_types_YearMonthIntervalType
-                      quickstep_types_containers_ColumnVector
-                      quickstep_types_containers_ColumnVectorsValueAccessor
-                      quickstep_types_operations_comparisons_Comparison
-                      quickstep_types_operations_comparisons_ComparisonFactory
-                      quickstep_types_operations_comparisons_ComparisonID)
+# add_executable(AggregationHandle_tests
+#               "${CMAKE_CURRENT_SOURCE_DIR}/tests/AggregationHandleAvg_unittest.cpp"
+#               "${CMAKE_CURRENT_SOURCE_DIR}/tests/AggregationHandleCount_unittest.cpp"
+#               "${CMAKE_CURRENT_SOURCE_DIR}/tests/AggregationHandleMax_unittest.cpp"
+#               "${CMAKE_CURRENT_SOURCE_DIR}/tests/AggregationHandleMin_unittest.cpp"
+#               "${CMAKE_CURRENT_SOURCE_DIR}/tests/AggregationHandleSum_unittest.cpp")
+# target_link_libraries(AggregationHandle_tests
+#                      gtest
+#                      gtest_main
+#                      quickstep_catalog_CatalogTypedefs
+#                      quickstep_expressions_aggregation_AggregateFunction
+#                      quickstep_expressions_aggregation_AggregateFunctionFactory
+#                      quickstep_expressions_aggregation_AggregationHandle
+#                      quickstep_expressions_aggregation_AggregationHandleAvg
+#                      quickstep_expressions_aggregation_AggregationHandleCount
+#                      quickstep_expressions_aggregation_AggregationHandleMax
+#                      quickstep_expressions_aggregation_AggregationHandleMin
+#                      quickstep_expressions_aggregation_AggregationHandleSum
+#                      quickstep_expressions_aggregation_AggregationID
+#                      quickstep_storage_HashTableBase
+#                      quickstep_storage_StorageManager
+#                      quickstep_types_CharType
+#                      quickstep_types_DateOperatorOverloads
+#                      quickstep_types_DatetimeIntervalType
+#                      quickstep_types_DatetimeType
+#                      quickstep_types_DoubleType
+#                      quickstep_types_FloatType
+#                      quickstep_types_IntType
+#                      quickstep_types_IntervalLit
+#                      quickstep_types_LongType
+#                      quickstep_types_Type
+#                      quickstep_types_TypeFactory
+#                      quickstep_types_TypeID
+#                      quickstep_types_TypedValue
+#                      quickstep_types_VarCharType
+#                      quickstep_types_YearMonthIntervalType
+#                      quickstep_types_containers_ColumnVector
+#                      quickstep_types_containers_ColumnVectorsValueAccessor
+#                      quickstep_types_operations_comparisons_Comparison
+#                      quickstep_types_operations_comparisons_ComparisonFactory
+#                      quickstep_types_operations_comparisons_ComparisonID)
 #add_test(AggregationHandle_tests AggregationHandle_tests)

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/storage/AggregationOperationState.cpp
----------------------------------------------------------------------
diff --git a/storage/AggregationOperationState.cpp b/storage/AggregationOperationState.cpp
index ab1430c..8019473 100644
--- a/storage/AggregationOperationState.cpp
+++ b/storage/AggregationOperationState.cpp
@@ -461,7 +461,7 @@ void AggregationOperationState::finalizeSingleState(InsertDestination *output_de
 
 void AggregationOperationState::mergeGroupByHashTables(AggregationStateHashTableBase *src,
                                                        AggregationStateHashTableBase *dst) {
-    HashTableMergerNewFast merger(dst);
+    HashTableMergerFast merger(dst);
     (static_cast<FastHashTable<true, false, true, false> *>(src))->forEachCompositeKeyFast(&merger);
 }
 
@@ -478,18 +478,15 @@ void AggregationOperationState::finalizeHashTable(InsertDestination *output_dest
   // e.g. Keep merging entries from smaller hash tables to larger.
 
   auto *hash_tables = group_by_hashtable_pools_[0]->getAllHashTables();
-  for (std::size_t agg_idx = 0; agg_idx < handles_.size(); ++agg_idx) {
-    if (hash_tables->size() > 1) {
-      for (int hash_table_index = 0;
-           hash_table_index < static_cast<int>(hash_tables->size() - 1);
-           ++hash_table_index) {
-        // Merge each hash table to the last hash table.
-        mergeGroupByHashTables(
-            (*hash_tables)[hash_table_index].get(),
-            hash_tables->back().get());
-      }
+  if (hash_tables->size() > 1) {
+    for (int hash_table_index = 0;
+         hash_table_index < static_cast<int>(hash_tables->size() - 1);
+         ++hash_table_index) {
+      // Merge each hash table to the last hash table.
+      mergeGroupByHashTables(
+          (*hash_tables)[hash_table_index].get(),
+          hash_tables->back().get());
     }
-    break;
   }
 
   // Collect per-aggregate finalized values.

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/storage/FastHashTable.hpp
----------------------------------------------------------------------
diff --git a/storage/FastHashTable.hpp b/storage/FastHashTable.hpp
index c659a20..909fcc0 100644
--- a/storage/FastHashTable.hpp
+++ b/storage/FastHashTable.hpp
@@ -236,8 +236,6 @@ class FastHashTable : public HashTableBase<resizable,
    *         resizable is false and storage space for the hash table has been
    *         exhausted.
    **/
-  HashTablePutResult putCompositeKey(const std::vector<TypedValue> &key,
-                                     const uint8_t &value);
 
   HashTablePutResult putCompositeKeyFast(const std::vector<TypedValue> &key,
                                      const uint8_t *value_ptr);
@@ -380,7 +378,7 @@ class FastHashTable : public HashTableBase<resizable,
    **/
   template <typename FunctorT>
   bool upsert(const TypedValue &key,
-              const uint8_t &initial_value,
+              const uint8_t *initial_value_ptr,
               FunctorT *functor);
 
   /**
@@ -422,12 +420,6 @@ class FastHashTable : public HashTableBase<resizable,
    *         enough space to insert a new entry in this HashTable.
    **/
   template <typename FunctorT>
-  bool upsertCompositeKey(const std::vector<TypedValue> &key,
-                          const uint8_t &initial_value,
-                          FunctorT *functor);
-
-
-  template <typename FunctorT>
   bool upsertCompositeKeyFast(const std::vector<TypedValue> &key,
                           const uint8_t *init_value_ptr,
                           FunctorT *functor);
@@ -435,9 +427,10 @@ class FastHashTable : public HashTableBase<resizable,
   template <typename FunctorT>
   bool upsertCompositeKeyFast(const std::vector<TypedValue> &key,
                           const uint8_t *init_value_ptr,
-                          FunctorT *functor, int index);
+                          FunctorT *functor,
+                          int index);
 
-  bool upsertCompositeKeyNewFast(const std::vector<TypedValue> &key,
+  bool upsertCompositeKeyFast(const std::vector<TypedValue> &key,
                           const uint8_t *init_value_ptr,
                           const uint8_t *source_state);
 
@@ -488,14 +481,6 @@ class FastHashTable : public HashTableBase<resizable,
    *         accessor's iteration will be left on the first tuple which could
    *         not be inserted).
    **/
-  template <typename FunctorT>
-  bool upsertValueAccessor(ValueAccessor *accessor,
-                           const attribute_id key_attr_id,
-                           const bool check_for_null_keys,
-                           const uint8_t &initial_value,
-                           FunctorT *functor);
-
-
   bool upsertValueAccessorFast(const std::vector<std::vector<attribute_id>> &argument_ids,
                            ValueAccessor *accessor,
                            const attribute_id key_attr_id,
@@ -548,14 +533,6 @@ class FastHashTable : public HashTableBase<resizable,
    *         accessor's iteration will be left on the first tuple which could
    *         not be inserted).
    **/
-  template <typename FunctorT>
-  bool upsertValueAccessorCompositeKey(
-      ValueAccessor *accessor,
-      const std::vector<attribute_id> &key_attr_ids,
-      const bool check_for_null_keys,
-      const uint8_t &initial_value,
-      FunctorT *functor);
-
   bool upsertValueAccessorCompositeKeyFast(
       const std::vector<std::vector<attribute_id>> &argument,
       ValueAccessor *accessor,
@@ -631,7 +608,8 @@ class FastHashTable : public HashTableBase<resizable,
    *         Otherwise, return NULL.
    **/
   virtual const uint8_t* getSingleCompositeKey(const std::vector<TypedValue> &key) const = 0;
-  virtual const uint8_t* getSingleCompositeKey(const std::vector<TypedValue> &key, int index) const = 0;
+  virtual const uint8_t* getSingleCompositeKey(const std::vector<TypedValue> &key,
+                                               int index) const = 0;
 
   /**
    * @brief Lookup a key against this hash table to find matching entries.
@@ -1002,13 +980,12 @@ class FastHashTable : public HashTableBase<resizable,
    * @return The number of key-value pairs visited.
    **/
   template <typename FunctorT>
-  std::size_t forEachCompositeKey(FunctorT *functor) const;
-
-  template <typename FunctorT>
   std::size_t forEachCompositeKeyFast(FunctorT *functor) const;
 
   template <typename FunctorT>
-  std::size_t forEachCompositeKeyFast(FunctorT *functor, int index) const;
+  std::size_t forEachCompositeKeyFast(FunctorT *functor,
+                                      int index) const;
+
   /**
    * @brief A call to this function will cause a bloom filter to be built
    *        during the build phase of this hash table.
@@ -1196,10 +1173,6 @@ class FastHashTable : public HashTableBase<resizable,
                                          const std::size_t variable_key_size,
                                          const uint8_t &value,
                                          HashTablePreallocationState *prealloc_state) = 0;
-  virtual HashTablePutResult putCompositeKeyInternal(const std::vector<TypedValue> &key,
-                                                     const std::size_t variable_key_size,
-                                                     const uint8_t &value,
-                                                     HashTablePreallocationState *prealloc_state) = 0;
 
   virtual HashTablePutResult putCompositeKeyInternalFast(const std::vector<TypedValue> &key,
                                                      const std::size_t variable_key_size,
@@ -1213,15 +1186,9 @@ class FastHashTable : public HashTableBase<resizable,
   // return NULL if there is not enough space to insert a new key, in which
   // case a resizable HashTable should release the 'resize_shared_mutex_' and
   // call resize(), then try again.
-  virtual uint8_t* upsertInternal(const TypedValue &key,
-                                 const std::size_t variable_key_size,
-                                 const uint8_t &initial_value) = 0;
   virtual uint8_t* upsertInternalFast(const TypedValue &key,
-                                 const std::uint8_t *init_value_ptr,
-                                 const std::size_t variable_key_size) = 0;
-  virtual uint8_t* upsertCompositeKeyInternal(const std::vector<TypedValue> &key,
-                                             const std::size_t variable_key_size,
-                                             const uint8_t &initial_value) = 0;
+                                 const std::size_t variable_key_size,
+                                 const std::uint8_t *init_value_ptr) = 0;
 
   virtual uint8_t* upsertCompositeKeyInternalFast(const std::vector<TypedValue> &key,
                                                   const std::uint8_t *init_value_ptr,
@@ -1423,31 +1390,6 @@ template <bool resizable,
           bool force_key_copy,
           bool allow_duplicate_keys>
 HashTablePutResult FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
-    ::putCompositeKey(const std::vector<TypedValue> &key,
-                      const uint8_t& value) {
-  const std::size_t variable_size = calculateVariableLengthCompositeKeyCopySize(key);
-  if (resizable) {
-    HashTablePutResult result = HashTablePutResult::kOutOfSpace;
-    while (result == HashTablePutResult::kOutOfSpace) {
-      {
-        SpinSharedMutexSharedLock<true> lock(resize_shared_mutex_);
-        result = putCompositeKeyInternal(key, variable_size, value, nullptr);
-      }
-      if (result == HashTablePutResult::kOutOfSpace) {
-        resize(0, variable_size);
-      }
-    }
-    return result;
-  } else {
-    return putCompositeKeyInternal(key, variable_size, value, nullptr);
-  }
-}
-
-template <bool resizable,
-          bool serializable,
-          bool force_key_copy,
-          bool allow_duplicate_keys>
-HashTablePutResult FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
     ::putCompositeKeyFast(const std::vector<TypedValue> &key,
                       const std::uint8_t* init_value_ptr) {
   const std::size_t variable_size = calculateVariableLengthCompositeKeyCopySize(key);
@@ -1713,7 +1655,7 @@ template <bool resizable,
 template <typename FunctorT>
 bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
     ::upsert(const TypedValue &key,
-             const uint8_t &initial_value,
+             const uint8_t *initial_value_ptr,
              FunctorT *functor) {
   DEBUG_ASSERT(!allow_duplicate_keys);
   const std::size_t variable_size = (force_key_copy && !scalar_key_inline_) ? key.getDataSize() : 0;
@@ -1721,7 +1663,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
     for (;;) {
       {
         SpinSharedMutexSharedLock<true> resize_lock(resize_shared_mutex_);
-        uint8_t *value = upsertInternal(key, variable_size, initial_value);
+        uint8_t *value = upsertInternalFast(key, variable_size, initial_value_ptr);
         if (value != nullptr) {
           (*functor)(value);
           return true;
@@ -1730,41 +1672,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
       resize(0, force_key_copy && !scalar_key_inline_ ? key.getDataSize() : 0);
     }
   } else {
-    uint8_t *value = upsertInternal(key, variable_size, initial_value);
-    if (value == nullptr) {
-      return false;
-    } else {
-      (*functor)(value);
-      return true;
-    }
-  }
-}
-
-template <bool resizable,
-          bool serializable,
-          bool force_key_copy,
-          bool allow_duplicate_keys>
-template <typename FunctorT>
-bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
-    ::upsertCompositeKey(const std::vector<TypedValue> &key,
-                         const uint8_t &initial_value,
-                         FunctorT *functor) {
-  DEBUG_ASSERT(!allow_duplicate_keys);
-  const std::size_t variable_size = calculateVariableLengthCompositeKeyCopySize(key);
-  if (resizable) {
-    for (;;) {
-      {
-        SpinSharedMutexSharedLock<true> resize_lock(resize_shared_mutex_);
-        uint8_t *value = upsertCompositeKeyInternal(key, variable_size, initial_value);
-        if (value != nullptr) {
-          (*functor)(value);
-          return true;
-        }
-      }
-      resize(0, variable_size);
-    }
-  } else {
-    uint8_t *value = upsertCompositeKeyInternal(key, variable_size, initial_value);
+    uint8_t *value = upsertInternalFast(key, variable_size, initial_value_ptr);
     if (value == nullptr) {
       return false;
     } else {
@@ -1774,7 +1682,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
   }
 }
 
-class HashTableMergerNewFast {
+class HashTableMergerFast {
  public:
   /**
    * @brief Constructor
@@ -1783,7 +1691,7 @@ class HashTableMergerNewFast {
    * @param destination_hash_table The destination hash table to which other
    *        hash tables will be merged.
    **/
-  explicit HashTableMergerNewFast(AggregationStateHashTableBase *destination_hash_table)
+  explicit HashTableMergerFast(AggregationStateHashTableBase *destination_hash_table)
       : destination_hash_table_(static_cast<FastHashTable<true, false, true, false> *>(destination_hash_table)) {}
 
   /**
@@ -1801,7 +1709,7 @@ class HashTableMergerNewFast {
       // The CHECK is required as upsertCompositeKey can return false if the
       // hash table runs out of space during the upsert process. The ideal
       // solution will be to retry again if the upsert fails.
-      CHECK(destination_hash_table_->upsertCompositeKeyNewFast(
+      CHECK(destination_hash_table_->upsertCompositeKeyFast(
           group_by_key, original_state, source_state));
     } else {
       destination_hash_table_->putCompositeKeyFast(group_by_key, source_state);
@@ -1811,7 +1719,7 @@ class HashTableMergerNewFast {
  private:
   FastHashTable<true, false, true, false> *destination_hash_table_;
 
-  DISALLOW_COPY_AND_ASSIGN(HashTableMergerNewFast);
+  DISALLOW_COPY_AND_ASSIGN(HashTableMergerFast);
 };
 
 
@@ -1857,7 +1765,8 @@ template <typename FunctorT>
 bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
     ::upsertCompositeKeyFast(const std::vector<TypedValue> &key,
                          const std::uint8_t *init_value_ptr,
-                         FunctorT *functor, int index) {
+                         FunctorT *functor,
+                         int index) {
   DEBUG_ASSERT(!allow_duplicate_keys);
   const std::size_t variable_size = calculateVariableLengthCompositeKeyCopySize(key);
   if (resizable) {
@@ -1889,7 +1798,7 @@ template <bool resizable,
           bool force_key_copy,
           bool allow_duplicate_keys>
 bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
-    ::upsertCompositeKeyNewFast(const std::vector<TypedValue> &key,
+    ::upsertCompositeKeyFast(const std::vector<TypedValue> &key,
                          const std::uint8_t *init_value_ptr,
                          const std::uint8_t *source_state) {
   DEBUG_ASSERT(!allow_duplicate_keys);
@@ -1927,68 +1836,6 @@ template <bool resizable,
           bool serializable,
           bool force_key_copy,
           bool allow_duplicate_keys>
-template <typename FunctorT>
-bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
-    ::upsertValueAccessor(ValueAccessor *accessor,
-                          const attribute_id key_attr_id,
-                          const bool check_for_null_keys,
-                          const uint8_t &initial_value,
-                          FunctorT *functor) {
-  DEBUG_ASSERT(!allow_duplicate_keys);
-  std::size_t variable_size;
-  return InvokeOnAnyValueAccessor(
-      accessor,
-      [&](auto *accessor) -> bool {  // NOLINT(build/c++11)
-    if (resizable) {
-      bool continuing = true;
-      while (continuing) {
-        {
-          continuing = false;
-          SpinSharedMutexSharedLock<true> lock(resize_shared_mutex_);
-          while (accessor->next()) {
-            TypedValue key = accessor->getTypedValue(key_attr_id);
-            if (check_for_null_keys && key.isNull()) {
-              continue;
-            }
-            variable_size = (force_key_copy && !scalar_key_inline_) ? key.getDataSize() : 0;
-            uint8_t *value = this->upsertInternal(key, variable_size, initial_value);
-            if (value == nullptr) {
-              continuing = true;
-              break;
-            } else {
-              (*functor)(*accessor, value);
-            }
-          }
-        }
-        if (continuing) {
-          this->resize(0, variable_size);
-          accessor->previous();
-        }
-      }
-    } else {
-      while (accessor->next()) {
-        TypedValue key = accessor->getTypedValue(key_attr_id);
-        if (check_for_null_keys && key.isNull()) {
-          continue;
-        }
-        variable_size = (force_key_copy && !scalar_key_inline_) ? key.getDataSize() : 0;
-        uint8_t *value = this->upsertInternal(key, variable_size, initial_value);
-        if (value == nullptr) {
-          return false;
-        } else {
-          (*functor)(*accessor, value);
-        }
-      }
-    }
-
-    return true;
-  });
-}
-
-template <bool resizable,
-          bool serializable,
-          bool force_key_copy,
-          bool allow_duplicate_keys>
 bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
     ::upsertValueAccessorFast(const std::vector<std::vector<attribute_id>> &argument_ids,
                           ValueAccessor *accessor,
@@ -2012,7 +1859,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
               continue;
             }
             variable_size = (force_key_copy && !scalar_key_inline_) ? key.getDataSize() : 0;
-            uint8_t *value = this->upsertInternalFast(key, nullptr, variable_size);
+            uint8_t *value = this->upsertInternalFast(key, variable_size, nullptr);
             if (value == nullptr) {
               continuing = true;
               break;
@@ -2040,7 +1887,7 @@ bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys
           continue;
         }
         variable_size = (force_key_copy && !scalar_key_inline_) ? key.getDataSize() : 0;
-        uint8_t *value = this->upsertInternalFast(key, nullptr, variable_size);
+        uint8_t *value = this->upsertInternalFast(key, variable_size, nullptr);
         if (value == nullptr) {
           return false;
         } else {
@@ -2064,78 +1911,6 @@ template <bool resizable,
           bool serializable,
           bool force_key_copy,
           bool allow_duplicate_keys>
-template <typename FunctorT>
-bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
-    ::upsertValueAccessorCompositeKey(ValueAccessor *accessor,
-                                      const std::vector<attribute_id> &key_attr_ids,
-                                      const bool check_for_null_keys,
-                                      const uint8_t &initial_value,
-                                      FunctorT *functor) {
-  DEBUG_ASSERT(!allow_duplicate_keys);
-  std::size_t variable_size;
-  std::vector<TypedValue> key_vector;
-  key_vector.resize(key_attr_ids.size());
-  return InvokeOnAnyValueAccessor(
-      accessor,
-      [&](auto *accessor) -> bool {  // NOLINT(build/c++11)
-    if (resizable) {
-      bool continuing = true;
-      while (continuing) {
-        {
-          continuing = false;
-          SpinSharedMutexSharedLock<true> lock(resize_shared_mutex_);
-          while (accessor->next()) {
-            if (this->GetCompositeKeyFromValueAccessor(*accessor,
-                                                       key_attr_ids,
-                                                       check_for_null_keys,
-                                                       &key_vector)) {
-              continue;
-            }
-            variable_size = this->calculateVariableLengthCompositeKeyCopySize(key_vector);
-            uint8_t *value = this->upsertCompositeKeyInternal(key_vector,
-                                                             variable_size,
-                                                             initial_value);
-            if (value == nullptr) {
-              continuing = true;
-              break;
-            } else {
-              (*functor)(*accessor, value);
-            }
-          }
-        }
-        if (continuing) {
-          this->resize(0, variable_size);
-          accessor->previous();
-        }
-      }
-    } else {
-      while (accessor->next()) {
-        if (this->GetCompositeKeyFromValueAccessor(*accessor,
-                                                   key_attr_ids,
-                                                   check_for_null_keys,
-                                                   &key_vector)) {
-          continue;
-        }
-        variable_size = this->calculateVariableLengthCompositeKeyCopySize(key_vector);
-        uint8_t *value = this->upsertCompositeKeyInternal(key_vector,
-                                                         variable_size,
-                                                         initial_value);
-        if (value == nullptr) {
-          return false;
-        } else {
-          (*functor)(*accessor, value);
-        }
-      }
-    }
-
-    return true;
-  });
-}
-
-template <bool resizable,
-          bool serializable,
-          bool force_key_copy,
-          bool allow_duplicate_keys>
 bool FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
     ::upsertValueAccessorCompositeKeyFast(const std::vector<std::vector<attribute_id>> &argument_ids,
                                       ValueAccessor *accessor,
@@ -2514,25 +2289,6 @@ template <bool resizable,
           bool allow_duplicate_keys>
 template <typename FunctorT>
 std::size_t FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
-    ::forEachCompositeKey(FunctorT *functor) const {
-  std::size_t entries_visited = 0;
-  std::size_t entry_num = 0;
-  std::vector<TypedValue> key;
-  const uint8_t *value_ptr;
-  while (getNextEntryCompositeKey(&key, &value_ptr, &entry_num)) {
-    ++entries_visited;
-    (*functor)(key, *value_ptr);
-    key.clear();
-  }
-  return entries_visited;
-}
-
-template <bool resizable,
-          bool serializable,
-          bool force_key_copy,
-          bool allow_duplicate_keys>
-template <typename FunctorT>
-std::size_t FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
     ::forEachCompositeKeyFast(FunctorT *functor) const {
   std::size_t entries_visited = 0;
   std::size_t entry_num = 0;
@@ -2553,7 +2309,8 @@ template <bool resizable,
           bool allow_duplicate_keys>
 template <typename FunctorT>
 std::size_t FastHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
-    ::forEachCompositeKeyFast(FunctorT *functor, int index) const {
+    ::forEachCompositeKeyFast(FunctorT *functor,
+                              int index) const {
   std::size_t entries_visited = 0;
   std::size_t entry_num = 0;
   std::vector<TypedValue> key;


[3/4] incubator-quickstep git commit: Removed some dead code and made minor updates.

Posted by ra...@apache.org.
http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/5a80e33e/storage/FastSeparateChainingHashTable.hpp
----------------------------------------------------------------------
diff --git a/storage/FastSeparateChainingHashTable.hpp b/storage/FastSeparateChainingHashTable.hpp
index 49cea5b..0670993 100644
--- a/storage/FastSeparateChainingHashTable.hpp
+++ b/storage/FastSeparateChainingHashTable.hpp
@@ -120,25 +120,15 @@ class FastSeparateChainingHashTable : public FastHashTable<resizable,
                                  const std::size_t variable_key_size,
                                  const uint8_t &value,
                                  HashTablePreallocationState *prealloc_state) override;
-  HashTablePutResult putCompositeKeyInternal(const std::vector<TypedValue> &key,
-                                             const std::size_t variable_key_size,
-                                             const uint8_t &value,
-                                             HashTablePreallocationState *prealloc_state) override;
+
   HashTablePutResult putCompositeKeyInternalFast(const std::vector<TypedValue> &key,
                                              const std::size_t variable_key_size,
                                              const std::uint8_t *init_value_ptr,
                                              HashTablePreallocationState *prealloc_state) override;
 
-  uint8_t* upsertInternal(const TypedValue &key,
-                         const std::size_t variable_key_size,
-                         const uint8_t &initial_value) override;
   uint8_t* upsertInternalFast(const TypedValue &key,
-                         const std::uint8_t *init_value_ptr,
-                         const std::size_t variable_key_size) override;
-
-  uint8_t* upsertCompositeKeyInternal(const std::vector<TypedValue> &key,
-                                     const std::size_t variable_key_size,
-                                     const uint8_t &initial_value) override;
+                         const std::size_t variable_key_size,
+                         const std::uint8_t *init_value_ptr) override;
 
   uint8_t* upsertCompositeKeyInternalFast(const std::vector<TypedValue> &key,
                                      const std::uint8_t *init_value_ptr,
@@ -788,76 +778,6 @@ template <bool resizable,
           bool allow_duplicate_keys>
 HashTablePutResult
     FastSeparateChainingHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
-        ::putCompositeKeyInternal(const std::vector<TypedValue> &key,
-                                  const std::size_t variable_key_size,
-                                  const uint8_t &value,
-                                  HashTablePreallocationState *prealloc_state) {
-  DEBUG_ASSERT(this->key_types_.size() == key.size());
-
-  if (prealloc_state == nullptr) {
-    // Early check for a free bucket.
-    if (header_->buckets_allocated.load(std::memory_order_relaxed) >= header_->num_buckets) {
-      return HashTablePutResult::kOutOfSpace;
-    }
-
-    // TODO(chasseur): If allow_duplicate_keys is true, avoid storing more than
-    // one copy of the same variable-length key.
-    if (!key_manager_.allocateVariableLengthKeyStorage(variable_key_size)) {
-      // Ran out of variable-length key storage space.
-      return HashTablePutResult::kOutOfSpace;
-    }
-  }
-
-  const std::size_t hash_code = this->hashCompositeKey(key);
-  void *bucket = nullptr;
-  std::atomic<std::size_t> *pending_chain_ptr;
-  std::size_t pending_chain_ptr_finish_value;
-  for (;;) {
-    if (locateBucketForInsertion(hash_code,
-                                 0,
-                                 &bucket,
-                                 &pending_chain_ptr,
-                                 &pending_chain_ptr_finish_value,
-                                 prealloc_state)) {
-      // Found an empty bucket.
-      break;
-    } else if (bucket == nullptr) {
-      // Ran out of buckets. Deallocate any variable space that we were unable
-      // to use.
-      DEBUG_ASSERT(prealloc_state == nullptr);
-      key_manager_.deallocateVariableLengthKeyStorage(variable_key_size);
-      return HashTablePutResult::kOutOfSpace;
-    } else {
-      // Hash collision found, and duplicates aren't allowed.
-      DEBUG_ASSERT(!allow_duplicate_keys);
-      DEBUG_ASSERT(prealloc_state == nullptr);
-      if (key_manager_.compositeKeyCollisionCheck(key, bucket)) {
-        // Duplicate key. Deallocate any variable storage space and return.
-        key_manager_.deallocateVariableLengthKeyStorage(variable_key_size);
-        return HashTablePutResult::kDuplicateKey;
-      }
-    }
-  }
-
-  // Write the key and hash.
-  writeCompositeKeyToBucket(key, hash_code, bucket, prealloc_state);
-
-  // Store the value by using placement new with ValueT's copy constructor.
-  new(static_cast<char*>(bucket) + kValueOffset) uint8_t(value);
-
-  // Update the previous chain pointer to point to the new bucket.
-  pending_chain_ptr->store(pending_chain_ptr_finish_value, std::memory_order_release);
-
-  // We're all done.
-  return HashTablePutResult::kOK;
-}
-
-template <bool resizable,
-          bool serializable,
-          bool force_key_copy,
-          bool allow_duplicate_keys>
-HashTablePutResult
-    FastSeparateChainingHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
         ::putCompositeKeyInternalFast(const std::vector<TypedValue> &key,
                                   const std::size_t variable_key_size,
                                   const uint8_t *init_value_ptr,
@@ -923,76 +843,14 @@ HashTablePutResult
   return HashTablePutResult::kOK;
 }
 
-
-template <bool resizable,
-          bool serializable,
-          bool force_key_copy,
-          bool allow_duplicate_keys>
-uint8_t* FastSeparateChainingHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
-    ::upsertInternal(const TypedValue &key,
-                     const std::size_t variable_key_size,
-                     const uint8_t &initial_value) {
-  DEBUG_ASSERT(!allow_duplicate_keys);
-  DEBUG_ASSERT(this->key_types_.size() == 1);
-  DEBUG_ASSERT(key.isPlausibleInstanceOf(this->key_types_.front()->getSignature()));
-
-  if (variable_key_size > 0) {
-    // Don't allocate yet, since the key may already be present. However, we
-    // do check if either the allocated variable storage space OR the free
-    // space is big enough to hold the key (at least one must be true: either
-    // the key is already present and allocated, or we need to be able to
-    // allocate enough space for it).
-    std::size_t allocated_bytes = header_->variable_length_bytes_allocated.load(std::memory_order_relaxed);
-    if ((allocated_bytes < variable_key_size)
-        && (allocated_bytes + variable_key_size > key_manager_.getVariableLengthKeyStorageSize())) {
-      return nullptr;
-    }
-  }
-
-  const std::size_t hash_code = key.getHash();
-  void *bucket = nullptr;
-  std::atomic<std::size_t> *pending_chain_ptr;
-  std::size_t pending_chain_ptr_finish_value;
-  for (;;) {
-    if (locateBucketForInsertion(hash_code,
-                                 variable_key_size,
-                                 &bucket,
-                                 &pending_chain_ptr,
-                                 &pending_chain_ptr_finish_value,
-                                 nullptr)) {
-      // Found an empty bucket.
-      break;
-    } else if (bucket == nullptr) {
-      // Ran out of buckets or variable-key space.
-      return nullptr;
-    } else if (key_manager_.scalarKeyCollisionCheck(key, bucket)) {
-      // Found an already-existing entry for this key.
-      return reinterpret_cast<uint8_t*>(static_cast<char*>(bucket) + kValueOffset);
-    }
-  }
-
-  // We are now writing to an empty bucket.
-  // Write the key and hash.
-  writeScalarKeyToBucket(key, hash_code, bucket, nullptr);
-
-  // Copy the supplied 'initial_value' into place.
-  uint8_t *value = new(static_cast<char*>(bucket) + kValueOffset) uint8_t(initial_value);
-
-  // Update the previous chain pointer to point to the new bucket.
-  pending_chain_ptr->store(pending_chain_ptr_finish_value, std::memory_order_release);
-
-  // Return the value.
-  return value;
-}
-
 template <bool resizable,
           bool serializable,
           bool force_key_copy,
           bool allow_duplicate_keys>
 uint8_t* FastSeparateChainingHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
     ::upsertInternalFast(const TypedValue &key,
-                     const std::uint8_t *init_value_ptr,
-                     const std::size_t variable_key_size) {
+                     const std::size_t variable_key_size,
+                     const std::uint8_t *init_value_ptr) {
   DEBUG_ASSERT(!allow_duplicate_keys);
   DEBUG_ASSERT(this->key_types_.size() == 1);
   DEBUG_ASSERT(key.isPlausibleInstanceOf(this->key_types_.front()->getSignature()));
@@ -1052,67 +910,6 @@ uint8_t* FastSeparateChainingHashTable<resizable, serializable, force_key_copy,
   return value;
 }
 
-
-template <bool resizable,
-          bool serializable,
-          bool force_key_copy,
-          bool allow_duplicate_keys>
-uint8_t* FastSeparateChainingHashTable<resizable, serializable, force_key_copy, allow_duplicate_keys>
-    ::upsertCompositeKeyInternal(const std::vector<TypedValue> &key,
-                                 const std::size_t variable_key_size,
-                                 const uint8_t &initial_value) {
-  DEBUG_ASSERT(!allow_duplicate_keys);
-  DEBUG_ASSERT(this->key_types_.size() == key.size());
-
-  if (variable_key_size > 0) {
-    // Don't allocate yet, since the key may already be present. However, we
-    // do check if either the allocated variable storage space OR the free
-    // space is big enough to hold the key (at least one must be true: either
-    // the key is already present and allocated, or we need to be able to
-    // allocate enough space for it).
-    std::size_t allocated_bytes = header_->variable_length_bytes_allocated.load(std::memory_order_relaxed);
-    if ((allocated_bytes < variable_key_size)
-        && (allocated_bytes + variable_key_size > key_manager_.getVariableLengthKeyStorageSize())) {
-      return nullptr;
-    }
-  }
-
-  const std::size_t hash_code = this->hashCompositeKey(key);
-  void *bucket = nullptr;
-  std::atomic<std::size_t> *pending_chain_ptr;
-  std::size_t pending_chain_ptr_finish_value;
-  for (;;) {
-    if (locateBucketForInsertion(hash_code,
-                                 variable_key_size,
-                                 &bucket,
-                                 &pending_chain_ptr,
-                                 &pending_chain_ptr_finish_value,
-                                 nullptr)) {
-      // Found an empty bucket.
-      break;
-    } else if (bucket == nullptr) {
-      // Ran out of buckets or variable-key space.
-      return nullptr;
-    } else if (key_manager_.compositeKeyCollisionCheck(key, bucket)) {
-      // Found an already-existing entry for this key.
-      return reinterpret_cast<uint8_t*>(static_cast<char*>(bucket) + kValueOffset);
-    }
-  }
-
-  // We are now writing to an empty bucket.
-  // Write the key and hash.
-  writeCompositeKeyToBucket(key, hash_code, bucket, nullptr);
-
-  // Copy the supplied 'initial_value' into place.
-  uint8_t *value = new(static_cast<char*>(bucket) + kValueOffset) uint8_t(initial_value);
-
-  // Update the previous chaing pointer to point to the new bucket.
-  pending_chain_ptr->store(pending_chain_ptr_finish_value, std::memory_order_release);
-
-  // Return the value.
-  return value;
-}
-
 template <bool resizable,
           bool serializable,
           bool force_key_copy,