Aleksandr Efimov has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24422 )

Change subject: IMPALA-15081: Add ISO timestamp filters
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/24422/6/be/src/util/impala-profile-tool.cc
File be/src/util/impala-profile-tool.cc:

http://gerrit.cloudera.org:8080/#/c/24422/6/be/src/util/impala-profile-tool.cc@161
PS6, Line 161:       timezone_offset = duration_from_string(normalized_offset);
> Could the duration_from_string parsing be replaced with simple stoi, like
Thanks for the review, Michael. I replaced duration_from_string with explicit 
std::stoi parsing. I kept a digit check before std::stoi so malformed offsets 
such as +0a:00 are rejected rather than partially parsed.


http://gerrit.cloudera.org:8080/#/c/24422/6/be/src/util/impala-profile-tool.cc@167
PS6, Line 167:     if (offset_magnitude.hours() > 23
> Could this be replaced with
The offset_magnitude check is no longer needed after the parsing change. Hours 
and minutes are now validated directly against 23 and 59, and the parser only 
constructs whole-minute time_duration values.


http://gerrit.cloudera.org:8080/#/c/24422/6/be/src/util/impala-profile-tool.cc@190
PS6, Line 190:     *unix_time_millis = days_since_epoch * SECS_PER_DAY * 
MILLIS_PER_SEC
> Could this be
I switched the conversion to total_milliseconds(). Since it truncates negative 
sub-millisecond durations toward zero, I kept sign-aware rounding for inclusive 
bounds and added pre-epoch coverage for both min and max.



--
To view, visit http://gerrit.cloudera.org:8080/24422
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2c43e7535db48518b7d9dd0cbb87398e639c3b72
Gerrit-Change-Number: 24422
Gerrit-PatchSet: 6
Gerrit-Owner: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Fri, 17 Jul 2026 05:11:12 +0000
Gerrit-HasComments: Yes

Reply via email to