You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@quickstep.apache.org by ji...@apache.org on 2017/03/28 18:31:52 UTC
incubator-quickstep git commit: Fix the bug when derived_accessor is
TupleIdSequenceAdapterValueAccessor in
PackedPayloadHashTable [Forced Update!]
Repository: incubator-quickstep
Updated Branches:
refs/heads/fix-derived-accessor 19e7c94be -> b0ad86a8d (forced update)
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/b0ad86a8
Tree: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/tree/b0ad86a8
Diff: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/diff/b0ad86a8
Branch: refs/heads/fix-derived-accessor
Commit: b0ad86a8db9a77a760b4db71973f785d36e77ee8
Parents: 05b47b5
Author: Jianqiao Zhu <ji...@cs.wisc.edu>
Authored: Sat Mar 25 18:54:59 2017 -0500
Committer: Jianqiao Zhu <ji...@cs.wisc.edu>
Committed: Tue Mar 28 13:31:35 2017 -0500
----------------------------------------------------------------------
storage/AggregationOperationState.cpp | 2 +-
storage/PackedPayloadHashTable.cpp | 49 +++++++++++++++++++++---------
storage/PackedPayloadHashTable.hpp | 17 ++++++-----
3 files changed, 46 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/b0ad86a8/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/b0ad86a8/storage/PackedPayloadHashTable.cpp
----------------------------------------------------------------------
diff --git a/storage/PackedPayloadHashTable.cpp b/storage/PackedPayloadHashTable.cpp
index 8c4a9fc..7fc4b71 100644
--- a/storage/PackedPayloadHashTable.cpp
+++ b/storage/PackedPayloadHashTable.cpp
@@ -235,30 +235,51 @@ bool PackedPayloadHashTable::upsertValueAccessorCompositeKey(
ValueAccessor *base_accessor = accessor_mux.getBaseAccessor();
ValueAccessor *derived_accessor = accessor_mux.getDerivedAccessor();
- const bool has_derived_accessor = (derived_accessor != nullptr);
-
base_accessor->beginIterationVirtual();
- if (has_derived_accessor) {
+ if (derived_accessor != nullptr) {
+ // 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 (derived_accessor == nullptr) {
+ return this->upsertValueAccessorCompositeKeyInternal<
+ false, key_only_v, has_variable_size_v>(
+ argument_ids,
+ key_attr_ids,
+ base_accessor,
+ static_cast<ColumnVectorsValueAccessor*>(nullptr));
+ }
+
+ 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/b0ad86a8/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());