From 6f06ff78cc32cf67225d9d55e0679ce6e084c979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emel=20=C5=9Eim=C5=9Fek?= Date: Tue, 6 Sep 2022 16:46:41 +0300 Subject: [PATCH] Throw an error if there is a RangeTblEntry that is not assigned an RTE identity. (#6295) * Fix issue : 6109 Segfault or (assertion failure) is possible when using a SQL function * DESCRIPTION: Ensures disallowing the usage of SQL functions referencing to a distributed table and prevents a segfault. Using a SQL function may result in segmentation fault in some cases. This change fixes the issue by throwing an error message when a SQL function cannot be handled. Fixes #6109. * DESCRIPTION: Ensures disallowing the usage of SQL functions referencing to a distributed table and prevents a segfault. Using a SQL function may result in segmentation fault in some cases. This change fixes the issue by throwing an error message when a SQL function cannot be handled. Fixes #6109. Co-authored-by: Emel Simsek --- .../distributed/planner/distributed_planner.c | 21 +++++++++++++++++-- .../regress/expected/multi_sql_function.out | 3 +++ src/test/regress/sql/multi_sql_function.sql | 3 +++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/backend/distributed/planner/distributed_planner.c b/src/backend/distributed/planner/distributed_planner.c index 17816a3b4..3f431f514 100644 --- a/src/backend/distributed/planner/distributed_planner.c +++ b/src/backend/distributed/planner/distributed_planner.c @@ -559,9 +559,26 @@ int GetRTEIdentity(RangeTblEntry *rte) { Assert(rte->rtekind == RTE_RELATION); - Assert(rte->values_lists != NIL); + + /* + * Since SQL functions might be in-lined by standard_planner, + * we might miss assigning an RTE identity for RangeTblEntries + * related to SQL functions. We already have checks in other + * places to throw an error for SQL functions but they are not + * sufficient due to function in-lining; so here we capture such + * cases and throw an error here. + */ + if (list_length(rte->values_lists) != 2) + { + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot perform distributed planning on this " + "query because parameterized queries for SQL " + "functions referencing distributed tables are " + "not supported"), + errhint("Consider using PL/pgSQL functions instead."))); + } + Assert(IsA(rte->values_lists, IntList)); - Assert(list_length(rte->values_lists) == 2); return linitial_int(rte->values_lists); } diff --git a/src/test/regress/expected/multi_sql_function.out b/src/test/regress/expected/multi_sql_function.out index e1862c983..bcc4efcca 100644 --- a/src/test/regress/expected/multi_sql_function.out +++ b/src/test/regress/expected/multi_sql_function.out @@ -320,6 +320,9 @@ INSERT INTO test_parameterized_sql VALUES(1, 1); SELECT * FROM test_parameterized_sql_function(1); ERROR: cannot perform distributed planning on this query because parameterized queries for SQL functions referencing distributed tables are not supported HINT: Consider using PL/pgSQL functions instead. +SELECT (SELECT 1 FROM test_parameterized_sql limit 1) FROM test_parameterized_sql_function(1); +ERROR: cannot perform distributed planning on this query because parameterized queries for SQL functions referencing distributed tables are not supported +HINT: Consider using PL/pgSQL functions instead. SELECT test_parameterized_sql_function_in_subquery_where(1); ERROR: could not create distributed plan DETAIL: Possibly this is caused by the use of parameters in SQL functions, which is not supported in Citus. diff --git a/src/test/regress/sql/multi_sql_function.sql b/src/test/regress/sql/multi_sql_function.sql index 329d57996..1ef0ef40a 100644 --- a/src/test/regress/sql/multi_sql_function.sql +++ b/src/test/regress/sql/multi_sql_function.sql @@ -144,6 +144,9 @@ INSERT INTO test_parameterized_sql VALUES(1, 1); -- all of them should fail SELECT * FROM test_parameterized_sql_function(1); + +SELECT (SELECT 1 FROM test_parameterized_sql limit 1) FROM test_parameterized_sql_function(1); + SELECT test_parameterized_sql_function_in_subquery_where(1); -- postgres behaves slightly differently for the following