Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11563 )
Change subject: IMPALA-7644: Hide Parquet page index writing with feature flag ...................................................................... Patch Set 2: (5 comments) Thanks for the review! About query options: I see your point, but I prefer the command-line flag for the following reasons: - it is only a short-term and temporary solution, and I don't feel good about adding temporary query options - we don't really want customers to use it, and a command-line flag might be a bit more hidden/cumbersome to use for them - adding a new query option is more programming work and I am not sure if it worth the effort Maybe for developers it'd be a bit more convenient to set this flag via a query option, but I don't think that the difference is huge http://gerrit.cloudera.org:8080/#/c/11563/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11563/1//COMMIT_MSG@12 PS1, Line 12: write the page index. > Please mention here too that this is planned to be a temporary feature flag Done, also updated the description in global-flags.cc http://gerrit.cloudera.org:8080/#/c/11563/1/be/src/exec/hdfs-parquet-table-writer.cc File be/src/exec/hdfs-parquet-table-writer.cc: http://gerrit.cloudera.org:8080/#/c/11563/1/be/src/exec/hdfs-parquet-table-writer.cc@1316 PS1, Line 1316: Status HdfsParquetTableWriter::WriteFileFooter() { : // Write file_meta_data : > optional: I am not sure if this worth a separate function. Done http://gerrit.cloudera.org:8080/#/c/11563/1/tests/custom_cluster/test_parquet_page_index.py File tests/custom_cluster/test_parquet_page_index.py: http://gerrit.cloudera.org:8080/#/c/11563/1/tests/custom_cluster/test_parquet_page_index.py@39 PS1, Line 39: CustomClusterTestSuite > Please mention in a comment that this is meant to be a temporary solution u Updated the class comment and added TODO. http://gerrit.cloudera.org:8080/#/c/11563/1/tests/custom_cluster/test_parquet_page_index.py@272 PS1, Line 272: > flake8: E501 line too long (92 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/11563/1/tests/custom_cluster/test_parquet_page_index.py@346 PS1, Line 346: > flake8: E501 line too long (91 > 90 characters) Done -- To view, visit http://gerrit.cloudera.org:8080/11563 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9994882aa59cbaf3ae464100caa8211598287bc Gerrit-Change-Number: 11563 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 02 Oct 2018 14:24:17 +0000 Gerrit-HasComments: Yes
