Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added optimized handling of range filters on sorted data. #147

Merged
merged 4 commits into from Nov 14, 2018

Conversation

floriankramer
Copy link
Member

This pr adds special cases for range filters on sorted data to the computation of filer results. Instead of iterating over the entire input data and applying a comparator for each element, the upper and lower bound of the target range are found using std::lower_bound and std::upper_bound and the range is then copied to the result vector.

@niklas88
Copy link
Member

@floriankramer any idea why the this seems to crash in the End-to-End Tests?

@niklas88
Copy link
Member

@floriankramer while you're changing ResultTable.h please fix #148

} else {
getEngine().filter(*static_cast<vector<RT>*>(subRes->_fixedSizeData),
[lhs, rhs](const RT& e) {
return ValueReader<T>::get(e[lhs]) <
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need ValueReader here but not with the two cases above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the value reader is only used for non equality comparisons, as the assumption is, that two element are equal if their bytes are equal (the last four bytes of a float value are always zero).

data->begin(), data->end(), rhs_array,
[lhs](const RT& l, const RT& r) { return l[lhs] < r[lhs]; });
if (lower != data->end()) {
// rhs appears within the data, take all elements before and after
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we take the ones in the range not before and after

@@ -400,8 +416,9 @@ vector<RT>* Filter::computeFilterFixedValueForResultType(
data->begin(), data->end(), rhs_array,
[lhs](const RT& l, const RT& r) { return l[lhs] < r[lhs]; });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ValueReader?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prefix filter is currently only run on KB type columns, which can be interpreted directly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see

Copy link
Member

@niklas88 niklas88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM on the condition that Travis passes

@floriankramer floriankramer merged commit 16235bd into ad-freiburg:master Nov 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants