fix style and add tests

This commit is contained in:
Alexander Tretiakov 2019-05-25 16:43:52 +03:00
parent 9099f1810b
commit 946fa5b47e
19 changed files with 145 additions and 94 deletions

View File

@ -87,7 +87,7 @@ endif ()
add_subdirectory (src)
set(dbms_headers)
set(dbms_sources src/Interpreters/ReplaceQueryParameterVisitor.cpp src/Interpreters/ReplaceQueryParameterVisitor.h)
set(dbms_sources)
include(../cmake/dbms_glob_sources.cmake)
@ -134,6 +134,9 @@ list (APPEND dbms_headers src/TableFunctions/ITableFunction.h src/TableFunctio
list (APPEND dbms_sources src/Dictionaries/DictionaryFactory.cpp src/Dictionaries/DictionarySourceFactory.cpp src/Dictionaries/DictionaryStructure.cpp)
list (APPEND dbms_headers src/Dictionaries/DictionaryFactory.h src/Dictionaries/DictionarySourceFactory.h src/Dictionaries/DictionaryStructure.h)
list (APPEND dbms_sources src/Interpreters/ReplaceQueryParameterVisitor.cpp)
list (APPEND dbms_headers src/Interpreters/ReplaceQueryParameterVisitor.h)
add_library(clickhouse_common_io ${clickhouse_common_io_headers} ${clickhouse_common_io_sources})
if (OS_FREEBSD)

View File

@ -203,7 +203,7 @@ private:
std::list<ExternalTable> external_tables;
/// Dictionary with query parameters for prepared statements.
NameToNameMap params_substitution;
NameToNameMap parameters_substitution;
ConnectionParameters connection_parameters;
@ -806,10 +806,10 @@ private:
if (!parsed_query)
return true;
if (!params_substitution.empty())
if (!parameters_substitution.empty())
{
/// Replace ASTQueryParameter with ASTLiteral for prepared statements.
ReplaceQueryParameterVisitor visitor(params_substitution);
ReplaceQueryParameterVisitor visitor(parameters_substitution);
visitor.visit(parsed_query);
/// Get new query after substitutions.
@ -1550,11 +1550,11 @@ private:
std::cout << DBMS_NAME << " client version " << VERSION_STRING << VERSION_OFFICIAL << "." << std::endl;
}
static std::pair<String, String> parseParam(const String & s)
static std::pair<String, String> parseParameter(const String & s)
{
size_t pos = s.find('_') + 1;
/// Cut two first dash "--" and divide arg from name and value
return std::make_pair(s.substr(2, pos - 2), s.substr(pos));
return {s.substr(2, pos - 2), s.substr(pos)};
}
public:
@ -1574,7 +1574,7 @@ public:
Arguments common_arguments{""}; /// 0th argument is ignored.
std::vector<Arguments> external_tables_arguments;
std::vector<Arguments> param_arguments;
std::vector<Arguments> parameter_arguments;
bool in_external_group = false;
for (int arg_num = 1; arg_num < argc; ++arg_num)
@ -1621,8 +1621,8 @@ public:
/// Parameter arg after underline.
if (startsWith(arg, "--param_"))
{
param_arguments.emplace_back(Arguments{""});
param_arguments.back().emplace_back(arg);
parameter_arguments.emplace_back(Arguments{""});
parameter_arguments.back().emplace_back(arg);
}
else
common_arguments.emplace_back(arg);
@ -1702,36 +1702,30 @@ public:
;
/// Parse commandline options related to prepared statements.
po::options_description param_description("Query parameters options");
param_description.add_options()
("param_", po::value<std::string>(), "name and value of substitution")
po::options_description parameter_description("Query parameters options");
parameter_description.add_options()
("param_", po::value<std::string>(), "name and value of substitution, with syntax --param_name=value")
;
for (size_t i = 0; i < param_arguments.size(); ++i)
for (size_t i = 0; i < parameter_arguments.size(); ++i)
{
po::parsed_options parsed_param = po::command_line_parser(
param_arguments[i].size(), param_arguments[i].data()).options(param_description).extra_parser(
parseParam).run();
po::variables_map param_options;
po::store(parsed_param, param_options);
po::parsed_options parsed_parameter = po::command_line_parser(
parameter_arguments[i].size(), parameter_arguments[i].data()).options(parameter_description).extra_parser(
parseParameter).run();
po::variables_map parameter_options;
po::store(parsed_parameter, parameter_options);
/// Save name and values of substitution in dictionary.
try {
String param = param_options["param_"].as<std::string>();
size_t pos = param.find('=');
if (pos != String::npos && pos + 1 != param.size())
{
if (!params_substitution.insert({param.substr(0, pos), param.substr(pos + 1)}).second)
throw Exception("Expected various names of parameter field --param_{name}={value}", ErrorCodes::BAD_ARGUMENTS);
} else
throw Exception("Expected parameter field as --param_{name}={value}", ErrorCodes::BAD_ARGUMENTS);
}
catch (const Exception & e)
String parameter = parameter_options["param_"].as<std::string>();
size_t pos = parameter.find('=');
if (pos != String::npos && pos + 1 != parameter.size())
{
std::string text = e.displayText();
std::cerr << "Code: " << e.code() << ". " << text << std::endl;
exit(e.code());
const String name = parameter.substr(0, pos);
if (!parameters_substitution.insert({name, parameter.substr(pos + 1)}).second)
throw Exception("Duplicate name " + name + " of query parameter", ErrorCodes::BAD_ARGUMENTS);
}
else
throw Exception("Expected parameter field as --param_{name}={value}", ErrorCodes::BAD_ARGUMENTS);
}
/// Parse main commandline options.
@ -1758,6 +1752,7 @@ public:
|| (options.count("host") && options["host"].as<std::string>() == "elp")) /// If user writes -help instead of --help.
{
std::cout << main_description << "\n";
std::cout << parameter_description << "\n";
std::cout << external_description << "\n";
exit(0);
}

View File

@ -514,8 +514,8 @@ void HTTPHandler::processQuery(
else if (startsWith(it->first, "param_"))
{
/// Save name and values of substitution in dictionary.
String param_name = it->first.substr(strlen("param_"));
context.setParamSubstitution(param_name, it->second);
const String parameter_name = it->first.substr(strlen("param_"));
context.setParameterSubstitution(parameter_name, it->second);
}
else
{

View File

@ -1866,25 +1866,25 @@ Context::SampleBlockCache & Context::getSampleBlockCache() const
}
bool Context::checkEmptyParamSubstitution() const
bool Context::hasQueryParameters() const
{
return params_substitution.empty();
return !parameters_substitution.empty();
}
void Context::setParamSubstitution(const String & name, const String & value)
NameToNameMap Context::getParameterSubstitution() const
{
if (hasQueryParameters())
return parameters_substitution;
throw Exception("Query without parameters", ErrorCodes::LOGICAL_ERROR);
}
void Context::setParameterSubstitution(const String & name, const String & value)
{
auto lock = getLock();
if (!params_substitution.insert({name, value}).second)
throw Exception("Expected various names of parameter field --param_{name}={value}", ErrorCodes::BAD_ARGUMENTS);
}
NameToNameMap Context::getParamSubstitution() const
{
if (!params_substitution.empty())
return params_substitution;
throw Exception("Context haven't query parameters", ErrorCodes::LOGICAL_ERROR);
if (!parameters_substitution.insert({name, value}).second)
throw Exception("Duplicate name " + name + " of query parameter", ErrorCodes::BAD_ARGUMENTS);
}

View File

@ -145,7 +145,7 @@ private:
using DatabasePtr = std::shared_ptr<IDatabase>;
using Databases = std::map<String, std::shared_ptr<IDatabase>>;
NameToNameMap params_substitution; /// Dictionary with query parameters for prepared statements.
NameToNameMap parameters_substitution; /// Dictionary with query parameters for prepared statements.
/// (key=name, value)
IHostContextPtr host_context; /// Arbitrary object that may used to attach some host specific information to query context,
@ -471,9 +471,9 @@ public:
SampleBlockCache & getSampleBlockCache() const;
/// Query parameters for prepared statements.
bool checkEmptyParamSubstitution() const;
NameToNameMap getParamSubstitution() const;
void setParamSubstitution(const String & name, const String & value);
bool hasQueryParameters() const;
NameToNameMap getParameterSubstitution() const;
void setParameterSubstitution(const String & name, const String & value);
#if USE_EMBEDDED_COMPILER
std::shared_ptr<CompiledExpressionCache> getCompiledExpressionCache() const;

View File

@ -12,13 +12,6 @@
namespace DB
{
namespace ErrorCodes
{
extern const int UNKNOWN_IDENTIFIER;
extern const int LOGICAL_ERROR;
extern const int ILLEGAL_TYPE_OF_ARGUMENT;
}
void ReplaceQueryParameterVisitor::visit(ASTPtr & ast)
{
for (auto & child : ast->children)
@ -32,11 +25,11 @@ void ReplaceQueryParameterVisitor::visit(ASTPtr & ast)
String ReplaceQueryParameterVisitor::getParamValue(const String & name)
{
auto search = params_substitution.find(name);
if (search != params_substitution.end())
auto search = parameters_substitution.find(name);
if (search != parameters_substitution.end())
return search->second;
else
throw Exception("Expected same names in parameter field --param_{name}={value} and in query {name:type}", ErrorCodes::BAD_ARGUMENTS);
throw Exception("Expected name " + name + " in argument --param_{name}", ErrorCodes::BAD_ARGUMENTS);
}
void ReplaceQueryParameterVisitor::visitQP(ASTPtr & ast)
@ -52,7 +45,7 @@ void ReplaceQueryParameterVisitor::visitQP(ASTPtr & ast)
data_type->deserializeAsWholeText(temp_column, read_buffer, format_settings);
Field field = temp_column[0];
ast = std::make_shared<ASTLiteral>(field);
ast = std::make_shared<ASTLiteral>(std::move(field));
}
}

View File

@ -12,16 +12,16 @@ class ASTQueryParameter;
class ReplaceQueryParameterVisitor
{
public:
ReplaceQueryParameterVisitor(const NameToNameMap & params)
: params_substitution(params)
ReplaceQueryParameterVisitor(const NameToNameMap & parameters)
: parameters_substitution(parameters)
{}
void visit(ASTPtr & ast);
private:
const NameToNameMap params_substitution;
void visitQP(ASTPtr & ast);
const NameToNameMap parameters_substitution;
String getParamValue(const String & name);
void visitQP(ASTPtr & ast);
};
}

