Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11268 )
Change subject: IMPALA-6373: Allow primitive type widening on parquet tables ...................................................................... Patch Set 3: (3 comments) Rebased the CR. Csaba/Zoltan, do you want to take another look for the +2? http://gerrit.cloudera.org:8080/#/c/11268/2/be/src/exec/parquet-common.h File be/src/exec/parquet-common.h: http://gerrit.cloudera.org:8080/#/c/11268/2/be/src/exec/parquet-common.h@253 PS2, Line 253: inline int DecodeWithConversion(const uint8_t* buffer, const uint8_t* buffer_end, To* v) { > optional: I would prefer to merge the common functionality somehow. Thanks for the suggestion. I updated the code to remove code duplication as per your first suggestion. I didn't go further as to create a type mapper, but I can do that in a different patch :) http://gerrit.cloudera.org:8080/#/c/11268/2/be/src/exec/parquet-plain-test.cc File be/src/exec/parquet-plain-test.cc: http://gerrit.cloudera.org:8080/#/c/11268/2/be/src/exec/parquet-plain-test.cc@138 PS2, Line 138: TestType > nit: I'd choose a different name, e.g. 'TestTypeWidening' Done http://gerrit.cloudera.org:8080/#/c/11268/2/tests/query_test/test_scanners.py File tests/query_test/test_scanners.py: http://gerrit.cloudera.org:8080/#/c/11268/2/tests/query_test/test_scanners.py@758 PS2, Line 758: self.run_test_case('QueryTest/hdfs-text-scan', vector) : : # Test various escape char cases. We have to check the count(*) result against : # the count(col) result because if the scan range is split right after the escape : # char, the escape char has no effect because we cannot scan backwards to the : # previous scan range. : for t in self.ESCAPE_TABLE_LIST: : expected_result = self.client.execute("select count(col) from " + t) > It seems https://gerrit.cloudera.org/#/c/11127/ is about to go in, so you c Thanks. It looks a lot nicer. Done. -- To view, visit http://gerrit.cloudera.org:8080/11268 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If93394b035c64cf6fc5f37b54d29c034cc1f86e4 Gerrit-Change-Number: 11268 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 22 Aug 2018 19:28:32 +0000 Gerrit-HasComments: Yes
