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 expressions in metadata #9262

Merged
merged 10 commits into from
Feb 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 0 additions & 31 deletions dbms/programs/server/config.d/ssl.xml

This file was deleted.

30 changes: 29 additions & 1 deletion dbms/programs/server/config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,40 @@
<http_port>8123</http_port>
<tcp_port>9000</tcp_port>
<mysql_port>9004</mysql_port>
<!-- For HTTPS and SSL over native protocol. See also ssl.xml in config.d/-->
<!-- For HTTPS and SSL over native protocol. -->
<!--
<https_port>8443</https_port>
<tcp_port_secure>9440</tcp_port_secure>
-->

<!-- Used with https_port and tcp_port_secure. Full ssl options list: https://github.com/ClickHouse-Extras/poco/blob/master/NetSSL_OpenSSL/include/Poco/Net/SSLManager.h#L71 -->
<openSSL>
<server> <!-- Used for https server AND secure tcp port -->
<!-- openssl req -subj "/CN=localhost" -new -newkey rsa:2048 -days 365 -nodes -x509 -keyout /etc/clickhouse-server/server.key -out /etc/clickhouse-server/server.crt -->
<certificateFile>/etc/clickhouse-server/server.crt</certificateFile>
<privateKeyFile>/etc/clickhouse-server/server.key</privateKeyFile>
<!-- openssl dhparam -out /etc/clickhouse-server/dhparam.pem 4096 -->
<dhParamsFile>/etc/clickhouse-server/dhparam.pem</dhParamsFile>
<verificationMode>none</verificationMode>
<loadDefaultCAFile>true</loadDefaultCAFile>
<cacheSessions>true</cacheSessions>
<disableProtocols>sslv2,sslv3</disableProtocols>
<preferServerCiphers>true</preferServerCiphers>
</server>

<client> <!-- Used for connecting to https dictionary source -->
<loadDefaultCAFile>true</loadDefaultCAFile>
<cacheSessions>true</cacheSessions>
<disableProtocols>sslv2,sslv3</disableProtocols>
<preferServerCiphers>true</preferServerCiphers>
<!-- Use for self-signed: <verificationMode>none</verificationMode> -->
<invalidCertificateHandler>
<!-- Use for self-signed: <name>AcceptCertificateHandler</name> -->
<name>RejectCertificateHandler</name>
</invalidCertificateHandler>
</client>
</openSSL>

