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

Shard-level const column #27020

Merged
merged 8 commits into from
Aug 2, 2021

Conversation

amosbird
Copy link
Collaborator

@amosbird amosbird commented Jul 30, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Now functions can be shard-level constants, which means if it's executed in the context of some distributed table, it generates a normal column, otherwise it produces a constant value. Notable functions are: hostName(), tcpPort(), version(), buildId(), uptime(), etc.

This PR also introduces two functions: shardNum() and shardCount(). They are also shard-level constants and provide shard info during query execution.

The old _shard_num virtual column will still work but should be deprecated.

Detailed description / Documentation draft:
.

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Jul 30, 2021
@@ -42,6 +47,9 @@ class FunctionBuildId : public IFunction
{
return DataTypeString().createColumnConst(input_rows_count, SymbolIndex::instance()->getBuildIDHex());
}

private:
ContextPtr context;
Copy link
Member

Choose a reason for hiding this comment

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

Let's better store only is_distributed flag instead of whole context.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@KochetovNicolai KochetovNicolai left a comment

Choose a reason for hiding this comment

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

Awesome!

/** Get special scalar values
*/
template <typename Scalar>
class FunctionGetSpecialScalar : public IFunction, WithContext
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we don't actually need WithContext here.
Context is used only to get scalar in the constructor

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed.

@KochetovNicolai
Copy link
Member

Fast test is ok.
Not waiting for other checks, cause prev commit is green and the change is tiny

@KochetovNicolai KochetovNicolai merged commit 6951e81 into ClickHouse:master Aug 2, 2021
@gyuton
Copy link
Contributor

gyuton commented Aug 31, 2021

Internal documentation ticket: DOCSUP-14057.

azat added a commit to azat/ClickHouse that referenced this pull request Jan 27, 2022
_shard_num via constant identifier (from ClickHouse#7624) has too much issues,
take a look at ClickHouse#16947 for example.

shardNum() function (from ClickHouse#27020) should works better.

This changes the behaviour of _shard_num slightly, now it returns nested
shard number in case of nested distributed tables (Distributed over
Distributed), but this should be minor.

v2: Rewrite _shard_num to shardNum() in TreeRewriter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-improvement Pull request with some product improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants