You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2017/03/02 19:20:32 UTC
[kudu-CR](branch-1.2.x) KUDU-1893 Ensure evaluation of added columns
Andrew Wong has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6225
Change subject: KUDU-1893 Ensure evaluation of added columns
......................................................................
KUDU-1893 Ensure evaluation of added columns
During a normal scan, a CFileIterator sets a flag to indicate whether
the block supports decoder-level evaluation, and if, after returning,
this flag has not been set to false, the returned data will be
evaluated against the predicate.
Columns that are added after table creation and after a flush will use
DefaultColumnValueIterators to scan. These iterators were not setting
the flag, so the predicates would not be evaluated on these columns.
All rows in the existing results set would be returned, regardless of
predicates on added columns.
If a column predicate is added to an added column that should filter
out some rows, the scan will yield incorrect results. There is added
test coverage in all_types-scan-correctness-test.cc. Without the
changes in this patch, RunTests() will fail at the following points:
* Null default, Range+IsNotNull
* Null default, Range
* Non-null default, Range+IsNotNull
* Non-null default, Range with Default
* Non-null default, Range without Default
This patch addresses this by forcing predicate evaluation by the
DefaultColumnValueIterator.
Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
M src/kudu/common/partial_row.h
M src/kudu/tablet/CMakeLists.txt
A src/kudu/tablet/all_types-scan-correctness-test.cc
6 files changed, 572 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6225/1
--
To view, visit http://gerrit.cloudera.org:8080/6225
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR](branch-1.2.x) KUDU-1893 Ensure evaluation of added columns
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change.
Change subject: KUDU-1893 Ensure evaluation of added columns
......................................................................
Patch Set 1:
(7 comments)
Posted differences between this and master:kudu-1893.
http://gerrit.cloudera.org:8080/#/c/6225/1//COMMIT_MSG
Commit Message:
PS1, Line 24:
: * Null default, Range
: * Non-null default, Range+IsNotNull
: * Non-null default, Range with Default
: * Non-null default, Range without Default
Removed IsNull test cases
http://gerrit.cloudera.org:8080/#/c/6225/1/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:
PS1, Line 515: )
Removed "&& !ctx->EvaluatingIsNull()", as IsNull is not in 1.2.x
http://gerrit.cloudera.org:8080/#/c/6225/1/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:
PS1, Line 46: namespace tablet {
: template<typename KeyTypeWrapper> struct SliceTypeRowOps;
: template<typename KeyTypeWrapper> struct NumTypeRowOps;
: } // namespace tablet
This was added in master:kudu-1880, which is not being included in 1.2.x.
Line 486: template<typename KeyTypeWrapper> friend struct tablet::SliceTypeRowOps;
Same here.
http://gerrit.cloudera.org:8080/#/c/6225/1/src/kudu/tablet/CMakeLists.txt
File src/kudu/tablet/CMakeLists.txt:
Line 99: ADD_KUDU_TEST(all_types-scan-correctness-test)
This was added in master:kudu-1880, which is not being included in 1.2.x.
http://gerrit.cloudera.org:8080/#/c/6225/1/src/kudu/tablet/all_types-scan-correctness-test.cc
File src/kudu/tablet/all_types-scan-correctness-test.cc:
Line 345: int count = 0;
Removed Null default, Range+IsNull case.
Line 395: AddColumn(read_default);
Removed Non-null default, Range+IsNull case.
--
To view, visit http://gerrit.cloudera.org:8080/6225
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR](branch-1.2.x) KUDU-1893 Ensure evaluation of added columns
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has submitted this change and it was merged.
Change subject: KUDU-1893 Ensure evaluation of added columns
......................................................................
KUDU-1893 Ensure evaluation of added columns
During a normal scan, a CFileIterator sets a flag to indicate whether
the block supports decoder-level evaluation, and if, after returning,
this flag has not been set to false, the returned data will be
evaluated against the predicate.
Columns that are added after table creation and after a flush will use
DefaultColumnValueIterators to scan. These iterators were not setting
the flag, so the predicates would not be evaluated on these columns.
All rows in the existing results set would be returned, regardless of
predicates on added columns.
If a column predicate is added to an added column that should filter
out some rows, the scan will yield incorrect results. There is added
test coverage in all_types-scan-correctness-test.cc. Without the
changes in this patch, RunTests() will fail at the following points:
* Null default, Range+IsNotNull
* Null default, Range
* Non-null default, Range+IsNotNull
* Non-null default, Range with Default
* Non-null default, Range without Default
This patch addresses this by forcing predicate evaluation by the
DefaultColumnValueIterator.
Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Reviewed-on: http://gerrit.cloudera.org:8080/6225
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/common/column_predicate.cc
M src/kudu/common/column_predicate.h
M src/kudu/common/partial_row.h
M src/kudu/tablet/CMakeLists.txt
A src/kudu/tablet/all_types-scan-correctness-test.cc
6 files changed, 572 insertions(+), 1 deletion(-)
Approvals:
David Ribeiro Alves: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6225
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.2.x) KUDU-1893 Ensure evaluation of added columns
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: KUDU-1893 Ensure evaluation of added columns
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6225
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic10919962d11effbb1b66d97660acd012b6b4be9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No