Skip to content

[BUG](date_trunc) fix date_trunc function only handle lower string #22602

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.

Sign up for GitHub

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

Jump to bottom
Merged
BiteTheDDDDt merged 4 commits into from
Aug 5, 2023
Merged

[BUG](date_trunc) fix date_trunc function only handle lower string #22602

BiteTheDDDDt merged 4 commits into from
Aug 5, 2023

Conversation

@zhangstar333
Copy link
Contributor

@zhangstar333 zhangstar333 commented Aug 4, 2023

Proposed changes

date_trunc function only handle lower string, so need convert this param to lower case.

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@zhangstar333 zhangstar333 marked this pull request as ready for review August 4, 2023 05:33
@zhangstar333
Copy link
Contributor Author

zhangstar333 commented Aug 4, 2023

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

clang-tidy review says "All clean, LGTM! 👍"

yiguolei
yiguolei reviewed Aug 4, 2023
View reviewed changes
be/src/vec/functions/function_timestamp.cpp
const StringRef& rdata, PaddedPODArray<ArgType>& res,
NullMap& null_map, size_t input_rows_count) {
res.resize(input_rows_count);
std::string lower_str(rdata.data, rdata.size);
Copy link
Contributor

@yiguolei yiguolei Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add regression test please

@yiguolei yiguolei added dev/2.0.0 2.0.0 release usercase Important user case type label labels Aug 4, 2023
@zhangstar333
Copy link
Contributor Author

zhangstar333 commented Aug 4, 2023

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

clang-tidy review says "All clean, LGTM! 👍"

@zhangstar333
Copy link
Contributor Author

zhangstar333 commented Aug 4, 2023

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

clang-tidy review says "All clean, LGTM! 👍"

@hello-stephen
Copy link
Contributor

hello-stephen commented Aug 4, 2023

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 44.77 seconds
stream load tsv: 512 seconds loaded 74807831229 Bytes, about 139 MB/s
stream load json: 20 seconds loaded 2358488459 Bytes, about 112 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 31 seconds loaded 861443392 Bytes, about 26 MB/s
insert into select: 29.1 seconds inserted 10000000 Rows, about 343K ops/s
storage size: 17162084832 Bytes

@hello-stephen
Copy link
Contributor

hello-stephen commented Aug 4, 2023

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 47.31 seconds
stream load tsv: 514 seconds loaded 74807831229 Bytes, about 138 MB/s
stream load json: 20 seconds loaded 2358488459 Bytes, about 112 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 30 seconds loaded 861443392 Bytes, about 27 MB/s
insert into select: 30.1 seconds inserted 10000000 Rows, about 332K ops/s
storage size: 17162270464 Bytes

@zhangstar333
Copy link
Contributor Author

zhangstar333 commented Aug 4, 2023

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

clang-tidy review says "All clean, LGTM! 👍"

@hello-stephen
Copy link
Contributor

hello-stephen commented Aug 4, 2023

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 48.37 seconds
stream load tsv: 514 seconds loaded 74807831229 Bytes, about 138 MB/s
stream load json: 20 seconds loaded 2358488459 Bytes, about 112 MB/s
stream load orc: 66 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
insert into select: 29.4 seconds inserted 10000000 Rows, about 340K ops/s
storage size: 17162345718 Bytes

yiguolei
yiguolei approved these changes Aug 4, 2023
View reviewed changes
Copy link
Contributor

@yiguolei yiguolei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Aug 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

PR approved by anyone and no changes requested.

BiteTheDDDDt
BiteTheDDDDt reviewed Aug 5, 2023
View reviewed changes
regression-test/suites/query_p0/sql_functions/datetime_functions/test_date_trunc.groovy

try {
sql """ SELECT date_trunc(k11,k7) FROM baseall ; """
} catch (Exception e) {
Copy link
Contributor

@BiteTheDDDDt BiteTheDDDDt Aug 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no exception is thrown?

BiteTheDDDDt
BiteTheDDDDt reviewed Aug 5, 2023
View reviewed changes
regression-test/data/query_p0/sql_functions/datetime_functions/test_date_trunc.out
1989-03-21T13:00 1989-03-21T13:00
2000-01-01T00:00 2000-01-01T00:00
2015-03-13T10:30 2015-03-13T10:30
2015-03-13T12:36:38 2015-03-13T12:36:38
Copy link
Contributor

@BiteTheDDDDt BiteTheDDDDt Aug 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the result looks unordered？

@BiteTheDDDDt BiteTheDDDDt merged commit d3b50e3 into apache:master Aug 5, 2023
@xiaokang xiaokang removed the dev/2.0.1 label Aug 5, 2023
@xiaokang xiaokang added dev/2.0.0-merged and removed dev/2.0.0 2.0.0 release labels Aug 5, 2023
xiaokang pushed a commit that referenced this pull request Aug 5, 2023
@zhangstar333 @xiaokang
[BUG](date_trunc) fix date_trunc function only handle lower string (#…
c5db635 
…22602)

fix date_trunc function only handle lower string
@zhangstar333 zhangstar333 mentioned this pull request Oct 24, 2023
Merged
yiguolei pushed a commit that referenced this pull request Oct 26, 2023
@zhangstar333
[improvement](function) improve date_trunc function performance when …
da4de17 
…timeunit is const (#25824)

this PR #22602 have check function.
only support date_trunc(column, const), so the second must be const literal
and no need to check time unit every row.
dutyu pushed a commit to dutyu/doris that referenced this pull request Oct 28, 2023
@zhangstar333
[improvement](function) improve date_trunc function performance when …
be7043b 
…timeunit is const (apache#25824)

this PR apache#22602 have check function.
only support date_trunc(column, const), so the second must be const literal
and no need to check time unit every row.
xiaokang pushed a commit to xiaokang/doris that referenced this pull request Oct 31, 2023
@zclllyybb @xiaokang
[improvement](function) improve date_trunc function performance when …
4df276a 
…timeunit is const (apache#25824)

this PR apache#22602 have check function.
only support date_trunc(column, const), so the second must be const literal
and no need to check time unit every row.
xiaokang pushed a commit to xiaokang/doris that referenced this pull request Nov 1, 2023
@zclllyybb @xiaokang
[improvement](function) improve date_trunc function performance when …
4e78a3d 
…timeunit is const (apache#25824)

this PR apache#22602 have check function.
only support date_trunc(column, const), so the second must be const literal
and no need to check time unit every row.
gnehil pushed a commit to gnehil/doris that referenced this pull request Dec 4, 2023
@zclllyybb
[improvement](function) improve date_trunc function performance when …
c1d29c7 
…timeunit is const (apache#25824)

this PR apache#22602 have check function.
only support date_trunc(column, const), so the second must be const literal
and no need to check time unit every row.
XuJianxu pushed a commit to XuJianxu/doris that referenced this pull request Dec 14, 2023
@zhangstar333
[improvement](function) improve date_trunc function performance when …
0373db9 
…timeunit is const (apache#25824)

this PR apache#22602 have check function.
only support date_trunc(column, const), so the second must be const literal
and no need to check time unit every row.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BiteTheDDDDt BiteTheDDDDt BiteTheDDDDt left review comments

@yiguolei yiguolei yiguolei approved these changes

@morrySnow morrySnow morrySnow approved these changes

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. dev/2.0.0-merged reviewed usercase Important user case type label

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@zhangstar333 @hello-stephen @BiteTheDDDDt @yiguolei @morrySnow @xiaokang