[Impala-ASF-CR] IMPALA-4123: Columnar decoding in Parquet

["Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>]

Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8319 )

Change subject: IMPALA-4123: Columnar decoding in Parquet
......................................................................


Patch Set 14:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8319/14/be/src/exec/parquet-column-readers.cc
File be/src/exec/parquet-column-readers.cc:

http://gerrit.cloudera.org:8080/#/c/8319/14/be/src/exec/parquet-column-readers.cc@597
PS14, Line 597:   // If the file is corrupt, we may have more cached def levels than values in the page.
Shouldn't we raise a warning when the file is corrupt?


http://gerrit.cloudera.org:8080/#/c/8319/14/be/src/runtime/tuple.cc
File be/src/runtime/tuple.cc:

http://gerrit.cloudera.org:8080/#/c/8319/14/be/src/runtime/tuple.cc@210
PS14, Line 210: local_offset
nit: it seems that 'local_offset' is unnecessary and we could just use 'offset' in L212


http://gerrit.cloudera.org:8080/#/c/8319/14/be/src/util/rle-encoding.h
File be/src/util/rle-encoding.h:

http://gerrit.cloudera.org:8080/#/c/8319/14/be/src/util/rle-encoding.h@132
PS14, Line 132:       int64_t dict_len, OutType* values, int64_t stride) WARN_UNUSED_RESULT;
nit: you could use a default argument instead of having two functions, i.e. "..., int64_t stride = sizeof(OutType))".


http://gerrit.cloudera.org:8080/#/c/8319/14/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/8319/14/testdata/datasets/functional/functional_schema_template.sql@717
PS14, Line 717: INSERT OVERWRITE TABLE {db_name}{db_suffix}.{table_name} SELECT c.* FROM functional_parquet.complextypestbl c join functional.alltypes sort by id;
Do we need the same insert stmt after LOAD and after DEPENDENT_LOAD_HIVE?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c03006981c46ef0dae30602f2b73c253d9b49ef
Gerrit-Change-Number: 8319
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 17:23:24 +0000
Gerrit-HasComments: Yes