Skip to content
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

[Bug](datev2) Fix wrong result when use datev2 as partition key #15094

Merged
merged 3 commits into from Dec 15, 2022
Merged

[Bug](datev2) Fix wrong result when use datev2 as partition key #15094

merged 3 commits into from Dec 15, 2022

Conversation

Gabriel39
Copy link
Contributor

@Gabriel39 Gabriel39 commented Dec 15, 2022 •
edited

Proposed changes

Hash value for datev2 is wrong.

Before:
image

after:
image

Problem summary

Describe your changes.

Checklist(Required)

  1. Does it affect the original behavior:
    • Yes
    • No
    • I don't know
  2. Has unit tests been added:
    • Yes
    • No
    • No Need
  3. Has document been added or modified:
    • Yes
    • No
    • No Need
  4. Does it need to update dependencies:
    • Yes
    • No
  5. Are there any changes that cannot be rolled back:
    • Yes (If Yes, please explain WHY)
    • No

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...

@github-actions github-actions bot added area/planner Issues or PRs related to the query planner kind/test labels Dec 15, 2022
@github-actions
Copy link
Contributor

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

1 similar comment
@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@hello-stephen
Copy link
Contributor

hello-stephen commented Dec 15, 2022 •
edited

TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 36.74 seconds
load time: 456 seconds
storage size: 17123356987 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221215075450_clickbench_pr_63773.html

@github-actions github-actions bot added the area/load Issues or PRs related to all kinds of load label Dec 15, 2022
@github-actions
Copy link
Contributor

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

yiguolei
yiguolei approved these changes Dec 15, 2022
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

@yiguolei yiguolei merged commit 5ef4c42 into apache:master Dec 15, 2022
@Gabriel39 Gabriel39 mentioned this pull request Dec 16, 2022
Merged
13 tasks
morningman pushed a commit that referenced this pull request Dec 16, 2022
@Gabriel39
[Bug](datev2) Fix compatible problems caused by datev2 (#15131)
67b9d46 
This bug is introduced by #15094
morningman pushed a commit that referenced this pull request Dec 19, 2022
@Gabriel39 @morningman
[Bug](datev2) Fix wrong result when use datev2 as partition key (#15094)
a282d72
morningman pushed a commit that referenced this pull request Dec 19, 2022
@Gabriel39 @morningman
[Bug](datev2) Fix compatible problems caused by datev2 (#15131)
d6f4f08 
This bug is introduced by #15094
@morningman morningman mentioned this pull request Dec 30, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@morningman morningman morningman left review comments

@yiguolei yiguolei yiguolei approved these changes

Assignees
No one assigned
Labels
area/load Issues or PRs related to all kinds of load area/planner Issues or PRs related to the query planner dev/1.2.1-merged kind/test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Gabriel39 @hello-stephen @morningman @yiguolei