<!-- Default root page on http[s] server. For example load UI from https://tabix.io/ when opening http://localhost:8123 -->
<!--
<http_server_default_response><![CDATA[<html ng-app="SMI2"><head><base href="http://ui.tabix.io/"></head><body><div ui-view="" class="content-ui"></div><script src="http://loader.tabix.io/master.js"></script></body></html>]]></http_server_default_response>
Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Dictionaries/ClickHouseDictionarySource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ ClickHouseDictionarySource::ClickHouseDictionarySource(
, query_builder{dict_struct, db, table, where, IdentifierQuotingStyle::Backticks}
, sample_block{sample_block_}
, context(context_)
, is_local{isLocalAddress({host, port}, context.getTCPPort())}
, is_local{isLocalAddress({host, port}, secure ? context.getTCPPortSecure().value_or(0) : context.getTCPPort())}
, pool{is_local ? nullptr : createPool(host, port, secure, db, user, password)}
, load_all_query{query_builder.composeLoadAllQuery()}
{
Expand Down
1 change: 1 addition & 0 deletions dbms/src/Interpreters/InterpreterCreateQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ bool InterpreterCreateQuery::doCreateTable(const ASTCreateQuery & create,
return false;

StoragePtr res;
/// NOTE: CREATE query may be rewritten by Storage creator or table function
if (create.as_table_function)
{
const auto & table_function = create.as_table_function->as<ASTFunction &>();
Expand Down
27 changes: 21 additions & 6 deletions dbms/src/Interpreters/evaluateConstantExpression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <TableFunctions/TableFunctionFactory.h>
#include <Common/typeid_cast.h>
#include <Interpreters/ReplaceQueryParameterVisitor.h>
#include <Poco/Util/AbstractConfiguration.h>


namespace DB
Expand All @@ -25,6 +26,7 @@ namespace ErrorCodes
{
extern const int LOGICAL_ERROR;
extern const int BAD_ARGUMENTS;
extern const int UNKNOWN_DATABASE;
}


Expand Down Expand Up @@ -65,12 +67,6 @@ ASTPtr evaluateConstantExpressionAsLiteral(const ASTPtr & node, const Context &
/// If it's already a literal.
if (node->as<ASTLiteral>())
return node;

/// Skip table functions.
if (const auto * table_func_ptr = node->as<ASTFunction>())
if (TableFunctionFactory::instance().isTableFunctionName(table_func_ptr->name))
return node;

return std::make_shared<ASTLiteral>(evaluateConstantExpression(node, context).first);
}

Expand All @@ -82,6 +78,25 @@ ASTPtr evaluateConstantExpressionOrIdentifierAsLiteral(const ASTPtr & node, cons
return evaluateConstantExpressionAsLiteral(node, context);
}

ASTPtr evaluateConstantExpressionForDatabaseName(const ASTPtr & node, const Context & context)
{
ASTPtr res = evaluateConstantExpressionOrIdentifierAsLiteral(node, context);
auto & literal = res->as<ASTLiteral &>();
if (literal.value.safeGet<String>().empty())
{
String current_database = context.getCurrentDatabase();
if (current_database.empty())
{
/// Table was created on older version of ClickHouse and CREATE contains not folded expression.
/// Current database is not set yet during server startup, so we cannot evaluate it correctly.
literal.value = context.getConfigRef().getString("default_database", "default");
}
else
literal.value = current_database;
}
return res;
}

namespace
{
using Conjunction = ColumnsWithTypeAndName;
Expand Down
6 changes: 6 additions & 0 deletions dbms/src/Interpreters/evaluateConstantExpression.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ ASTPtr evaluateConstantExpressionAsLiteral(const ASTPtr & node, const Context &
*/
ASTPtr evaluateConstantExpressionOrIdentifierAsLiteral(const ASTPtr & node, const Context & context);

/** The same as evaluateConstantExpressionOrIdentifierAsLiteral(...),
* but if result is an empty string, replace it with current database name
* or default database name.
*/
ASTPtr evaluateConstantExpressionForDatabaseName(const ASTPtr & node, const Context & context);

/** Try to fold condition to countable set of constant values.
* @param condition a condition that we try to fold.
* @param target_expr expression evaluated over a set of constants.
Expand Down
8 changes: 8 additions & 0 deletions dbms/src/Interpreters/getClusterName.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,12 @@ std::string getClusterName(const IAST & node)
throw Exception("Illegal expression instead of cluster name.", ErrorCodes::BAD_ARGUMENTS);
}


String getClusterNameAndMakeLiteral(ASTPtr & node)
{
String cluster_name = getClusterName(*node);
node = std::make_shared<ASTLiteral>(cluster_name);
return cluster_name;
}

}
8 changes: 4 additions & 4 deletions dbms/src/Interpreters/getClusterName.h
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
#pragma once

#include <string>

#include <Parsers/IAST_fwd.h>

namespace DB
{

class IAST;

/// Get the cluster name from AST.
/** The name of the cluster is the name of the tag in the xml configuration.
* Usually it is parsed as an identifier. That is, it can contain underscores, but can not contain hyphens,
Expand All @@ -16,6 +14,8 @@ class IAST;
* This name will be parsed as an expression with an operator minus - not at all what you need.
* Therefore, consider this case separately.
*/
std::string getClusterName(const IAST & node);
String getClusterName(const IAST & node);

String getClusterNameAndMakeLiteral(ASTPtr & node);

}
73 changes: 40 additions & 33 deletions dbms/src/Storages/MergeTree/registerStorageMergeTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,10 @@ static StoragePtr create(const StorageFactory::Arguments & args)
}

ASTs & engine_args = args.engine_args;
size_t arg_num = 0;
size_t arg_cnt = engine_args.size();

if (engine_args.size() < min_num_params || engine_args.size() > max_num_params)
if (arg_cnt < min_num_params || arg_cnt > max_num_params)
{
String msg;
if (is_extended_storage_def)
Expand Down Expand Up @@ -477,15 +479,16 @@ static StoragePtr create(const StorageFactory::Arguments & args)

if (replicated)
{
const auto * ast = engine_args[0]->as<ASTLiteral>();
const auto * ast = engine_args[arg_num]->as<ASTLiteral>();
if (ast && ast->value.getType() == Field::Types::String)
zookeeper_path = safeGet<String>(ast->value);
else
throw Exception(
"Path in ZooKeeper must be a string literal" + getMergeTreeVerboseHelp(is_extended_storage_def),
ErrorCodes::BAD_ARGUMENTS);
++arg_num;

ast = engine_args[1]->as<ASTLiteral>();
ast = engine_args[arg_num]->as<ASTLiteral>();
if (ast && ast->value.getType() == Field::Types::String)
replica_name = safeGet<String>(ast->value);
else
Expand All @@ -497,39 +500,36 @@ static StoragePtr create(const StorageFactory::Arguments & args)
throw Exception(
"No replica name in config" + getMergeTreeVerboseHelp(is_extended_storage_def),
ErrorCodes::NO_REPLICA_NAME_GIVEN);

engine_args.erase(engine_args.begin(), engine_args.begin() + 2);
++arg_num;
}

if (merging_params.mode == MergeTreeData::MergingParams::Collapsing)
{
if (!tryGetIdentifierNameInto(engine_args.back(), merging_params.sign_column))
if (!tryGetIdentifierNameInto(engine_args[arg_cnt - 1], merging_params.sign_column))
throw Exception(
"Sign column name must be an unquoted string" + getMergeTreeVerboseHelp(is_extended_storage_def),
ErrorCodes::BAD_ARGUMENTS);

engine_args.pop_back();
--arg_cnt;
}
else if (merging_params.mode == MergeTreeData::MergingParams::Replacing)
{
/// If the last element is not index_granularity or replica_name (a literal), then this is the name of the version column.
if (!engine_args.empty() && !engine_args.back()->as<ASTLiteral>())
if (arg_cnt && !engine_args[arg_cnt - 1]->as<ASTLiteral>())
{
if (!tryGetIdentifierNameInto(engine_args.back(), merging_params.version_column))
if (!tryGetIdentifierNameInto(engine_args[arg_cnt - 1], merging_params.version_column))
throw Exception(
"Version column name must be an unquoted string" + getMergeTreeVerboseHelp(is_extended_storage_def),
ErrorCodes::BAD_ARGUMENTS);

engine_args.pop_back();
--arg_cnt;
}
}
else if (merging_params.mode == MergeTreeData::MergingParams::Summing)
{
/// If the last element is not index_granularity or replica_name (a literal), then this is a list of summable columns.
if (!engine_args.empty() && !engine_args.back()->as<ASTLiteral>())
if (arg_cnt && !engine_args[arg_cnt - 1]->as<ASTLiteral>())
{
merging_params.columns_to_sum = extractColumnNames(engine_args.back());
engine_args.pop_back();
merging_params.columns_to_sum = extractColumnNames(engine_args[arg_cnt - 1]);
--arg_cnt;
}
}
else if (merging_params.mode == MergeTreeData::MergingParams::Graphite)
Expand All @@ -538,7 +538,7 @@ static StoragePtr create(const StorageFactory::Arguments & args)
String error_msg = "Last parameter of GraphiteMergeTree must be name (in single quotes) of element in configuration file with Graphite options";
error_msg += getMergeTreeVerboseHelp(is_extended_storage_def);

if (const auto * ast = engine_args.back()->as<ASTLiteral>())
if (const auto * ast = engine_args[arg_cnt - 1]->as<ASTLiteral>())
{
if (ast->value.getType() != Field::Types::String)
throw Exception(error_msg, ErrorCodes::BAD_ARGUMENTS);
Expand All @@ -548,24 +548,24 @@ static StoragePtr create(const StorageFactory::Arguments & args)
else
throw Exception(error_msg, ErrorCodes::BAD_ARGUMENTS);

engine_args.pop_back();
--arg_cnt;
setGraphitePatternsFromConfig(args.context, graphite_config_name, merging_params.graphite_params);
}
else if (merging_params.mode == MergeTreeData::MergingParams::VersionedCollapsing)
{
if (!tryGetIdentifierNameInto(engine_args.back(), merging_params.version_column))
if (!tryGetIdentifierNameInto(engine_args[arg_cnt - 1], merging_params.version_column))
throw Exception(
"Version column name must be an unquoted string" + getMergeTreeVerboseHelp(is_extended_storage_def),
ErrorCodes::BAD_ARGUMENTS);

engine_args.pop_back();
--arg_cnt;

if (!tryGetIdentifierNameInto(engine_args.back(), merging_params.sign_column))
if (!tryGetIdentifierNameInto(engine_args[arg_cnt - 1], merging_params.sign_column))
throw Exception(
"Sign column name must be an unquoted string" + getMergeTreeVerboseHelp(is_extended_storage_def),
ErrorCodes::BAD_ARGUMENTS);

engine_args.pop_back();
--arg_cnt;
}

String date_column_name;
Expand Down Expand Up @@ -614,31 +614,38 @@ static StoragePtr create(const StorageFactory::Arguments & args)
}
else
{
/// If there is an expression for sampling. MergeTree(date, [sample_key], primary_key, index_granularity)
if (engine_args.size() == 4)
{
sample_by_ast = engine_args[1];
engine_args.erase(engine_args.begin() + 1);
}

/// Now only three parameters remain - date (or partitioning expression), primary_key, index_granularity.

if (!tryGetIdentifierNameInto(engine_args[0], date_column_name))
/// Syntax: *MergeTree(..., date, [sample_key], primary_key, index_granularity, ...)
/// Get date:
if (!tryGetIdentifierNameInto(engine_args[arg_num], date_column_name))
throw Exception(
"Date column name must be an unquoted string" + getMergeTreeVerboseHelp(is_extended_storage_def),
ErrorCodes::BAD_ARGUMENTS);
++arg_num;

order_by_ast = engine_args[1];
/// If there is an expression for sampling
if (arg_cnt - arg_num == 3)
{
sample_by_ast = engine_args[arg_num];
++arg_num;
}

const auto * ast = engine_args.back()->as<ASTLiteral>();
/// Now only two parameters remain - primary_key, index_granularity.
order_by_ast = engine_args[arg_num];
++arg_num;

const auto * ast = engine_args[arg_num]->as<ASTLiteral>();
if (ast && ast->value.getType() == Field::Types::UInt64)
storage_settings->index_granularity = safeGet<UInt64>(ast->value);
else
throw Exception(
"Index granularity must be a positive integer" + getMergeTreeVerboseHelp(is_extended_storage_def),
ErrorCodes::BAD_ARGUMENTS);
++arg_num;
}

if (arg_num != arg_cnt)
throw Exception("Wrong number of engine arguments.", ErrorCodes::BAD_ARGUMENTS);

if (!args.attach && !indices_description.empty() && !args.local_context.getSettingsRef().allow_experimental_data_skipping_indices)
throw Exception("You must set the setting `allow_experimental_data_skipping_indices` to 1 " \
"before using data skipping indices.", ErrorCodes::BAD_ARGUMENTS);
Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Storages/StorageBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ void registerStorageBuffer(StorageFactory & factory)
" destination_database, destination_table, num_buckets, min_time, max_time, min_rows, max_rows, min_bytes, max_bytes.",
ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH);

engine_args[0] = evaluateConstantExpressionOrIdentifierAsLiteral(engine_args[0], args.local_context);
engine_args[0] = evaluateConstantExpressionForDatabaseName(engine_args[0], args.local_context);
engine_args[1] = evaluateConstantExpressionOrIdentifierAsLiteral(engine_args[1], args.local_context);

String destination_database = engine_args[0]->as<ASTLiteral &>().value.safeGet<String>();
Expand Down
5 changes: 4 additions & 1 deletion dbms/src/Storages/StorageDistributed.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,9 @@ void StorageDistributed::alter(const AlterCommands & params, const Context & con

void StorageDistributed::startup()
{
if (remote_database.empty() && !remote_table_function_ptr)
LOG_WARNING(log, "Name of remote database is empty. Default database will be used implicitly.");

if (!volume)
return;

Expand Down Expand Up @@ -719,7 +722,7 @@ void registerStorageDistributed(StorageFactory & factory)
"policy to store data in (optional).",
ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH);

String cluster_name = getClusterName(*engine_args[0]);
String cluster_name = getClusterNameAndMakeLiteral(engine_args[0]);

engine_args[1] = evaluateConstantExpressionOrIdentifierAsLiteral(engine_args[1], args.local_context);
engine_args[2] = evaluateConstantExpressionOrIdentifierAsLiteral(engine_args[2], args.local_context);
Expand Down