You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2016/11/30 02:48:01 UTC
[kudu-CR] [c++] ColumnPredicate simplification for inequalities on boundary values
Hello Adar Dembo, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5273
to review the following change.
Change subject: [c++] ColumnPredicate simplification for inequalities on boundary values
......................................................................
[c++] ColumnPredicate simplification for inequalities on boundary values
Adds special casing for inequality predicates on minimum and maximum
values. This tightens partition pruning in boundary cases. New tests are
added to cover the specific transormations. We already have good test
coverage of boundary values in predicate-test.
These changes uncovered a bug in type traits where the minimum value for
float types was defined as the lowest discrete value instead of
-infinity, after adding the predicate simplification mutliple test cases
in predicate-test failed.
Change-Id: I9bb150249a51a69fb85af67d34e85b7d7bed325c
---
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/types.cc
M src/kudu/common/types.h
5 files changed, 184 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/5273/1
--
To view, visit http://gerrit.cloudera.org:8080/5273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9bb150249a51a69fb85af67d34e85b7d7bed325c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [c++] ColumnPredicate simplification for inequalities on boundary values
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: [c++] ColumnPredicate simplification for inequalities on boundary values
......................................................................
[c++] ColumnPredicate simplification for inequalities on boundary values
Adds special casing for inequality predicates on minimum and maximum
values. This tightens partition pruning in boundary cases. New tests are
added to cover the specific transformations. We already have good test
coverage of boundary values in predicate-test.
These changes uncovered a bug in type traits where the minimum value for
float types was defined as the lowest discrete value instead of
-infinity, after adding the predicate simplification multiple test cases
in predicate-test failed.
Change-Id: I9bb150249a51a69fb85af67d34e85b7d7bed325c
Reviewed-on: http://gerrit.cloudera.org:8080/5273
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/types.cc
M src/kudu/common/types.h
5 files changed, 184 insertions(+), 12 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/5273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9bb150249a51a69fb85af67d34e85b7d7bed325c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [c++] ColumnPredicate simplification for inequalities on boundary values
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [c++] ColumnPredicate simplification for inequalities on boundary values
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5273/1/src/kudu/common/column_predicate.cc
File src/kudu/common/column_predicate.cc:
Line 163: DCHECK(lower_ != nullptr || upper_ != nullptr);
> So it's just not possible to construct a boundless range predicate?
Correct, a 'boundless' range predicate would just be an IS NOT NULL predicate.
--
To view, visit http://gerrit.cloudera.org:8080/5273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9bb150249a51a69fb85af67d34e85b7d7bed325c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++] ColumnPredicate simplification for inequalities on boundary values
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [c++] ColumnPredicate simplification for inequalities on boundary values
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/5273/1//COMMIT_MSG
Commit Message:
PS1, Line 11: transormations
> transformations
Done
PS1, Line 16: mutliple
> multiple
Done
--
To view, visit http://gerrit.cloudera.org:8080/5273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9bb150249a51a69fb85af67d34e85b7d7bed325c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++] ColumnPredicate simplification for inequalities on boundary values
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [c++] ColumnPredicate simplification for inequalities on boundary values
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/5273/1//COMMIT_MSG
Commit Message:
PS1, Line 11: transormations
transformations
PS1, Line 16: mutliple
multiple
http://gerrit.cloudera.org:8080/#/c/5273/1/src/kudu/common/column_predicate.cc
File src/kudu/common/column_predicate.cc:
Line 163: DCHECK(lower_ != nullptr || upper_ != nullptr);
So it's just not possible to construct a boundless range predicate?
--
To view, visit http://gerrit.cloudera.org:8080/5273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9bb150249a51a69fb85af67d34e85b7d7bed325c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] [c++] ColumnPredicate simplification for inequalities on boundary values
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: [c++] ColumnPredicate simplification for inequalities on boundary values
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9bb150249a51a69fb85af67d34e85b7d7bed325c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [c++] ColumnPredicate simplification for inequalities on boundary values
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5273
to look at the new patch set (#2).
Change subject: [c++] ColumnPredicate simplification for inequalities on boundary values
......................................................................
[c++] ColumnPredicate simplification for inequalities on boundary values
Adds special casing for inequality predicates on minimum and maximum
values. This tightens partition pruning in boundary cases. New tests are
added to cover the specific transformations. We already have good test
coverage of boundary values in predicate-test.
These changes uncovered a bug in type traits where the minimum value for
float types was defined as the lowest discrete value instead of
-infinity, after adding the predicate simplification multiple test cases
in predicate-test failed.
Change-Id: I9bb150249a51a69fb85af67d34e85b7d7bed325c
---
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/partition_pruner-test.cc
M src/kudu/common/types.cc
M src/kudu/common/types.h
5 files changed, 184 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/5273/2
--
To view, visit http://gerrit.cloudera.org:8080/5273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9bb150249a51a69fb85af67d34e85b7d7bed325c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>