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.

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

fix: bugs related to merge scan #2118

Merged
merged 6 commits into from
Aug 8, 2023

Conversation

waynexia
Copy link
Member

@waynexia waynexia commented Aug 8, 2023

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

Fix some bugs related distributed query

  • prevent TypeConversionRule from convert types other than timestamp and boolean
  • prevent optimizer from optimize plan after MergeScan
  • use the schema from optimized plan as the output schema of MergeScan
  • optimize input plan if the MergeScan is executed only in frontend

The error log:

MySQL [(none)]>  SELECT SUM(-1) FROM numbers WHERE number=-1;
ERROR 1815 (HY000): Failed to execute query: SELECT SUM(-1) FROM numbers WHERE number=-1, source: Failed to execute logical plan, source: Failure during query execution, source: Fail to create physical plan: Internal error: The type of UInt32 Eq Int64 of binary physical should be same. This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

Signed-off-by: Ruihang Xia <[email protected]>
Copy link
Contributor

@evenyag evenyag left a comment

Choose a reason for hiding this comment

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

Please fix the title.

@waynexia waynexia changed the title Fix opt merge scan fix: bugs related to merge scan Aug 8, 2023
@waynexia waynexia requested a review from evenyag August 8, 2023 08:44
Copy link
Contributor

@evenyag evenyag left a comment

Choose a reason for hiding this comment

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

If it works not only on your machine ......

Signed-off-by: Ruihang Xia <[email protected]>
Copy link
Contributor

@v0y4g3r v0y4g3r left a comment

Choose a reason for hiding this comment

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

LGTM

@waynexia waynexia enabled auto-merge August 8, 2023 11:33
@waynexia waynexia added this pull request to the merge queue Aug 8, 2023
@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #2118 (6e10d78) into develop (7210b35) will decrease coverage by 0.43%.
Report is 13 commits behind head on develop.
The diff coverage is 97.36%.

@@             Coverage Diff             @@
##           develop    #2118      +/-   ##
===========================================
- Coverage    84.94%   84.51%   -0.43%     
===========================================
  Files          687      687              
  Lines       108052   108596     +544     
===========================================
- Hits         91786    91784       -2     
- Misses       16266    16812     +546     

Merged via the queue into GreptimeTeam:develop with commit 4c69379 Aug 8, 2023
11 checks passed
@waynexia waynexia deleted the fix-opt-merge-scan branch August 8, 2023 11:53
paomian pushed a commit to paomian/greptimedb that referenced this pull request Oct 19, 2023
* fix: prevent optimize merge scan, mark distinct as unsupported

Signed-off-by: Ruihang Xia <[email protected]>

* fix some other problems

Signed-off-by: Ruihang Xia <[email protected]>

* fix unit tests

Signed-off-by: Ruihang Xia <[email protected]>

* remove deadcode

Signed-off-by: Ruihang Xia <[email protected]>

* add some comments

Signed-off-by: Ruihang Xia <[email protected]>

* Update src/query/src/optimizer/type_conversion.rs

Co-authored-by: Lei, HUANG <[email protected]>

---------

Signed-off-by: Ruihang Xia <[email protected]>
Co-authored-by: Lei, HUANG <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants