Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9693 )
Change subject: IMPALA-5842: Write page index in Parquet files ...................................................................... Patch Set 18: (4 comments) http://gerrit.cloudera.org:8080/#/c/9693/17/tests/query_test/test_parquet_page_index.py File tests/query_test/test_parquet_page_index.py: http://gerrit.cloudera.org:8080/#/c/9693/17/tests/query_test/test_parquet_page_index.py@71 PS17, Line 71: if column_index_offset and column_index_length: > It might make things faster / more elegant to pass an open file here, other Done http://gerrit.cloudera.org:8080/#/c/9693/17/tests/query_test/test_parquet_page_index.py@126 PS17, Line 126: es = page_header.data > Why not "not page_is_null"? Guess I wrote too many "is None"... Done. http://gerrit.cloudera.org:8080/#/c/9693/17/tests/query_test/test_parquet_page_index.py@179 PS17, Line 179: > "reverse" seems to rely on everyone thinking of ascending as the default. M The Python sorted() function also has a reversed flag with the same meaning, so maybe it's the Python convention. If you still think I should be more explicit, I'll change the name of it. We could use BoundaryOrder, and define it to UNORDERED, but we couldn't really use it because BoundaryOrder says something about two lists, not just one. I.e., testing the following in case of UNORDERED wouldn't be enough: is_sorted(min_values, UNORDERED) or is_sorted(max_values, UNORDERED) E.g. if min_values is ascending and max_values is descending the above expression would evaluate to false, but the boundary order should be UNORDERED. http://gerrit.cloudera.org:8080/#/c/9693/17/tests/util/get_parquet_metadata.py File tests/util/get_parquet_metadata.py: http://gerrit.cloudera.org:8080/#/c/9693/17/tests/util/get_parquet_metadata.py@163 PS17, Line 163: > If you follow the suggestion in the other file to pass an open file to read Done -- To view, visit http://gerrit.cloudera.org:8080/9693 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icbacf7fe3b7672e3ce719261ecef445b16f8dec9 Gerrit-Change-Number: 9693 Gerrit-PatchSet: 18 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Anonymous Coward #248 Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 15 May 2018 16:53:48 +0000 Gerrit-HasComments: Yes
