-
Notifications
You must be signed in to change notification settings - Fork 7k
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
ISSUES-3134 fix merge and distributed engine query stage #3159
ISSUES-3134 fix merge and distributed engine query stage #3159
Conversation
a7d28f2
to
3dc4172
Compare
Two tests have failed. |
@@ -3,6 +3,3 @@ | |||
1000 | |||
1000 | |||
1000 | |||
0 |
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.
Changes to test behavior. zero should be different from non_existing
. But I'm not sure.
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.
It's important to heave zeroes here.
@@ -14,7 +14,8 @@ | |||
1: | |||
1 | |||
1 | |||
-1: | |||
-1: | |||
18446744073709551615 |
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.
Maybe the previous test cases were problematic, but I'm not sure.
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.
It is not correct, because the query was SELECT x FROM test.merge_s64_u64 WHERE x IN (-1);
.
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.
@KochetovNicolai I need some help : )
My current idea is to rerun the filter stream. but the required columns in the where_expression may no longer exist. can I get some advice here ? Code
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.
Maybe I need to push down the where_expression_info
and remove_where_column ? Like prewhere_expression_info
?
958e75a
to
866dacb
Compare
Done. |
Something is wrong: https://travis-ci.org/yandex/ClickHouse/builds/430907161#L8332 |
866dacb
to
884eaca
Compare
884eaca
to
e0e805b
Compare
fixed test failure and |
/// If the processed_stage greater than FetchColumns and the block structure between streams is different. | ||
/// the where expression maybe invalid because of convertingBlockInputStream. | ||
/// So we need to throw exception. | ||
if (!header_column.type->equals(*before_column.type.get()) && processed_stage > QueryProcessingStage::FetchColumns) |
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.
added new exception. related test cases: https://github.com/yandex/ClickHouse/pull/3159/files#diff-703c5c0ba023631e656f4ed2d2df4c39R79.
Align
processed_stage
with subqueries when not distributed tables.#3134 #3110
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en