You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2020/04/14 16:40:11 UTC

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types 

Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15730


Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................

IMPALA-9469: ORC scanner vectorization for collection types

This commit only keeps the batched read path of ORC columns, i.e.
from now we always read ORC values into a scratch batch. Thanks to
this we also get codegen out of the box.

From now materialization of the table-level tuples are always driven
by the root struct reader. This will enable us to implement row
validation (against a valid write id list) much easier. It's needed
for IMPALA-9512.

I eliminated the OrcComplexColumnReader::TransferTuple() interface
and the related codes.

HdfsOrcScanner became simpler. Now it just calls
TopLevelReadValueBatch() on the root struct reader which tracks the
row index of the table-level tuples and calls ReadValueBatch on its
children accordingly. The children don't need to track the state
as they are always being told which row they need to read.

Testing:
 * ran exhaustive tests

Performance:
 * non-nested benchmark results stayed the same as expected
 * Overall 1-2% gain on TPCH Nested, scale=1
 ** In some cases scanning was ~20% more efficient

Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
---
M be/src/exec/hdfs-orc-scanner.cc
M be/src/exec/hdfs-orc-scanner.h
M be/src/exec/orc-column-readers.cc
M be/src/exec/orc-column-readers.h
4 files changed, 216 insertions(+), 246 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/15730/1
-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15730 )

Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................

IMPALA-9469: ORC scanner vectorization for collection types

This commit only keeps the batched read path of ORC columns, i.e.
from now on we always read ORC values into a scratch batch. Thanks to
this we also get codegen out of the box.

From now on materialization of the table-level tuples are always driven
by the root struct reader. This will enable us to implement row
validation (against a valid write id list) much easier. It's needed
for IMPALA-9512.

I eliminated the OrcComplexColumnReader::TransferTuple() interface
and the related codes. HdfsOrcScanner became simpler. Now it just calls
TopLevelReadValueBatch() on the root struct reader which tracks the
row index of the table-level tuples and calls ReadValueBatch on its
children accordingly. The children don't need to track the state
as they are always being told which row they need to read.

Testing:
 * ran exhaustive tests

Performance:
 * non-nested benchmark results stayed the same as expected
 * Overall 1-2% gain on TPCH Nested, scale=1
 ** In some cases scanning was ~20% more efficient

Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Reviewed-on: http://gerrit.cloudera.org:8080/15730
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exec/hdfs-orc-scanner.cc
M be/src/exec/hdfs-orc-scanner.h
M be/src/exec/orc-column-readers.cc
M be/src/exec/orc-column-readers.h
4 files changed, 225 insertions(+), 263 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15730 )

Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5674/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2020 08:55:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15730 )

Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................


Patch Set 3: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2020 13:12:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15730 )

Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5812/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Apr 2020 15:42:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/15730 )

Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................


Patch Set 1:

(11 comments)

Nice change and cleanup!
I took a look but have to admit that the part that touches batch->offsets (mostly in SetPositionSlot()) is not 100% clear for me.

http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG@10
PS1, Line 10: now
nit: now on


http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG@13
PS1, Line 13: now
same


http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG@18
PS1, Line 18: I eliminated the OrcComplexColumnReader::TransferTuple() interface
            : and the related codes.
nit: I feel that this can be merged with the next paragraph.


http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG@32
PS1, Line 32: scale=1
Is there a plan to check performance on a higher scale data, e.g. scale=25 so that we can compare these improvements to the ones we achieved for primitive types?

Giving it a second thought we tested primitives on TPCH instead of TPCH Nested, but still I'm wondering if it makes sense to see these numbers on a higher scale.


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h
File be/src/exec/orc-column-readers.h:

http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h@50
PS1, Line 50: alwas
typo


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h@95
PS1, Line 95: ReadValue
From now on ReadValue() is only called from ReadValueBatch(), right? If this is the case can you make it private?


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h@108
PS1, Line 108: lists
Not just lists but maps as well, right?


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h@487
PS1, Line 487: int NumElements() const override {
             :     if (MaterializeTuple()) return vbatch_->numElements;
             :     DCHECK_EQ(children().size(), 1);
             :     OrcColumnReader* child = children()[0];
             :     return child->NumElements();
             :   }
Both OrcListReader and OrcMapReader overrides this so actually this will be only for OrcStructReader, right? Why not move it there?


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc
File be/src/exec/orc-column-readers.cc:

