Csaba Ringhofer 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 2: Code-Review+1 (1 comment) 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 ParquetPlainEncoder::Decode<int64_t, parquet::Type::INT32>( optional: I would prefer to merge the common functionality somehow. One way is to call from these (and probably some other similar) functions a template function like template<FROM_T, TO_T>DecodeWithConversion(const uint8_t* buffer, const uint8_t* buffer_end, TO_T* v) that would do the memcpy + *v=dest. Another possibility would be to create a type mapper struct and extend the non-specialized Decode(): it could branch on std::is_same<InternalType, MapToParquetType<PARQUET_TYPE>> to decide between memcpy and memcpy + *v=dest. Doing the type mapping is quite simple and more useful then the comment table at line 189 IMO. Here is an example: https://stackoverflow.com/questions/4512757/type-mapping-by-templates -- 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: 2 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 21 Aug 2018 17:23:24 +0000 Gerrit-HasComments: Yes
