Lars Volker has posted comments on this change.

Change subject: IMPALA-4815, IMPALA-4817, IMPALA-4819: Write and Read Parquet 
Statistics for remaining types
......................................................................


Patch Set 8:

(2 comments)

Thanks for the review. I addressed your comments in PS9. Next, I'll rebase the 
change.

http://gerrit.cloudera.org:8080/#/c/6563/6/be/src/exec/parquet-column-stats.cc
File be/src/exec/parquet-column-stats.cc:

Line 104:     case TYPE_DECIMAL:
> doesn't look like it does, but if i'm wrong, that would need to be disabled
The stats Update() method receives the same values that the ParquetPlainEncoder 
writes to the file as data. The len in those values is set to 
UnpaddedCharLength(). test_write_statistics_char_types() also ensures that they 
are written unpadded.


http://gerrit.cloudera.org:8080/#/c/6563/8/be/src/exec/parquet-column-stats.h
File be/src/exec/parquet-column-stats.h:

Line 99:   // Copies the memory of 'value' into 'buffer' and make 'value' point 
to 'buffer'.
> i didn't realize that this resets 'buffer' ('copies into' can also mean app
I expanded the comment.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ef4a5d25a57c82577fd498d6d1c4297ecf39312
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <[email protected]>
Gerrit-Reviewer: Attila Jeges <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to