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/19e7c94b Tree: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/tree/19e7c94b Diff: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/diff/19e7c94b Branch: refs/heads/fix-derived-accessor Commit: 19e7c94be4b2576955b672ab46da3947fb78b433 Parents: 05b47b5 Author: Jianqiao Zhu <jianq...@cs.wisc.edu> Authored: Sat Mar 25 18:54:59 2017 -0500 Committer: Jianqiao Zhu <jianq...@cs.wisc.edu> Committed: Mon Mar 27 15:26:07 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/19e7c94b/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/19e7c94b/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/19e7c94b/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());
