Michael Smith 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

    const char sign = normalized_offset[0]; // '+' or '-'
    // normalized_offset is already validated to be 3, 5, or 6 chars
    int offset_hours = 0, offset_minutes = 0;
    if (normalized_offset.length() == 3) {           // +HH
      offset_hours = std::stoi(normalized_offset.substr(1));
    } else if (normalized_offset.length() == 5) {    // +HHMM
      offset_hours   = std::stoi(normalized_offset.substr(1, 2));
      offset_minutes = std::stoi(normalized_offset.substr(3, 2));
    } else {                                          // +HH:MM
      offset_hours   = std::stoi(normalized_offset.substr(1, 2));
      offset_minutes = std::stoi(normalized_offset.substr(4, 2));
    }
    if (offset_hours > 23 || offset_minutes > 59) return false;
    timezone_offset = time_duration(offset_hours, offset_minutes, 0);
    if (sign == '-') timezone_offset = -timezone_offset;


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

    if (offset_magnitude.hours() > 23
        || offset_magnitude.minutes() > 59
        || offset_magnitude.seconds() != 0
        || offset_magnitude.fractional_seconds() != 0) {


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

    *unix_time_millis = (utc_time - ptime(UNIX_EPOCH)).total_milliseconds();



--
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: Thu, 16 Jul 2026 16:21:35 +0000
Gerrit-HasComments: Yes

Reply via email to