quickstep-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jianq...@apache.org
Subject incubator-quickstep git commit: Fix the bug when derived_accessor is TupleIdSequenceAdapterValueAccessor<ColumnVectorsValueAccesor> in PackedPayloadHashTable
Date Sat, 25 Mar 2017 23:55:31 GMT
Repository: incubator-quickstep
Updated Branches:
  refs/heads/fix-derived-accessor [created] a100b9ecc


Fix the bug when derived_accessor is TupleIdSequenceAdapterValueAccessor<ColumnVectorsValueAccesor>
in PackedPayloadHashTable


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

Branch: refs/heads/fix-derived-accessor
Commit: a100b9ecce61773e5dec5a824c505ea1999f1c74
Parents: 42bf626
Author: Jianqiao Zhu <jianqiao@cs.wisc.edu>
Authored: Sat Mar 25 18:54:59 2017 -0500
Committer: Jianqiao Zhu <jianqiao@cs.wisc.edu>
Committed: Sat Mar 25 18:54:59 2017 -0500

----------------------------------------------------------------------
 storage/AggregationOperationState.cpp |  2 +-
 storage/PackedPayloadHashTable.cpp    | 43 ++++++++++++++++++++++--------
 storage/PackedPayloadHashTable.hpp    | 17 +++++++-----
 3 files changed, 43 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/a100b9ec/storage/AggregationOperationState.cpp
----------------------------------------------------------------------
diff --git a/storage/AggregationOperationState.cpp b/storage/AggregationOperationState.cpp
index eef2c9d..aea5bf5 100644
--- a/storage/AggregationOperationState.cpp
+++ b/storage/AggregationOperationState.cpp
@@ -419,7 +419,7 @@ bool AggregationOperationState::checkAggregatePartitioned(
 
   // There are GROUP BYs without DISTINCT. Check if the estimated number of
   // groups is large enough to warrant a partitioned aggregation.
-  return estimated_num_groups >
+  return estimated_num_groups >=
          static_cast<std::size_t>(
              FLAGS_partition_aggregation_num_groups_threshold);
 }

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/a100b9ec/storage/PackedPayloadHashTable.cpp
----------------------------------------------------------------------
diff --git a/storage/PackedPayloadHashTable.cpp b/storage/PackedPayloadHashTable.cpp
index 8c4a9fc..d702396 100644
--- a/storage/PackedPayloadHashTable.cpp
+++ b/storage/PackedPayloadHashTable.cpp
@@ -239,26 +239,47 @@ bool PackedPayloadHashTable::upsertValueAccessorCompositeKey(
 
   base_accessor->beginIterationVirtual();
   if (has_derived_accessor) {
+    // NOTE(jianqiao): Currently we expect derived_accessor to be either
+    // ColumnVectorsValueAccesor or
+    // TupleIdSequenceAdapterValueAccessor<ColumnVectorsValueAccesor>
     DCHECK(derived_accessor->getImplementationType()
                == ValueAccessor::Implementation::kColumnVectors);
+    DCHECK(!derived_accessor->isOrderedTupleIdSequenceAdapter());
     derived_accessor->beginIterationVirtual();
   }
 
   return InvokeOnBools(
-      has_derived_accessor,
       handles_.empty(),
       !all_keys_inline_,
-      [&](auto use_two_accessors,  // NOLINT(build/c++11)
-          auto key_only,
+      [&](auto key_only,  // NOLINT(build/c++11)
           auto has_variable_size) -> bool {
-    return this->upsertValueAccessorCompositeKeyInternal<
-        decltype(use_two_accessors)::value,
-        decltype(key_only)::value,
-        decltype(has_variable_size)::value>(
-            argument_ids,
-            key_attr_ids,
-            base_accessor,
-            static_cast<ColumnVectorsValueAccessor *>(derived_accessor));
+    constexpr bool key_only_v = decltype(key_only)::value;
+    constexpr bool has_variable_size_v = decltype(has_variable_size)::value;
+
+    if (!has_derived_accessor) {
+      return this->upsertValueAccessorCompositeKeyInternal<
+          false, key_only_v, has_variable_size_v>(
+              argument_ids,
+              key_attr_ids,
+              base_accessor,
+              static_cast<ColumnVectorsValueAccessor*>(nullptr));
+    } else if (derived_accessor->isTupleIdSequenceAdapter()) {
+      return this->upsertValueAccessorCompositeKeyInternal<
+          true, key_only_v, has_variable_size_v>(
+              argument_ids,
+              key_attr_ids,
+              base_accessor,
+              static_cast<
+                  TupleIdSequenceAdapterValueAccessor<
+                      ColumnVectorsValueAccessor>*>(derived_accessor));
+    } else {
+      return this->upsertValueAccessorCompositeKeyInternal<
+          true, key_only_v, has_variable_size_v>(
+              argument_ids,
+              key_attr_ids,
+              base_accessor,
+              static_cast<ColumnVectorsValueAccessor*>(derived_accessor));
+    }
   });
 }
 

http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/a100b9ec/storage/PackedPayloadHashTable.hpp
----------------------------------------------------------------------
diff --git a/storage/PackedPayloadHashTable.hpp b/storage/PackedPayloadHashTable.hpp
index c49bdb4..b06200d 100644
--- a/storage/PackedPayloadHashTable.hpp
+++ b/storage/PackedPayloadHashTable.hpp
@@ -343,12 +343,13 @@ class PackedPayloadHashTable : public AggregationStateHashTableBase
{
       const std::vector<TypedValue> &key,
       const std::size_t variable_key_size);
 
-  template <bool use_two_accessors, bool key_only, bool has_variable_size>
+  template <bool use_two_accessors, bool key_only, bool has_variable_size,
+            typename ValueAccessorT>
   inline bool upsertValueAccessorCompositeKeyInternal(
       const std::vector<std::vector<MultiSourceAttributeId>> &argument_ids,
       const std::vector<MultiSourceAttributeId> &key_ids,
       ValueAccessor *base_accessor,
-      ColumnVectorsValueAccessor *derived_accessor);
+      ValueAccessorT *derived_accessor);
 
   // Generate a hash for a composite key by hashing each component of 'key' and
   // mixing their bits with CombineHashes().
@@ -377,11 +378,12 @@ class PackedPayloadHashTable : public AggregationStateHashTableBase
{
   // and returns true if any of the values is null, otherwise returns false.
   template <bool use_two_accessors,
             bool check_for_null_keys,
-            typename ValueAccessorT>
+            typename BaseAccessorT,
+            typename DerivedAccessorT>
   inline static bool GetCompositeKeyFromValueAccessor(
       const std::vector<MultiSourceAttributeId> &key_ids,
-      const ValueAccessorT *accessor,
-      const ColumnVectorsValueAccessor *derived_accessor,
+      const BaseAccessorT *accessor,
+      const DerivedAccessorT *derived_accessor,
       std::vector<TypedValue> *key_vector) {
     for (std::size_t key_idx = 0; key_idx < key_ids.size(); ++key_idx) {
       const MultiSourceAttributeId &key_id = key_ids[key_idx];
@@ -825,12 +827,13 @@ inline std::uint8_t* PackedPayloadHashTable::upsertCompositeKeyInternal(
   return value;
 }
 
-template <bool use_two_accessors, bool key_only, bool has_variable_size>
+template <bool use_two_accessors, bool key_only, bool has_variable_size,
+          typename ValueAccessorT>
 inline bool PackedPayloadHashTable::upsertValueAccessorCompositeKeyInternal(
     const std::vector<std::vector<MultiSourceAttributeId>> &argument_ids,
     const std::vector<MultiSourceAttributeId> &key_ids,
     ValueAccessor *base_accessor,
-    ColumnVectorsValueAccessor *derived_accessor) {
+    ValueAccessorT *derived_accessor) {
   std::size_t variable_size = 0;
   std::vector<TypedValue> key_vector;
   key_vector.resize(key_ids.size());


Mime
View raw message