diff --git a/src/Interpreters/ExpressionAnalyzer.cpp b/src/Interpreters/ExpressionAnalyzer.cpp index 2c77b18aafd..8197c0fa0dd 100644 --- a/src/Interpreters/ExpressionAnalyzer.cpp +++ b/src/Interpreters/ExpressionAnalyzer.cpp @@ -162,8 +162,10 @@ ExpressionAnalyzer::ExpressionAnalyzer( analyzeAggregation(); } -static ASTPtr checkPositionalArgument(ASTPtr argument, const NamesAndTypesList & columns) +static ASTPtr checkPositionalArgument(ASTPtr argument, const ASTSelectQuery * select_query, ASTSelectQuery::Expression expression) { + auto columns = select_query->select()->children; + /// Case when GROUP BY element is position. /// Do not consider case when GROUP BY element is not a literal, but expression, even if all values are constants. if (auto * ast_literal = typeid_cast(argument.get())) @@ -174,9 +176,19 @@ static ASTPtr checkPositionalArgument(ASTPtr argument, const NamesAndTypesList & auto pos = ast_literal->value.get(); if ((0 < pos) && (pos <= columns.size())) { - const auto & column_name = std::next(columns.begin(), pos - 1)->name; - return std::make_shared(column_name); + --pos; + const auto & column = columns[pos]; + if (const auto * literal_ast = typeid_cast(column.get())) + { + return std::make_shared(literal_ast->name()); + } + else + { + throw Exception(ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, "Illegal value for positional argument in {}", + ASTSelectQuery::expressionToString(expression)); + } } + /// Do not throw if out of bounds, see appendUnusedGroupByColumn. } } return nullptr; @@ -257,7 +269,6 @@ void ExpressionAnalyzer::analyzeAggregation() { NameSet unique_keys; ASTs & group_asts = select_query->groupBy()->children; - const auto & columns = syntax->source_columns; for (ssize_t i = 0; i < ssize_t(group_asts.size()); ++i) { @@ -266,7 +277,7 @@ void ExpressionAnalyzer::analyzeAggregation() if (getContext()->getSettingsRef().enable_positional_arguments) { - auto new_argument = checkPositionalArgument(group_asts[i], columns); + auto new_argument = checkPositionalArgument(group_asts[i], select_query, ASTSelectQuery::Expression::GROUP_BY); if (new_argument) group_asts[i] = new_argument; } @@ -1252,7 +1263,6 @@ ActionsDAGPtr SelectQueryExpressionAnalyzer::appendOrderBy(ExpressionActionsChai bool with_fill = false; NameSet order_by_keys; - const auto & columns = syntax->source_columns; for (auto & child : select_query->orderBy()->children) { @@ -1262,7 +1272,7 @@ ActionsDAGPtr SelectQueryExpressionAnalyzer::appendOrderBy(ExpressionActionsChai if (getContext()->getSettingsRef().enable_positional_arguments) { - auto new_argument = checkPositionalArgument(ast->children.at(0), columns); + auto new_argument = checkPositionalArgument(ast->children.at(0), select_query, ASTSelectQuery::Expression::ORDER_BY); if (new_argument) ast->children[0] = new_argument; } @@ -1317,13 +1327,12 @@ bool SelectQueryExpressionAnalyzer::appendLimitBy(ExpressionActionsChain & chain } auto & children = select_query->limitBy()->children; - const auto & columns = syntax->source_columns; for (size_t i = 0; i < children.size(); ++i) { if (getContext()->getSettingsRef().enable_positional_arguments) { - auto new_argument = checkPositionalArgument(children[i], columns); + auto new_argument = checkPositionalArgument(children[i], select_query, ASTSelectQuery::Expression::LIMIT_BY); if (new_argument) children[i] = new_argument; } diff --git a/src/Parsers/ASTSelectQuery.h b/src/Parsers/ASTSelectQuery.h index 12817199d13..e439c5edaa5 100644 --- a/src/Parsers/ASTSelectQuery.h +++ b/src/Parsers/ASTSelectQuery.h @@ -35,6 +35,43 @@ public: SETTINGS }; + static String expressionToString(Expression expr) + { + switch (expr) + { + case Expression::WITH: + return "WITH"; + case Expression::SELECT: + return "SELECT"; + case Expression::TABLES: + return "TABLES"; + case Expression::PREWHERE: + return "PREWHERE"; + case Expression::WHERE: + return "WHERE"; + case Expression::GROUP_BY: + return "GROUP BY"; + case Expression::HAVING: + return "HAVING"; + case Expression::WINDOW: + return "WINDOW"; + case Expression::ORDER_BY: + return "ORDER BY"; + case Expression::LIMIT_BY_OFFSET: + return "LIMIT BY OFFSET"; + case Expression::LIMIT_BY_LENGTH: + return "LIMIT BY LENGTH"; + case Expression::LIMIT_BY: + return "LIMIT BY"; + case Expression::LIMIT_OFFSET: + return "LIMIT OFFSET"; + case Expression::LIMIT_LENGTH: + return "LIMIT LENGTH"; + case Expression::SETTINGS: + return "SETTINGS"; + } + } + /** Get the text that identifies this element. */ String getID(char) const override { return "SelectQuery"; } diff --git a/tests/queries/0_stateless/02006_test_positional_arguments.reference b/tests/queries/0_stateless/02006_test_positional_arguments.reference index e497af0918a..a8e8ccec100 100644 --- a/tests/queries/0_stateless/02006_test_positional_arguments.reference +++ b/tests/queries/0_stateless/02006_test_positional_arguments.reference @@ -1,90 +1,50 @@ -- { echo } -set enable_positional_arguments = 1; -drop table if exists test; -create table test (col1 Int32, col2 Int32, col3 Int32) engine = Memory(); -insert into test select number, number, 5 from numbers(2); -insert into test select number, number, 4 from numbers(2); -insert into test select number, number, 3 from numbers(2); -insert into test select number, number, 2 from numbers(2); -insert into test select number, number, 1 from numbers(2); -select * from test where col1 = 1 order by 3 desc; -1 1 5 -1 1 4 -1 1 3 -1 1 2 -1 1 1 -select * from test where col2 = 1 order by 3 asc; -1 1 1 -1 1 2 -1 1 3 -1 1 4 -1 1 5 -insert into test select number, number+1, 1 from numbers(2); -insert into test select number, number+1, 2 from numbers(2); -insert into test select number, number+1, 3 from numbers(2); -insert into test select number, number+1, 4 from numbers(2); -insert into test select number, number+1, 5 from numbers(2); -select * from test order by col1, col2, col3 asc limit 2 by col2; -0 0 1 -0 0 2 -0 1 1 -0 1 2 -1 2 1 -1 2 2 -select * from test order by 1, 2, 3 asc limit 2 by 2; -0 0 1 -0 0 2 -0 1 1 -0 1 2 -1 2 1 -1 2 2 -select col1, col2 from test group by col1, col2 order by col1, col2; -0 0 -0 1 -1 1 -1 2 -select col1, col2 from test group by 1, 2 order by 1, 2; -0 0 -0 1 -1 1 -1 2 -select col2, col3 from test group by col3, col2 order by col3, col2; -0 1 -1 1 -2 1 -0 2 -1 2 -2 2 -0 3 -1 3 -2 3 -0 4 -1 4 -2 4 -0 5 -1 5 -2 5 -select col2, col3 from test group by 3, 2 order by 3, 2; -0 1 -1 1 -2 1 -0 2 -1 2 -2 2 -0 3 -1 3 -2 3 -0 4 -1 4 -2 4 -0 5 -1 5 -2 5 -select col2 from test group by 2 order by 2; -0 -1 -2 -select col2 + 100 from test group by 2 order by 2; -100 -101 -102 +select x3, x2, x1 from test order by 1; +1 100 100 +10 1 10 +100 10 1 +select x3, x2, x1 from test order by x3; +1 100 100 +10 1 10 +100 10 1 +select x3, x2, x1 from test order by 1 desc; +100 10 1 +10 1 10 +1 100 100 +select x3, x2, x1 from test order by x3 desc; +100 10 1 +10 1 10 +1 100 100 +insert into test values (1, 10, 200), (10, 1, 200), (100, 100, 1); +select x3, x2 from test group by x3, x2; +200 1 +10 1 +200 10 +1 100 +100 10 +select x3, x2 from test group by 1, 2; +200 1 +10 1 +200 10 +1 100 +100 10 +select x1, x2, x3 from test order by x3 limit 1 by x3; +100 100 1 +10 1 10 +1 10 100 +1 10 200 +select x1, x2, x3 from test order by 3 limit 1 by 3; +100 100 1 +10 1 10 +1 10 100 +1 10 200 +select x1, x2, x3 from test order by x3 limit 1 by x1; +100 100 1 +10 1 10 +1 10 100 +select x1, x2, x3 from test order by 3 limit 1 by 1; +100 100 1 +10 1 10 +1 10 100 +select max(x3), max(x2), max(x1) from test group by 1; -- { serverError 43 } +select max(x1) from test order by 1; -- { serverError 43 } diff --git a/tests/queries/0_stateless/02006_test_positional_arguments.sql b/tests/queries/0_stateless/02006_test_positional_arguments.sql index bbfd1dbfd64..dc45b288016 100644 --- a/tests/queries/0_stateless/02006_test_positional_arguments.sql +++ b/tests/queries/0_stateless/02006_test_positional_arguments.sql @@ -1,32 +1,26 @@ --- { echo } set enable_positional_arguments = 1; drop table if exists test; -create table test (col1 Int32, col2 Int32, col3 Int32) engine = Memory(); +create table test(x1 Int, x2 Int, x3 Int) engine=Memory(); +insert into test values (1, 10, 100), (10, 1, 10), (100, 100, 1); -insert into test select number, number, 5 from numbers(2); -insert into test select number, number, 4 from numbers(2); -insert into test select number, number, 3 from numbers(2); -insert into test select number, number, 2 from numbers(2); -insert into test select number, number, 1 from numbers(2); +-- { echo } +select x3, x2, x1 from test order by 1; +select x3, x2, x1 from test order by x3; -select * from test where col1 = 1 order by 3 desc; -select * from test where col2 = 1 order by 3 asc; +select x3, x2, x1 from test order by 1 desc; +select x3, x2, x1 from test order by x3 desc; -insert into test select number, number+1, 1 from numbers(2); -insert into test select number, number+1, 2 from numbers(2); -insert into test select number, number+1, 3 from numbers(2); -insert into test select number, number+1, 4 from numbers(2); -insert into test select number, number+1, 5 from numbers(2); +insert into test values (1, 10, 200), (10, 1, 200), (100, 100, 1); +select x3, x2 from test group by x3, x2; +select x3, x2 from test group by 1, 2; -select * from test order by col1, col2, col3 asc limit 2 by col2; -select * from test order by 1, 2, 3 asc limit 2 by 2; +select x1, x2, x3 from test order by x3 limit 1 by x3; +select x1, x2, x3 from test order by 3 limit 1 by 3; +select x1, x2, x3 from test order by x3 limit 1 by x1; +select x1, x2, x3 from test order by 3 limit 1 by 1; -select col1, col2 from test group by col1, col2 order by col1, col2; -select col1, col2 from test group by 1, 2 order by 1, 2; +select max(x3), max(x2), max(x1) from test group by 1; -- { serverError 43 } +select max(x1) from test order by 1; -- { serverError 43 } -select col2, col3 from test group by col3, col2 order by col3, col2; -select col2, col3 from test group by 3, 2 order by 3, 2; -select col2 from test group by 2 order by 2; -select col2 + 100 from test group by 2 order by 2;