http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@451
PS1, Line 451: Status OrcCollectionReader::ReadValueBatch
This looks very much the same as OrcPrimitiveColumnReader::ReadValueBatch() except L462. Do you know a way to avoid this code duplication?


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@473
PS1, Line 473: return children_[0]->NumElements();
Is it worth a DCHECK to verify that this OrcListReader has exactly one child?


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@477
PS1, Line 477:   if (slot_desc_ == nullptr) {
This check is not that clear for me. Isn't this identical to !MaterializeTuple() check?
I see other occurences below as well all for collections.



-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 15 Apr 2020 13:34:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Norbert Luksa, Gabor Kaszab, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15730

to look at the new patch set (#2).

Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................

IMPALA-9469: ORC scanner vectorization for collection types

This commit only keeps the batched read path of ORC columns, i.e.
from now on we always read ORC values into a scratch batch. Thanks to
this we also get codegen out of the box.

From now on materialization of the table-level tuples are always driven
by the root struct reader. This will enable us to implement row
validation (against a valid write id list) much easier. It's needed
for IMPALA-9512.

I eliminated the OrcComplexColumnReader::TransferTuple() interface
and the related codes. HdfsOrcScanner became simpler. Now it just calls
TopLevelReadValueBatch() on the root struct reader which tracks the
row index of the table-level tuples and calls ReadValueBatch on its
children accordingly. The children don't need to track the state
as they are always being told which row they need to read.

Testing:
 * ran exhaustive tests

Performance:
 * non-nested benchmark results stayed the same as expected
 * Overall 1-2% gain on TPCH Nested, scale=1
 ** In some cases scanning was ~20% more efficient

Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
---
M be/src/exec/hdfs-orc-scanner.cc
M be/src/exec/hdfs-orc-scanner.h
M be/src/exec/orc-column-readers.cc
M be/src/exec/orc-column-readers.h
4 files changed, 225 insertions(+), 263 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/30/15730/2
-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/15730 )

Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................


Patch Set 2:

(11 comments)

Thanks for the comments!

http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG@10
PS1, Line 10: now
> nit: now on
Done


http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG@13
PS1, Line 13: now
> same
Done


http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG@18
PS1, Line 18: I eliminated the OrcComplexColumnReader::TransferTuple() interface
            : and the related codes.
> nit: I feel that this can be merged with the next paragraph.
Done


http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG@32
PS1, Line 32: 
> Is there a plan to check performance on a higher scale data, e.g. scale=25 
Unfortunately there is no 'scale' option for loading the nested workloads. Maybe we could generate them from the non-nested scaled TPCH workloads? Anyway, I'd expect even a bigger improvement on the larger datasets thank to codegen.


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h
File be/src/exec/orc-column-readers.h:

http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h@50
PS1, Line 50: alway
> typo
Done


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h@95
PS1, Line 95: ReadValue
> From now on ReadValue() is only called from ReadValueBatch(), right? If thi
In that case we'd need more friend declarations, because both OrcListReader and OrcMapReader invoke ReadValue() on their children. I'd rather keep it here but I don't feel too strongly about it.


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h@108
PS1, Line 108: lists
> Not just lists but maps as well, right?
Done


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h@487
PS1, Line 487: /// Num of tuples inside the 'row_idx'-th row. LIST/MAP types will have 0 to N tuples.
             :   /// STRUCT type will always have one tuple.
             :   virtual int GetNumTuples(int row_idx) const = 0;
             : 
             :   /// Collection values (array items, map keys/values) are concatenated in the child's
             :   /
> Both OrcListReader and OrcMapReader overrides this so actually this will be
Done


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc
File be/src/exec/orc-column-readers.cc:

http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@451
PS1, Line 451: int OrcListReader::NumElements() const {
> This looks very much the same as OrcPrimitiveColumnReader::ReadValueBatch()
Yeah, it's actually the same function.

My original idea was to introduce a BatchedReader<T> CRTP class under OrcColumnReader. Then every class between BatchedReader<T> and the leaf classes (IntColumnReader, StructReader, ListReader) would've been templates, e.g.:

 OrcColumnReader <- BatchedReader<ListReader> <- OrcComplexColumnReader<ListReader> <- CollectionReader<ListReader> <- ListReader.

Worked pretty fine with some extra benefits until I needed a polymorphic downcast in AssembleCollection:

  auto child_reader = static_cast<OrcComplexColumnReader*>(
      complex_col_reader.children()[0]);

That being said I was able to save something from the original idea. I still have OrcBatchedReader<T>, primitive types still have all the benefits of static polymorphism. However, I couldn't get rid of virtual function calls for complex types since I couldn't make OrcComplexTypes template. But at least it didn't get worse than the current version.


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@473
PS1, Line 473: 
> Is it worth a DCHECK to verify that this OrcListReader has exactly one chil
If the list is an array of structs, then we can have many children, but in that case NumElements() must be the same for all the children because they write the same tuples.


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@477
PS1, Line 477:   return Status::OK();
> This check is not that clear for me. Isn't this identical to !MaterializeTu
Yeah, it's confusing. slot_desc_ being null is independent of MaterializeTuple(). If MaterializeTuple() is true, slot_desc_ can be either nullptr or have some value.

What we check here is whether this reader writes directly to a slot of the given tuple or just delegates writing to its children.

I introduced a new DirectReader() member to make the code a bit more readable.



-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 16 Apr 2020 15:01:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/15730 )

Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................


Patch Set 2: Code-Review+2

(5 comments)

http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG@32
PS1, Line 32: 
> Unfortunately there is no 'scale' option for loading the nested workloads. 
No need to put more efforts into this if there is no convenient way to produce scaled nested workloads. Thx for the explanation!


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h
File be/src/exec/orc-column-readers.h:

http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h@95
PS1, Line 95: ReadValue
> In that case we'd need more friend declarations, because both OrcListReader
Yeah, agree it's more convenient to leave it here. I don't have strong feelings for either option.


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc
File be/src/exec/orc-column-readers.cc:

http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@451
PS1, Line 451: int OrcListReader::NumElements() const {
> Yeah, it's actually the same function.
I see. But we still gain something with codegen nested types so it's still a win. Thx for rewriting this.


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@473
PS1, Line 473: 
> If the list is an array of structs, then we can have many children, but in 
Indeed. Thx!


http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@477
PS1, Line 477:   return Status::OK();
> Yeah, it's confusing. slot_desc_ being null is independent of MaterializeTu
Hmm, According to my understanding MaterializeTuple() is true when the reader writes directly to a slot and false if the reader delegates the writing to its children. I might have misunderstood then.
Still, introducing DirectReader() helps readability, thx.



-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2020 13:13:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15730 )

Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2020 08:55:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9469: ORC scanner vectorization for collection types

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15730 )

Change subject: IMPALA-9469: ORC scanner vectorization for collection types
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5803/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15730
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5
Gerrit-Change-Number: 15730
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 14 Apr 2020 17:21:30 +0000
Gerrit-HasComments: No