-
Notifications
You must be signed in to change notification settings - Fork 11
Distributed request to tables with Object Storage Engines #615
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
Changes from 17 commits
c22bf24
3a11374
dfd14d0
fb3e1b6
28d6c5c
78261d3
ac37da6
db44166
3fafe6f
cfc74ec
df462de
508e4ba
9dbd209
5b923c9
293ac83
5bc11ee
a4943e2
d5d3073
09321d3
17c53f4
e5dc250
14da835
0ccb6e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,10 +29,16 @@ class IStorageCluster : public IStorage | |
| SelectQueryInfo & query_info, | ||
| ContextPtr context, | ||
| QueryProcessingStage::Enum processed_stage, | ||
| size_t /*max_block_size*/, | ||
| size_t /*num_streams*/) override; | ||
| size_t max_block_size, | ||
| size_t num_streams) override; | ||
|
|
||
| ClusterPtr getCluster(ContextPtr context) const; | ||
| SinkToStoragePtr write( | ||
| const ASTPtr & query, | ||
| const StorageMetadataPtr & metadata_snapshot, | ||
| ContextPtr context, | ||
| bool async_insert) override; | ||
|
|
||
| ClusterPtr getCluster(ContextPtr context) const { return getClusterImpl(context, cluster_name); } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why move the implementation to the header file?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Single line implementation. In my opinion single-line implementation in header is more readable and simple to understanding than separate implementation in cpp-file, when declaration and implementation in two different places. |
||
| /// Query is needed for pruning by virtual columns (_file, _path) | ||
| virtual RemoteQueryExecutor::Extension getTaskIteratorExtension(const ActionsDAG::Node * predicate, const ContextPtr & context) const = 0; | ||
|
|
||
|
|
@@ -43,11 +49,38 @@ class IStorageCluster : public IStorage | |
| bool supportsOptimizationToSubcolumns() const override { return false; } | ||
| bool supportsTrivialCountOptimization(const StorageSnapshotPtr &, ContextPtr) const override { return true; } | ||
|
|
||
| const String & getOriginalClusterName() const { return cluster_name; } | ||
| virtual String getClusterName(ContextPtr /* context */) const { return getOriginalClusterName(); } | ||
|
|
||
| protected: | ||
| virtual void updateBeforeRead(const ContextPtr &) {} | ||
| virtual void updateQueryToSendIfNeeded(ASTPtr & /*query*/, const StorageSnapshotPtr & /*storage_snapshot*/, const ContextPtr & /*context*/) {} | ||
|
|
||
| virtual void readFallBackToPure( | ||
| QueryPlan & /* query_plan */, | ||
| const Names & /* column_names */, | ||
| const StorageSnapshotPtr & /* storage_snapshot */, | ||
| SelectQueryInfo & /* query_info */, | ||
| ContextPtr /* context */, | ||
| QueryProcessingStage::Enum /* processed_stage */, | ||
| size_t /* max_block_size */, | ||
| size_t /* num_streams */) | ||
| { | ||
| throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Method readFallBackToPure is not supported by storage {}", getName()); | ||
| } | ||
|
|
||
| virtual SinkToStoragePtr writeFallBackToPure( | ||
| const ASTPtr & /*query*/, | ||
| const StorageMetadataPtr & /*metadata_snapshot*/, | ||
| ContextPtr /*context*/, | ||
| bool /*async_insert*/) | ||
| { | ||
| throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Method writeFallBackToPure is not supported by storage {}", getName()); | ||
| } | ||
|
|
||
| private: | ||
| static ClusterPtr getClusterImpl(ContextPtr context, const String & cluster_name_); | ||
|
|
||
| LoggerPtr log; | ||
| String cluster_name; | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -363,11 +363,11 @@ void StorageS3Configuration::fromAST(ASTs & args, ContextPtr context, bool with_ | |
|
|
||
| if (engine_args_to_idx.contains("format")) | ||
| { | ||
| format = checkAndGetLiteralArgument<String>(args[engine_args_to_idx["format"]], "format"); | ||
| auto format_ = checkAndGetLiteralArgument<String>(args[engine_args_to_idx["format"]], "format"); | ||
| /// Set format to configuration only of it's not 'auto', | ||
| /// because we can have default format set in configuration. | ||
| if (format != "auto") | ||
| format = format; | ||
arthurpassos marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (format_ != "auto") | ||
| format = format_; | ||
| } | ||
|
|
||
| if (engine_args_to_idx.contains("structure")) | ||
|
|
@@ -585,6 +585,30 @@ void StorageS3Configuration::addStructureAndFormatToArgsIfNeeded( | |
| } | ||
| } | ||
|
|
||
| ASTPtr StorageS3Configuration::createArgsWithAccessData() const | ||
| { | ||
| auto arguments = std::make_shared<ASTExpressionList>(); | ||
|
|
||
| arguments->children.push_back(std::make_shared<ASTLiteral>(url.uri_str)); | ||
| if (auth_settings[S3AuthSetting::no_sign_request]) | ||
| { | ||
| arguments->children.push_back(std::make_shared<ASTLiteral>("NOSIGN")); | ||
| } | ||
| else | ||
| { | ||
| arguments->children.push_back(std::make_shared<ASTLiteral>(auth_settings[S3AuthSetting::access_key_id].value)); | ||
| arguments->children.push_back(std::make_shared<ASTLiteral>(auth_settings[S3AuthSetting::secret_access_key].value)); | ||
| if (!auth_settings[S3AuthSetting::session_token].value.empty()) | ||
| arguments->children.push_back(std::make_shared<ASTLiteral>(auth_settings[S3AuthSetting::session_token].value)); | ||
| if (format != "auto") | ||
| arguments->children.push_back(std::make_shared<ASTLiteral>(format)); | ||
| if (!compression_method.empty()) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it ok not to add some other arguments like
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this fields are already added in
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: perhaps add a comment saying this is already added somewhere else?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I rename method to |
||
| arguments->children.push_back(std::make_shared<ASTLiteral>(compression_method)); | ||
| } | ||
|
|
||
| return arguments; | ||
| } | ||
|
|
||
| } | ||
|
|
||
| #endif | ||
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.
Oh, one more thing. I believe you added this because of a CI/CD failure. @Enmk has merged a PR that fixes this. Perhaps you want to update your branch?