View File

@ -170,10 +170,10 @@ static std::tuple<ASTPtr, BlockIO> executeQueryImpl(
/// TODO Parser should fail early when max_query_size limit is reached.
ast = parseQuery(parser, begin, end, "", max_query_size);
if (!context.checkEmptyParamSubstitution()) /// Avoid change from TCPHandler.
if (context.hasQueryParameters()) /// Avoid change from TCPHandler.
{
/// Replace ASTQueryParameter with ASTLiteral for prepared statements.
ReplaceQueryParameterVisitor visitor(context.getParamSubstitution());
ReplaceQueryParameterVisitor visitor(context.getParameterSubstitution());
visitor.visit(ast);
}

View File

@ -7,7 +7,7 @@ namespace DB
void ASTQueryParameter::formatImplWithoutAlias(const FormatSettings & settings, FormatState &, FormatStateStacked) const
{
String name_type = name + type;
String name_type = name + ':' + type;
settings.ostr << name_type;
}

View File

@ -6,16 +6,18 @@
namespace DB
{
/// Query parameter: name and type.
/// Parameter in query with name and type of substitution ({name:type}).
/// Example: SELECT * FROM table WHERE id = {pid:UInt16}.
class ASTQueryParameter : public ASTWithAlias
{
public:
String name, type;
String name;
String type;
ASTQueryParameter(const String & name_, const String & type_) : name(name_), type(type_) {}
/** Get the text that identifies this element. */
String getID(char delim) const override { return "QueryParameter" + (delim + name + delim + type); }
String getID(char delim) const override { return "QueryParameter" + (delim + name + ':' + type); }
ASTPtr clone() const override { return std::make_shared<ASTQueryParameter>(*this); }

View File

@ -1200,16 +1200,23 @@ bool ParserQualifiedAsterisk::parseImpl(Pos & pos, ASTPtr & node, Expected & exp
}
bool ParserSubstitutionExpression::parseImpl(Pos & pos, ASTPtr & node, Expected & expected)
bool ParserSubstitution::parseImpl(Pos & pos, ASTPtr & node, Expected & expected)
{
if (pos->type != TokenType::OpeningFiguredBracket)
if (pos->type != TokenType::OpeningCurlyBrace)
return false;
auto old_pos = ++pos;
String s_name, s_type;
String name;
String type;
++pos;
while (pos.isValid() && pos->type != TokenType::Colon)
++pos;
if (pos->type != TokenType::BareWord)
{
expected.add(pos, "string literal");
return false;
}
name = String(pos->begin, pos->end);
++pos;
if (pos->type != TokenType::Colon)
{
@ -1217,21 +1224,25 @@ bool ParserSubstitutionExpression::parseImpl(Pos & pos, ASTPtr & node, Expected
return false;
}
s_name = String(old_pos->begin, pos->begin);
old_pos = ++pos;
++pos;
while (pos.isValid() && pos->type != TokenType::ClosingFiguredBracket)
++pos;
if (pos->type != TokenType::ClosingFiguredBracket)
if (pos->type != TokenType::BareWord)
{
expected.add(pos, "closing figured bracket");
expected.add(pos, "string literal");
return false;
}
s_type = String(old_pos->begin, pos->begin);
type = String(pos->begin, pos->end);
++pos;
node = std::make_shared<ASTQueryParameter>(s_name, s_type);
if (pos->type != TokenType::ClosingCurlyBrace)
{
expected.add(pos, "closing curly brace");
return false;
}
++pos;
node = std::make_shared<ASTQueryParameter>(name, type);
return true;
}
@ -1256,7 +1267,7 @@ bool ParserExpressionElement::parseImpl(Pos & pos, ASTPtr & node, Expected & exp
|| ParserQualifiedAsterisk().parse(pos, node, expected)
|| ParserAsterisk().parse(pos, node, expected)
|| ParserCompoundIdentifier().parse(pos, node, expected)
|| ParserSubstitutionExpression().parse(pos, node, expected);
|| ParserSubstitution().parse(pos, node, expected);
}

View File

@ -242,10 +242,10 @@ private:
};
/** A substitution expression.
/** Prepared statements.
* Parse query with parameter expression {name:type}.
*/
class ParserSubstitutionExpression : public IParserBase
class ParserSubstitution : public IParserBase
{
protected:
const char * getName() const { return "substitution"; }

View File

@ -174,9 +174,9 @@ Token Lexer::nextTokenImpl()
case ']':
return Token(TokenType::ClosingSquareBracket, token_begin, ++pos);
case '{':
return Token(TokenType::OpeningFiguredBracket, token_begin, ++pos);
return Token(TokenType::OpeningCurlyBrace, token_begin, ++pos);
case '}':
return Token(TokenType::ClosingFiguredBracket, token_begin, ++pos);
return Token(TokenType::ClosingCurlyBrace, token_begin, ++pos);
case ',':
return Token(TokenType::Comma, token_begin, ++pos);
case ';':

View File

@ -23,8 +23,8 @@ namespace DB
M(OpeningSquareBracket) \
M(ClosingSquareBracket) \
\
M(OpeningFiguredBracket) \
M(ClosingFiguredBracket) \
M(OpeningCurlyBrace) \
M(ClosingCurlyBrace) \
\
M(Comma) \
M(Semicolon) \

View File

@ -28,6 +28,8 @@ std::map<TokenType, const char *> hilite =
{TokenType::ClosingRoundBracket, "\033[1;33m"},
{TokenType::OpeningSquareBracket, "\033[1;33m"},
{TokenType::ClosingSquareBracket, "\033[1;33m"},
{TokenType::OpeningCurlyBrace, "\033[1;33m"},
{TokenType::ClosingCurlyBrace, "\033[1;33m"},
{TokenType::Comma, "\033[1;33m"},
{TokenType::Semicolon, "\033[1;33m"},
@ -76,6 +78,7 @@ int main(int, char **)
if (token.isEnd())
break;
writeChar(' ', out);
auto it = hilite.find(token.type);

View File

@ -0,0 +1,3 @@
1 Hello, world
1 Hello, world
2 test

View File

@ -0,0 +1,19 @@
#!/usr/bin/env bash
CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
. $CURDIR/../shell_config.sh
$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS ps";
$CLICKHOUSE_CLIENT -q "CREATE TABLE ps (i UInt8, s String) ENGINE = Memory";
$CLICKHOUSE_CLIENT -q "INSERT INTO ps VALUES (1, 'Hello, world')";
$CLICKHOUSE_CLIENT -q "INSERT INTO ps VALUES (2, 'test')";
$CLICKHOUSE_CLIENT --max_threads=1 --param_id=1\
-q "SELECT * FROM ps WHERE i = {id:UInt8}";
$CLICKHOUSE_CLIENT --max_threads=1 --param_phrase='Hello, world'\
-q "SELECT * FROM ps WHERE s = {phrase:String}";
$CLICKHOUSE_CLIENT --max_threads=1 --param_id=2 --param_phrase='test'\
-q "SELECT * FROM ps WHERE i = {id:UInt8} and s = {phrase:String}";
$CLICKHOUSE_CLIENT -q "DROP TABLE ps";

View File

@ -0,0 +1,3 @@
1 Hello, world
1 Hello, world
2 test

View File

@ -0,0 +1,19 @@
#!/usr/bin/env bash
CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
. $CURDIR/../shell_config.sh
${CLICKHOUSE_CURL} -sS $CLICKHOUSE_URL -d "DROP TABLE IF EXISTS ps";
${CLICKHOUSE_CURL} -sS $CLICKHOUSE_URL -d "CREATE TABLE ps (i UInt8, s String) ENGINE = Memory";
${CLICKHOUSE_CURL} -sS $CLICKHOUSE_URL -d "INSERT INTO ps VALUES (1, 'Hello, world')";
${CLICKHOUSE_CURL} -sS $CLICKHOUSE_URL -d "INSERT INTO ps VALUES (2, 'test')";
${CLICKHOUSE_CURL} "${CLICKHOUSE_URL}?param_id=1"\
-d "SELECT * FROM ps WHERE i = {id:UInt8}";
${CLICKHOUSE_CURL} "${CLICKHOUSE_URL}?param_phrase=Hello,+world"\
-d "SELECT * FROM ps WHERE s = {phrase:String}";
${CLICKHOUSE_CURL} "${CLICKHOUSE_URL}?param_id=2&param_phrase=test"\
-d "SELECT * FROM ps WHERE i = {id:UInt8} and s = {phrase:String}";
${CLICKHOUSE_CURL} -sS $CLICKHOUSE_URL -d "DROP TABLE ps";