-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[thirdparty](patch) Support a better way to get offset in cctz #57999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
TPC-H: Total hot run time: 34794 ms |
TPC-DS: Total hot run time: 187525 ms |
ClickBench: Total hot run time: 27.92 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run p0 |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
zclllyybb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
|
run buildall |
TPC-H: Total hot run time: 34174 ms |
TPC-DS: Total hot run time: 188174 ms |
ClickBench: Total hot run time: 27.43 s |
|
PR approved by at least one committer and no changes requested. |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run p0 |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
HappenLee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: The construction cost of civil_second in cctz is expensive, details can be seen: #57423. In some cases, we do not need to construct the complete civil_time, but only the UTC offset is required, like:`HOUR(FROM_UNIXTIME(ts))` This PR supports the lookup_offset function in cctz, which has the same return type as lookup, but the civil_second is empty, avoiding the expensive construction cost. ```cpp return {{}, utc_offset, is_dst, abbreviations[abbr_index]}; ```
### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: The construction cost of civil_second in cctz is expensive, details can be seen: #57423. In some cases, we do not need to construct the complete civil_time, but only the UTC offset is required, like:`HOUR(FROM_UNIXTIME(ts))` This PR supports the lookup_offset function in cctz, which has the same return type as lookup, but the civil_second is empty, avoiding the expensive construction cost. ```cpp return {{}, utc_offset, is_dst, abbreviations[abbr_index]}; ```
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
The construction cost of civil_second in cctz is expensive, details can be seen: #57423.
In some cases, we do not need to construct the complete civil_time, but only the UTC offset is required, like:
HOUR(FROM_UNIXTIME(ts))This PR supports the lookup_offset function in cctz, which has the same return type as lookup, but the civil_second is empty, avoiding the expensive construction cost.
return {{}, utc_offset, is_dst, abbreviations[abbr_index]};Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)