Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8548 )
Change subject: IMPALA-5052: Read and write signed integer logical types in Parquet ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/8548/1/testdata/data/signed_integer_logical_types.parquet File testdata/data/signed_integer_logical_types.parquet: PS1: Can you add a description of this file to the readme - i.e. what it has in it and how it was generated? I see that the commit message has some info but i should be in the readme too. http://gerrit.cloudera.org:8080/#/c/8548/1/tests/query_test/test_insert_parquet.py File tests/query_test/test_insert_parquet.py: http://gerrit.cloudera.org:8080/#/c/8548/1/tests/query_test/test_insert_parquet.py@295 PS1, Line 295: colummn column http://gerrit.cloudera.org:8080/#/c/8548/1/tests/query_test/test_insert_parquet.py@325 PS1, Line 325: execute_query should these (above and below as well) be execute_query_expect_success? http://gerrit.cloudera.org:8080/#/c/8548/1/tests/query_test/test_insert_parquet.py@331 PS1, Line 331: result_dst = self.execute_query("describe %s" % dst_tbl) > If we got both FE and BE wrong (mapping smallint to int32 for example) I th +1. I think the test would be a little easier to understand too if we asserted the column types directly after creating each table. -- To view, visit http://gerrit.cloudera.org:8080/8548 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I47a8371858c9597c6a440808cf6f933532468927 Gerrit-Change-Number: 8548 Gerrit-PatchSet: 1 Gerrit-Owner: anujphadke <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 15 Nov 2017 19:06:21 +0000 Gerrit-HasComments: Yes
