Skye Wanderman-Milne has posted comments on this change. Change subject: IMPALA-2466: Add more tests for the HDFS parquet scanner. ......................................................................
Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/1500/3/testdata/datasets/functional/functional_schema_template.sql File testdata/datasets/functional/functional_schema_template.sql: Line 1805: lineitem_multiblock_one_rg Let's call this lineitem_multiblock_one_row_group, it's long but then it's obvious what it is. http://gerrit.cloudera.org:8080/#/c/1500/3/tests/query_test/test_scanners.py File tests/query_test/test_scanners.py: Line 219: # least one row group. This comment is a little stale, you can just say "The table functional_parquet.lineitem_multiblock has 3 blocks, so each impalad should read 1 scan range." Line 223: TABLE_NAME = 'functional_parquet.lineitem_multiblock' nit: this isn't constant in this function so lowercase 'table_name' Line 224: self._multiple_blocks_helper(TABLE_NAME, 20000) May as well explicitly specify ranges_per_node=1. Line 239: self._multiple_blocks_helper(TABLE_NAME, 40000, one_row_group=True) Specify ranges_per_block here too Line 244: verifies if the number of scan ranges issued is equal to the number of blocks there "the number of scan ranges issued is equal to the number of block there are": do you actually do this? Isn't it more like verifies 'ranges_per_node' and then checks that at least one row group was read? Line 342: Remove trailing whitespace -- To view, visit http://gerrit.cloudera.org:8080/1500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4faccd9ce3fad42402652c8f17d4e7aa3d593368 Gerrit-PatchSet: 3 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]> Gerrit-HasComments: Yes
