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 <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 07 Nov 2018 17:23:24 +0000 Gerrit-HasComments: Yes
