From 10e4ef135d8c62ede00b4af9cd0c0711e57ceb22 Mon Sep 17 00:00:00 2001 From: vdimir Date: Mon, 18 Jul 2022 15:18:39 +0200 Subject: [PATCH] Set default value cross_to_inner_join_rewrite = 2 for comma join --- src/Core/Settings.h | 2 +- src/Interpreters/CrossToInnerJoinVisitor.cpp | 20 ++++++++++++++--- ...4_setting_cross_to_inner_rewrite.reference | 7 ++++++ .../02364_setting_cross_to_inner_rewrite.sql | 22 +++++++++++++++++++ 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 tests/queries/0_stateless/02364_setting_cross_to_inner_rewrite.reference create mode 100644 tests/queries/0_stateless/02364_setting_cross_to_inner_rewrite.sql diff --git a/src/Core/Settings.h b/src/Core/Settings.h index bda72f089eb..ea08658d851 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -758,7 +758,7 @@ static constexpr UInt64 operator""_GiB(unsigned long long value) M(Bool, output_format_pretty_row_numbers, false, "Add row numbers before each row for pretty output format", 0) \ M(Bool, insert_distributed_one_random_shard, false, "If setting is enabled, inserting into distributed table will choose a random shard to write when there is no sharding key", 0) \ \ - M(UInt64, cross_to_inner_join_rewrite, 1, "Use inner join instead of comma/cross join if possible. Possible values: 0 - no rewrite, 1 - apply if possible, 2 - force rewrite all cross joins", 0) \ + M(UInt64, cross_to_inner_join_rewrite, 2, "Use inner join instead of comma/cross join if possible. Possible values: 0 - no rewrite, 1 - apply if possible for comma/cross, 2 - force rewrite all comma joins, cross - if possible", 0) \ \ M(Bool, output_format_arrow_low_cardinality_as_dictionary, false, "Enable output LowCardinality type as Dictionary Arrow type", 0) \ M(Bool, output_format_arrow_string_as_string, false, "Use Arrow String type instead of Binary for String columns", 0) \ diff --git a/src/Interpreters/CrossToInnerJoinVisitor.cpp b/src/Interpreters/CrossToInnerJoinVisitor.cpp index d438ea9394e..be6c1101fb4 100644 --- a/src/Interpreters/CrossToInnerJoinVisitor.cpp +++ b/src/Interpreters/CrossToInnerJoinVisitor.cpp @@ -39,7 +39,10 @@ struct JoinedElement : element(table_element) { if (element.table_join) + { join = element.table_join->as(); + original_kind = join->kind; + } } void checkTableName(const DatabaseAndTableWithAlias & table, const String & current_database) const @@ -61,6 +64,8 @@ struct JoinedElement join->kind = ASTTableJoin::Kind::Cross; } + ASTTableJoin::Kind getOriginalKind() const { return original_kind; } + bool rewriteCrossToInner(ASTPtr on_expression) { if (join->kind != ASTTableJoin::Kind::Cross) @@ -83,6 +88,8 @@ struct JoinedElement private: const ASTTablesInSelectQueryElement & element; ASTTableJoin * join = nullptr; + + ASTTableJoin::Kind original_kind; }; bool isAllowedToRewriteCrossJoin(const ASTPtr & node, const Aliases & aliases) @@ -251,10 +258,17 @@ void CrossToInnerJoinMatcher::visit(ASTSelectQuery & select, ASTPtr &, Data & da } } - if (data.cross_to_inner_join_rewrite > 1 && !rewritten) + if (joined.getOriginalKind() == ASTTableJoin::Kind::Comma && + data.cross_to_inner_join_rewrite > 1 && + !rewritten) { - throw Exception(ErrorCodes::INCORRECT_QUERY, "Failed to rewrite '{} WHERE {}' to INNER JOIN", - query_before, queryToString(select.where())); + throw Exception( + ErrorCodes::INCORRECT_QUERY, + "Failed to rewrite comma join to INNER. " + "Please, try to simplify WHERE section " + "or set the setting `cross_to_inner_join_rewrite` to 1 to allow slow CROSS JOIN for this case" + "(cannot rewrite '{} WHERE {}' to INNER JOIN)", + query_before, queryToString(select.where())); } } } diff --git a/tests/queries/0_stateless/02364_setting_cross_to_inner_rewrite.reference b/tests/queries/0_stateless/02364_setting_cross_to_inner_rewrite.reference new file mode 100644 index 00000000000..fcb49fa9945 --- /dev/null +++ b/tests/queries/0_stateless/02364_setting_cross_to_inner_rewrite.reference @@ -0,0 +1,7 @@ +1 +1 +1 +1 +1 +1 +1 diff --git a/tests/queries/0_stateless/02364_setting_cross_to_inner_rewrite.sql b/tests/queries/0_stateless/02364_setting_cross_to_inner_rewrite.sql new file mode 100644 index 00000000000..8deddbaa037 --- /dev/null +++ b/tests/queries/0_stateless/02364_setting_cross_to_inner_rewrite.sql @@ -0,0 +1,22 @@ + + +DROP TABLE IF EXISTS t1; +DROP TABLE IF EXISTS t2; + +CREATE TABLE t1 ( x Int ) Engine = Memory; +INSERT INTO t1 VALUES ( 1 ), ( 2 ), ( 3 ); + +CREATE TABLE t2 ( x Int ) Engine = Memory; +INSERT INTO t2 VALUES ( 2 ), ( 3 ), ( 4 ); + +SET cross_to_inner_join_rewrite = 1; +SELECT count() = 1 FROM t1, t2 WHERE t1.x > t2.x; +SELECT count() = 1 2ROM t1, t2 WHERE t1.x = t2.x; +SELECT count() = 1 2ROM t1 CROSS JOIN t2 WHERE t1.x = t2.x; +SELECT count() = 1 FROM t1 CROSS JOIN t2 WHERE t1.x > t2.x; + +SET cross_to_inner_join_rewrite = 2; +SELECT count() = 1 FROM t1, t2 WHERE t1.x > t2.x; -- { serverError INCORRECT_QUERY } +SELECT count() = 2 FROM t1, t2 WHERE t1.x = t2.x; +SELECT count() = 2 FROM t1 CROSS JOIN t2 WHERE t1.x = t2.x; +SELECT count() = 1 FROM t1 CROSS JOIN t2 WHERE t1.x > t2.x; -- do not force rewrite explicit CROSS