-
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
[CLICKHOUSE-3476] add invalidate_query for ClickHouse in Dictionary #3126
Conversation
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.
return readInvalidateQuery(dynamic_cast<IProfilingBlockInputStream&>((*input_block))); | ||
} | ||
|
||
auto invalidate_stream = RemoteBlockInputStream(pool, request, invalidate_sample_block, context); |
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.
Why not RemoteBlockInputStream invalidate_stream(pool, request, invalidate_sample_block, context);
?
@@ -143,4 +159,19 @@ BlockInputStreamPtr ClickHouseDictionarySource::createStreamForSelectiveLoad(con | |||
return std::make_shared<RemoteBlockInputStream>(pool, query, sample_block, context); | |||
} | |||
|
|||
std::string ClickHouseDictionarySource::doInvalidateQuery(const std::string & request) const | |||
{ | |||
Block invalidate_sample_block; |
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.
Unused variable in case of is_local
. Better to create if (is_local) ... else ...
.
@@ -143,4 +159,19 @@ BlockInputStreamPtr ClickHouseDictionarySource::createStreamForSelectiveLoad(con | |||
return std::make_shared<RemoteBlockInputStream>(pool, query, sample_block, context); | |||
} | |||
|
|||
std::string ClickHouseDictionarySource::doInvalidateQuery(const std::string & request) const | |||
{ | |||
Block invalidate_sample_block; |
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.
This block is sample_block
, which is used as header. It must contain single string column. Look at similar dictionaries: https://github.com/yandex/ClickHouse/blob/master/dbms/src/Dictionaries/MySQLDictionarySource.cpp#L219-L221 and https://github.com/yandex/ClickHouse/blob/master/dbms/src/Dictionaries/ODBCDictionarySource.cpp#L187-L189
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.
No! It does not work with CH.
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.
ok
Ok. But test is missing. |
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en