vmware-archive / quickstep

Quickstep Project
Apache License 2.0
27 stars 13 forks source link

Resolves SMA bugs revealed by TPCH. #234

Closed cramja closed 8 years ago

cramja commented 8 years ago

Running the TPCH queries revealed some bugs in SMA. Bugfix 1 This resolves the issue where predicates which are not of the form 'Attribute comparison literal' (Ex. lo_orderkey > 100000) cause a GLOG(FATAL) when really, they can just be ignored by the SMA.

The commit eb6fecd resolves this.

Bugfix 2 There is another issue with SMA to do with conversion of TypedValues. For example on a simplified version of TPCH query 6:

SELECT sum(l_extendedprice * l_discount) AS revenue 
FROM lineitem
WHERE l_quantity < 24;

The literal value 24 is integer type compared to the double type which l_quantity is stored as.

The commit 824efaa fixes this.

d commented 8 years ago

Instead of aborting the process we now return a Selectivity::kUnknown. Was the code under test? Are we seeing a missing test for the new behavior now?

cramja commented 8 years ago

@d The previous code was not under test in as far as the unit test does not cover the method SMAIndexSubBlock::getSelectivityForPredicate() though the helper methods inside of it were tested.

This PR will include a test for getSelectivityForPredicate once finished, as the type-coercion commit will need a unit test to cover it and in that same unit test I'll add tests for the kUnknown case.

d commented 8 years ago

ah thx for the clarification marc

zuyu commented 8 years ago

@cramja I was wondering if we could upcast int to double when doing such comparisons. I believe we support so in TypedValue comparisons.

pateljm commented 8 years ago

LGTM. Thanks @marc! Merging.