Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12004 )
Change subject: IMPALA-7889: Write new logical types in Parquet ...................................................................... Patch Set 11: Code-Review+1 (2 comments) Looks like a test failed, too. http://gerrit.cloudera.org:8080/#/c/12004/11/be/src/exec/parquet/parquet-metadata-utils.cc File be/src/exec/parquet/parquet-metadata-utils.cc: http://gerrit.cloudera.org:8080/#/c/12004/11/be/src/exec/parquet/parquet-metadata-utils.cc@323 PS11, Line 323: case TYPE_DECIMAL: nit: indent case statements by 2 spaces (see rest of file). http://gerrit.cloudera.org:8080/#/c/12004/11/be/src/exec/parquet/parquet-metadata-utils.cc@354 PS11, Line 354: // Leave logical and converted types empty for other types. Have you considered enumerating the other types here? Then you can DCHECK on an unknown type. That way we make future mistakes less likely by forgetting to update this statement. -- To view, visit http://gerrit.cloudera.org:8080/12004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6f377950845683ab9c6dea79f4c54db0359d0b91 Gerrit-Change-Number: 12004 Gerrit-PatchSet: 11 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Anonymous Coward (359) Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Zoltan Ivanfi <[email protected]> Gerrit-Comment-Date: Wed, 19 Dec 2018 20:51:47 +0000 Gerrit-HasComments: Yes
