From 3f67e320dd3002bb7739fcac5db02ca2e652ade9 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Wed, 25 Nov 2020 21:32:52 +0300 Subject: [PATCH] Fix parsing of SETTINGS clause of the INSERT ... SELECT ... SETTINGS query Before this patch the following query ignores the settings for INSERT: insert into test_parallel_insert select * from numbers_mt(65535*2) settings max_insert_threads=10 And the reason is that SETTINGS was parsed by the SELECT parser. Fix this by push down the SETTINGS from the SELECT to INSERT. Also note that since INSERT parser does not use ParserQueryWithOutput the following works: insert into test_parallel_insert select * from numbers_mt(65535*2) format Null settings max_insert_threads=10 --- .../InsertQuerySettingsPushDownVisitor.cpp | 64 +++++++++++++++++++ .../InsertQuerySettingsPushDownVisitor.h | 37 +++++++++++ src/Parsers/ParserInsertQuery.cpp | 9 +++ src/Parsers/ya.make | 1 + .../01593_insert_settings.reference | 0 .../0_stateless/01593_insert_settings.sql | 8 +++ 6 files changed, 119 insertions(+) create mode 100644 src/Parsers/InsertQuerySettingsPushDownVisitor.cpp create mode 100644 src/Parsers/InsertQuerySettingsPushDownVisitor.h create mode 100644 tests/queries/0_stateless/01593_insert_settings.reference create mode 100644 tests/queries/0_stateless/01593_insert_settings.sql diff --git a/src/Parsers/InsertQuerySettingsPushDownVisitor.cpp b/src/Parsers/InsertQuerySettingsPushDownVisitor.cpp new file mode 100644 index 00000000000..a3bca76816f --- /dev/null +++ b/src/Parsers/InsertQuerySettingsPushDownVisitor.cpp @@ -0,0 +1,64 @@ +#include +#include +#include +#include +#include +#include + +#include +#include + +namespace DB +{ + +bool InsertQuerySettingsPushDownMatcher::needChildVisit(ASTPtr & node, const ASTPtr & child) +{ + if (node->as()) + return true; + if (node->as()) + return true; + if (child->as()) + return true; + return false; +} + +void InsertQuerySettingsPushDownMatcher::visit(ASTPtr & ast, Data & data) +{ + if (auto * select_query = ast->as()) + visit(*select_query, ast, data); +} + +void InsertQuerySettingsPushDownMatcher::visit(ASTSelectQuery & select_query, ASTPtr &, Data & data) +{ + ASTPtr select_settings_ast = select_query.settings(); + if (!select_settings_ast) + return; + + auto & insert_settings_ast = data.insert_settings_ast; + + if (!insert_settings_ast) + { + insert_settings_ast = select_settings_ast->clone(); + return; + } + + SettingsChanges & select_settings = select_settings_ast->as().changes; + SettingsChanges & insert_settings = insert_settings_ast->as().changes; + + for (auto & setting : select_settings) + { + auto it = std::find_if(insert_settings.begin(), insert_settings.end(), [&](auto & select_setting) + { + return select_setting.name == setting.name; + }); + if (it == insert_settings.end()) + insert_settings.push_back(setting); + else + { + /// Do not ovewrite setting that was passed for INSERT + /// by settings that was passed for SELECT + } + } +} + +} diff --git a/src/Parsers/InsertQuerySettingsPushDownVisitor.h b/src/Parsers/InsertQuerySettingsPushDownVisitor.h new file mode 100644 index 00000000000..1590a9198b8 --- /dev/null +++ b/src/Parsers/InsertQuerySettingsPushDownVisitor.h @@ -0,0 +1,37 @@ +#pragma once + +#include +#include + +namespace DB +{ + +class ASTSelectQuery; +struct SettingChange; +class SettingsChanges; + +/// Pushdown SETTINGS clause to the INSERT from the SELECT query: +/// (since SETTINGS after SELECT will be parsed by the SELECT parser.) +/// +/// NOTE: INSERT ... SELECT ... FORMAT Null SETTINGS max_insert_threads=10 works even w/o push down, +/// since ParserInsertQuery does not ParserInsertQuery. +class InsertQuerySettingsPushDownMatcher +{ +public: + using Visitor = InDepthNodeVisitor; + + struct Data + { + ASTPtr & insert_settings_ast; + }; + + static bool needChildVisit(ASTPtr & node, const ASTPtr & child); + static void visit(ASTPtr & ast, Data & data); + +private: + static void visit(ASTSelectQuery &, ASTPtr &, Data &); +}; + +using InsertQuerySettingsPushDownVisitor = InsertQuerySettingsPushDownMatcher::Visitor; + +} diff --git a/src/Parsers/ParserInsertQuery.cpp b/src/Parsers/ParserInsertQuery.cpp index 50baf7566d1..1f987edf13f 100644 --- a/src/Parsers/ParserInsertQuery.cpp +++ b/src/Parsers/ParserInsertQuery.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include @@ -127,6 +128,14 @@ bool ParserInsertQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) return false; } + if (select) + { + /// Copy SETTINGS from the INSERT ... SELECT ... SETTINGS + InsertQuerySettingsPushDownVisitor::Data visitor_data{settings_ast}; + InsertQuerySettingsPushDownVisitor(visitor_data).visit(select); + } + + if (format) { Pos last_token = pos; diff --git a/src/Parsers/ya.make b/src/Parsers/ya.make index 42c5719f60d..3305da3a4d0 100644 --- a/src/Parsers/ya.make +++ b/src/Parsers/ya.make @@ -67,6 +67,7 @@ SRCS( ExpressionListParsers.cpp IAST.cpp IParserBase.cpp + InsertQuerySettingsPushDownVisitor.cpp Lexer.cpp MySQL/ASTAlterCommand.cpp MySQL/ASTAlterQuery.cpp diff --git a/tests/queries/0_stateless/01593_insert_settings.reference b/tests/queries/0_stateless/01593_insert_settings.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/01593_insert_settings.sql b/tests/queries/0_stateless/01593_insert_settings.sql new file mode 100644 index 00000000000..626607a4295 --- /dev/null +++ b/tests/queries/0_stateless/01593_insert_settings.sql @@ -0,0 +1,8 @@ +drop table if exists data_01593; +create table data_01593 (key Int) engine=MergeTree() order by key partition by key; + +insert into data_01593 select * from numbers_mt(10); +-- TOO_MANY_PARTS error +insert into data_01593 select * from numbers_mt(10) settings max_partitions_per_insert_block=1; -- { serverError 252 } +-- settings for INSERT is prefered +insert into data_01593 select * from numbers_mt(10) settings max_partitions_per_insert_block=1 settings max_partitions_per_insert_block=100;