You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@quickstep.apache.org by hb...@apache.org on 2016/06/09 01:10:02 UTC
[33/48] incubator-quickstep git commit: Resolves SMA bugs revealed by
TPCH. (#234)
Resolves SMA bugs revealed by TPCH. (#234)
Project: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/commit/882cd386
Tree: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/tree/882cd386
Diff: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/diff/882cd386
Branch: refs/heads/query-manager-used-in-foreman
Commit: 882cd386ab184c69508a4d4ff71a64cca0053969
Parents: 0115e96
Author: Marc S <cr...@users.noreply.github.com>
Authored: Mon May 23 11:11:44 2016 -0700
Committer: Zuyu Zhang <zz...@pivotal.io>
Committed: Wed Jun 8 11:57:45 2016 -0700
----------------------------------------------------------------------
storage/CMakeLists.txt | 1 +
storage/SMAIndexSubBlock.cpp | 55 +++++++++++++++++++-----
storage/SMAIndexSubBlock.hpp | 3 +-
storage/tests/SMAIndexSubBlock_unittest.cpp | 52 ++++++++++++++++++++++
4 files changed, 99 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/882cd386/storage/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/storage/CMakeLists.txt b/storage/CMakeLists.txt
index 26a3e32..a3093df 100644
--- a/storage/CMakeLists.txt
+++ b/storage/CMakeLists.txt
@@ -1451,6 +1451,7 @@ target_link_libraries(SMAIndexSubBlock_unittest
quickstep_types_CharType
quickstep_types_DoubleType
quickstep_types_FloatType
+ quickstep_types_IntType
quickstep_types_LongType
quickstep_types_TypeFactory
quickstep_types_TypeID
http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/882cd386/storage/SMAIndexSubBlock.cpp
----------------------------------------------------------------------
diff --git a/storage/SMAIndexSubBlock.cpp b/storage/SMAIndexSubBlock.cpp
index aa9bc54..3a1ccc2 100644
--- a/storage/SMAIndexSubBlock.cpp
+++ b/storage/SMAIndexSubBlock.cpp
@@ -181,19 +181,22 @@ Selectivity getSelectivity(const TypedValue &literal,
}
SMAPredicate* SMAPredicate::ExtractSMAPredicate(const ComparisonPredicate &predicate) {
- if (predicate.getLeftOperand().hasStaticValue()) {
- DLOG_IF(FATAL, predicate.getRightOperand().getDataSource() != Scalar::kAttribute);
+ // Check that the predicate is contains an attribute and a literal value.
+ if (predicate.getLeftOperand().hasStaticValue() &&
+ predicate.getRightOperand().getDataSource() == Scalar::kAttribute) {
return new SMAPredicate(
static_cast<const ScalarAttribute&>(predicate.getRightOperand()).getAttribute().getID(),
flipComparisonID(predicate.getComparison().getComparisonID()),
predicate.getLeftOperand().getStaticValue().makeReferenceToThis());
- } else {
- DLOG_IF(FATAL, predicate.getLeftOperand().getDataSource() != Scalar::kAttribute);
+ } else if (predicate.getRightOperand().hasStaticValue() &&
+ predicate.getLeftOperand().getDataSource() == Scalar::kAttribute) {
return new SMAPredicate(
static_cast<const ScalarAttribute&>(predicate.getLeftOperand()).getAttribute().getID(),
predicate.getComparison().getComparisonID(),
predicate.getRightOperand().getStaticValue().makeReferenceToThis());
}
+ // Predicate is improper form, so return nullptr.
+ return nullptr;
}
/**
@@ -603,12 +606,47 @@ Selectivity SMAIndexSubBlock::getSelectivityForPredicate(const ComparisonPredica
}
std::unique_ptr<SMAPredicate> sma_predicate(SMAPredicate::ExtractSMAPredicate(predicate));
+ // The predicate did not contain a static value to compare against.
+ if (!sma_predicate) {
+ return Selectivity::kUnknown;
+ }
+
const SMAEntry *entry = getEntryChecked(sma_predicate->attribute);
// The attribute wasn't indexed.
if (entry == nullptr || !entry->min_entry_ref.valid || !entry->max_entry_ref.valid) {
return Selectivity::kUnknown;
}
+
+ // Check that the type of the comparison is the same or can be coerced the
+ // type of the underlying attribute.
+ if (sma_predicate->literal.getTypeID() != entry->type_id) {
+ // Try to coerce the literal type to the entry type.
+ // It may have to account for variable length attributes.
+ int literal_length = -1;
+ if (TypeFactory::TypeRequiresLengthParameter(sma_predicate->literal.getTypeID())) {
+ literal_length = sma_predicate->literal.getAsciiStringLength();
+ }
+
+ const Type &literal_type = literal_length == -1 ?
+ TypeFactory::GetType(sma_predicate->literal.getTypeID(), false) :
+ TypeFactory::GetType(sma_predicate->literal.getTypeID(), static_cast<std::size_t>(literal_length), false);
+ const Type &attribute_type = literal_length == -1 ?
+ TypeFactory::GetType(entry->type_id, false) :
+ TypeFactory::GetType(entry->type_id, static_cast<std::size_t>(literal_length), false);
+ if (attribute_type.isSafelyCoercibleFrom(literal_type)) {
+ // Convert the literal's type inside the predicate.
+ SMAPredicate *replacement = new SMAPredicate(
+ sma_predicate->attribute,
+ sma_predicate->comparison,
+ attribute_type.coerceValue(sma_predicate->literal, literal_type));
+ sma_predicate.reset(replacement);
+ } else {
+ // The literal type cannot be converted, so do not evaluate with the SMA.
+ return Selectivity::kUnknown;
+ }
+ }
+
return sma_internal::getSelectivity(
sma_predicate->literal,
sma_predicate->comparison,
@@ -623,12 +661,9 @@ predicate_cost_t SMAIndexSubBlock::estimatePredicateEvaluationCost(
DCHECK(initialized_);
// Check that at least one of the operands has a static value.
- if (predicate.getLeftOperand().hasStaticValue() ||
- predicate.getRightOperand().hasStaticValue()) {
- Selectivity selectivity = getSelectivityForPredicate(predicate);
- if (selectivity == Selectivity::kAll || selectivity == Selectivity::kNone) {
- return predicate_cost::kConstantTime;
- }
+ Selectivity selectivity = getSelectivityForPredicate(predicate);
+ if (selectivity == Selectivity::kAll || selectivity == Selectivity::kNone) {
+ return predicate_cost::kConstantTime;
}
return predicate_cost::kInfinite;
}
http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/882cd386/storage/SMAIndexSubBlock.hpp
----------------------------------------------------------------------
diff --git a/storage/SMAIndexSubBlock.hpp b/storage/SMAIndexSubBlock.hpp
index 71ca75b..895add9 100644
--- a/storage/SMAIndexSubBlock.hpp
+++ b/storage/SMAIndexSubBlock.hpp
@@ -120,7 +120,6 @@ struct SMAPredicate {
// that the predicate had not been used previously.
Selectivity selectivity;
- private:
SMAPredicate(const attribute_id attr,
const ComparisonID comp,
const TypedValue lit)
@@ -376,7 +375,7 @@ class SMAIndexSubBlock : public IndexSubBlock {
private:
bool requiresRebuild() const;
- inline sma_internal::Selectivity getSelectivityForPredicate(const ComparisonPredicate &predicate) const;
+ sma_internal::Selectivity getSelectivityForPredicate(const ComparisonPredicate &predicate) const;
// Retrieves an entry, first checking if the given attribute is indexed.
inline const sma_internal::SMAEntry* getEntryChecked(attribute_id attribute) const {
http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/882cd386/storage/tests/SMAIndexSubBlock_unittest.cpp
----------------------------------------------------------------------
diff --git a/storage/tests/SMAIndexSubBlock_unittest.cpp b/storage/tests/SMAIndexSubBlock_unittest.cpp
index b709712..ad842c2 100644
--- a/storage/tests/SMAIndexSubBlock_unittest.cpp
+++ b/storage/tests/SMAIndexSubBlock_unittest.cpp
@@ -41,6 +41,7 @@
#include "types/CharType.hpp"
#include "types/DoubleType.hpp"
#include "types/FloatType.hpp"
+#include "types/IntType.hpp"
#include "types/LongType.hpp"
#include "types/TypeFactory.hpp"
#include "types/TypeID.hpp"
@@ -301,6 +302,10 @@ class SMAIndexSubBlockTest : public ::testing::Test {
return !index_->header_->index_consistent;
}
+ Selectivity getSelectivityForPredicate(const ComparisonPredicate &predicate) const {
+ return index_->getSelectivityForPredicate(predicate);
+ }
+
std::unique_ptr<CatalogRelation> relation_;
std::unique_ptr<MockTupleStorageSubBlock> tuple_store_;
ScopedBuffer index_memory_;
@@ -359,6 +364,53 @@ TEST_F(SMAIndexSubBlockTest, TestConstructor) {
EXPECT_FALSE(requiresRebuild());
}
+TEST_F(SMAIndexSubBlockTest, TestGetSelectivityForPredicate) {
+ // Index long, float, bigchar, and varchar type.
+ createIndex({0, 2, 5, 6}, kIndexSubBlockSize);
+ int min = 0, max = 9010, step = 10;
+ for (std::size_t i = min; i <= static_cast<std::size_t>(max); i += step) {
+ generateAndInsertTuple(i, false, "suffix");
+ }
+ index_->rebuild();
+
+ // Tuple storage block now contains long values [0, 9010], and float values
+ // [0, 2252.5].
+ // Test several value types.
+ // Test with inline values, returning both possible value types.
+ std::unique_ptr<ComparisonPredicate> predicate(
+ generateNumericComparisonPredicate<LongType>(ComparisonID::kLess, 0, min));
+ Selectivity cost = getSelectivityForPredicate(*predicate);
+ EXPECT_EQ(Selectivity::kNone, cost);
+
+ // Test on the float attribute with a type that will need coercion. In this
+ // case, a long type should not coerce to an integer type.
+ predicate.reset(generateNumericComparisonPredicate<LongType>(ComparisonID::kLess, 2, min));
+ cost = getSelectivityForPredicate(*predicate);
+ EXPECT_EQ(Selectivity::kUnknown, cost);
+
+ predicate.reset(generateNumericComparisonPredicate<IntType>(ComparisonID::kLess, 2, min));
+ cost = getSelectivityForPredicate(*predicate);
+ EXPECT_EQ(Selectivity::kNone, cost);
+
+ // This should be unknown because of the loss of precision on coercion.
+ predicate.reset(generateNumericComparisonPredicate<FloatType>(ComparisonID::kLess, 0, min + 0.5));
+ cost = getSelectivityForPredicate(*predicate);
+ EXPECT_EQ(Selectivity::kUnknown, cost);
+
+ // Test coercion with String types as well.
+ predicate.reset(generateStringComparisonPredicate(ComparisonID::kLess, 6, ""));
+ cost = getSelectivityForPredicate(*predicate);
+ EXPECT_EQ(Selectivity::kNone, cost);
+
+ predicate.reset(generateStringComparisonPredicate(ComparisonID::kLess, 5, ""));
+ cost = getSelectivityForPredicate(*predicate);
+ EXPECT_EQ(Selectivity::kNone, cost);
+
+ predicate.reset(generateStringComparisonPredicate(ComparisonID::kLess, 5, "999999999999999"));
+ cost = getSelectivityForPredicate(*predicate);
+ EXPECT_EQ(Selectivity::kAll, cost);
+}
+
TEST_F(SMAIndexSubBlockTest, TestRebuild) {
// This test is checking to see if the min/max values are set correctly on
// a rebuild of the index.