From 0a56ed910bbb5406ade0247e26ae21d0297d904d Mon Sep 17 00:00:00 2001 From: velioglu Date: Wed, 16 Aug 2017 11:58:04 +0300 Subject: [PATCH] Change error message of queries with distributed and local table Citus can handle INSERT INTO ... SELECT queries if the query inserts into local table by reading data from distributed table. The opposite way is not correct. With this commit we warn the user if the latter option is used. --- .../planner/insert_select_planner.c | 57 ++++++++++++++++--- .../planner/multi_logical_planner.c | 5 ++ .../distributed/insert_select_planner.h | 1 + .../isolation_insert_select_vs_all.out | 2 +- ...lti_insert_select_non_pushable_queries.out | 10 ++++ ...lti_insert_select_non_pushable_queries.sql | 12 ++++ 6 files changed, 77 insertions(+), 10 deletions(-) diff --git a/src/backend/distributed/planner/insert_select_planner.c b/src/backend/distributed/planner/insert_select_planner.c index 896163784..7f69922ec 100644 --- a/src/backend/distributed/planner/insert_select_planner.c +++ b/src/backend/distributed/planner/insert_select_planner.c @@ -64,6 +64,7 @@ static DeferredErrorMessage * CoordinatorInsertSelectSupported(Query *insertSele static Query * WrapSubquery(Query *subquery); static void CastSelectTargetList(List *selectTargetList, Oid targetRelationId, List *insertTargetList); +static bool CheckInsertSelectQuery(Query *query); /* @@ -73,18 +74,62 @@ static void CastSelectTargetList(List *selectTargetList, Oid targetRelationId, * * Note that the input query should be the original parsetree of * the query (i.e., not passed trough the standard planner). + */ +bool +InsertSelectIntoDistributedTable(Query *query) +{ + bool insertSelectQuery = CheckInsertSelectQuery(query); + + if (insertSelectQuery) + { + RangeTblEntry *insertRte = ExtractInsertRangeTableEntry(query); + if (IsDistributedTable(insertRte->relid)) + { + return true; + } + } + + return false; +} + + +/* + * InsertSelectIntoLocalTable checks whether INSERT INTO ... SELECT inserts + * into local table. Note that query must be a sample of INSERT INTO ... SELECT + * type of query. + */ +bool +InsertSelectIntoLocalTable(Query *query) +{ + bool insertSelectQuery = CheckInsertSelectQuery(query); + + if (insertSelectQuery) + { + RangeTblEntry *insertRte = ExtractInsertRangeTableEntry(query); + if (!IsDistributedTable(insertRte->relid)) + { + return true; + } + } + + return false; +} + + +/* + * CheckInsertSelectQuery returns true when the input query is an INSERT INTO + * ... SELECT kind of query. * * This function is inspired from getInsertSelectQuery() on * rewrite/rewriteManip.c. */ -bool -InsertSelectIntoDistributedTable(Query *query) +static bool +CheckInsertSelectQuery(Query *query) { CmdType commandType = query->commandType; List *fromList = NULL; RangeTblRef *rangeTableReference = NULL; RangeTblEntry *subqueryRte = NULL; - RangeTblEntry *insertRte = NULL; if (commandType != CMD_INSERT) { @@ -117,12 +162,6 @@ InsertSelectIntoDistributedTable(Query *query) /* ensure that there is a query */ Assert(IsA(subqueryRte->subquery, Query)); - insertRte = ExtractInsertRangeTableEntry(query); - if (!IsDistributedTable(insertRte->relid)) - { - return false; - } - return true; } diff --git a/src/backend/distributed/planner/multi_logical_planner.c b/src/backend/distributed/planner/multi_logical_planner.c index f62c94491..b414ddc3f 100644 --- a/src/backend/distributed/planner/multi_logical_planner.c +++ b/src/backend/distributed/planner/multi_logical_planner.c @@ -2976,6 +2976,11 @@ NeedsDistributedPlanning(Query *queryTree) if (hasLocalRelation && hasDistributedRelation) { + if (InsertSelectIntoLocalTable(queryTree)) + { + ereport(ERROR, (errmsg("cannot INSERT rows from a distributed query into a " + "local table"))); + } ereport(ERROR, (errmsg("cannot plan queries which include both local and " "distributed relations"))); } diff --git a/src/include/distributed/insert_select_planner.h b/src/include/distributed/insert_select_planner.h index e89372f81..f1a1362b5 100644 --- a/src/include/distributed/insert_select_planner.h +++ b/src/include/distributed/insert_select_planner.h @@ -24,6 +24,7 @@ extern bool InsertSelectIntoDistributedTable(Query *query); +extern bool InsertSelectIntoLocalTable(Query *query); extern Query * ReorderInsertSelectTargetLists(Query *originalQuery, RangeTblEntry *insertRte, RangeTblEntry *subqueryRte); diff --git a/src/test/regress/expected/isolation_insert_select_vs_all.out b/src/test/regress/expected/isolation_insert_select_vs_all.out index 31e80fee8..3e19c6460 100644 --- a/src/test/regress/expected/isolation_insert_select_vs_all.out +++ b/src/test/regress/expected/isolation_insert_select_vs_all.out @@ -336,7 +336,7 @@ step s1-initialize: step s1-begin: BEGIN; step s1-insert-select: INSERT INTO insert_of_insert_select_hash SELECT * FROM select_of_insert_select_hash ORDER BY 1, 2 LIMIT 5;; -ERROR: cannot plan queries which include both local and distributed relations +ERROR: cannot INSERT rows from a distributed query into a local table step s2-distribute-table-on-inserted: SELECT create_distributed_table('insert_of_insert_select_hash', 'id'); create_distributed_table diff --git a/src/test/regress/expected/multi_insert_select_non_pushable_queries.out b/src/test/regress/expected/multi_insert_select_non_pushable_queries.out index d99bf6c20..a1cfaea68 100644 --- a/src/test/regress/expected/multi_insert_select_non_pushable_queries.out +++ b/src/test/regress/expected/multi_insert_select_non_pushable_queries.out @@ -1,5 +1,15 @@ ------------------------------------ ------------------------------------ +-- Insert into local table +------------------------------------ +------------------------------------ +CREATE TABLE test_table_1(id int); +INSERT INTO test_table_1 +SELECT user_id FROM users_table; +ERROR: cannot INSERT rows from a distributed query into a local table +DROP TABLE test_table_1; +------------------------------------ +------------------------------------ -- Vanilla funnel query ------------------------------------ ------------------------------------ diff --git a/src/test/regress/sql/multi_insert_select_non_pushable_queries.sql b/src/test/regress/sql/multi_insert_select_non_pushable_queries.sql index 3982900a8..a4fb4d06b 100644 --- a/src/test/regress/sql/multi_insert_select_non_pushable_queries.sql +++ b/src/test/regress/sql/multi_insert_select_non_pushable_queries.sql @@ -1,3 +1,15 @@ +------------------------------------ +------------------------------------ +-- Insert into local table +------------------------------------ +------------------------------------ +CREATE TABLE test_table_1(id int); + +INSERT INTO test_table_1 +SELECT user_id FROM users_table; + +DROP TABLE test_table_1; + ------------------------------------ ------------------------------------ -- Vanilla funnel query