Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/15222 )
Change subject: WIP: IMPALA-9385: Fix Orc scanner's TimestampValue conversion ...................................................................... Patch Set 2: (5 comments) Hi, left some minor comments. http://gerrit.cloudera.org:8080/#/c/15222/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15222/2//COMMIT_MSG@7 PS2, Line 7: Orc nit: ORC (here and below) http://gerrit.cloudera.org:8080/#/c/15222/2//COMMIT_MSG@36 PS2, Line 36: Further planned changes Is it possible to gain some performance with this change? If yes, adding some measurements would be nice, I think. http://gerrit.cloudera.org:8080/#/c/15222/2/be/src/exec/parquet/parquet-common.h File be/src/exec/parquet/parquet-common.h: http://gerrit.cloudera.org:8080/#/c/15222/2/be/src/exec/parquet/parquet-common.h@743 PS2, Line 743: is nit: typo http://gerrit.cloudera.org:8080/#/c/15222/2/be/src/runtime/runtime-state.cc File be/src/runtime/runtime-state.cc: http://gerrit.cloudera.org:8080/#/c/15222/2/be/src/runtime/runtime-state.cc@150 PS2, Line 150: nullptr nit: maybe it would be worth to mention here as a side note that UTCPTR is in fact nullptr http://gerrit.cloudera.org:8080/#/c/15222/2/be/src/runtime/runtime-state.cc@152 PS2, Line 152: local_time_zone_ = tz == &TimezoneDatabase::GetUtcTimezone() ? UTCPTR : tz; Not related to this point, but as I see, the only other place (outside of some tests) you left GetUtcTimezone() in the code is in ImpalaServer::PrepareQueryContext. Wouldn't it make sense to refactor the code there as welll? -- To view, visit http://gerrit.cloudera.org:8080/15222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I14e2a7e512ccd013d5d9fe480a5467ed4c46b76e Gerrit-Change-Number: 15222 Gerrit-PatchSet: 2 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Norbert Luksa <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Mon, 17 Feb 2020 13:06:01 +0000 Gerrit-HasComments: Yes
