Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21429 )
Change subject: IMPALA-13085: Add warning and NULL out DECIMAL values in Iceberg metadata tables ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/21429/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc File be/src/exec/iceberg-metadata/iceberg-row-reader.cc: http://gerrit.cloudera.org:8080/#/c/21429/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@125 PS1, Line 125: > represented as NULL, or displayed as NULL. NULLed out sounds a bit unusual. Done http://gerrit.cloudera.org:8080/#/c/21429/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@125 PS1, Line 125: RETURN_IF_ERROR(WriteDecimalSlot(slot_desc, tuple, state)); > The warning string could be extracted as a constant. I made it a constexpr char array. I don't think it would make sense to extract it out of this function, it is only used here and makes the function more readable. http://gerrit.cloudera.org:8080/#/c/21429/1/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@126 PS1, Line 126: break; > With this approach, the warning will be posted as many times the query enco I introduced a member that tracks whether the warning has already been emitted. Therefore one warning will be emitted for each scanner that encounters a DECIMAL from a metadata table. Also changed the type of the error from TErrorCode::GENERAL to TErrorCode::NOT_IMPLEMENTED_ERROR so these errors will be aggregated and displayed only once even if there are multiple scanners reading DECIMALs from metadata tables. -- To view, visit http://gerrit.cloudera.org:8080/21429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0c8791805bc4fa2112e092e65366ca2815f3fa22 Gerrit-Change-Number: 21429 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Peter Rozsa <pro...@cloudera.com> Gerrit-Comment-Date: Thu, 16 May 2024 11:01:54 +0000 Gerrit-HasComments: Yes
