From 825666f912273ed3e139d85cd0a593132858047b Mon Sep 17 00:00:00 2001 From: Hanefi Onaldi Date: Wed, 23 Jan 2019 14:32:25 +0300 Subject: [PATCH] Query samples in docs and better errors --- .../distributed/planner/recursive_planning.c | 97 ++++++++++++------- src/include/distributed/version_compat.h | 2 +- .../expected/multi_function_in_join.out | 12 ++- .../regress/sql/multi_function_in_join.sql | 8 +- 4 files changed, 77 insertions(+), 42 deletions(-) diff --git a/src/backend/distributed/planner/recursive_planning.c b/src/backend/distributed/planner/recursive_planning.c index 553ed922f..d07de832d 100644 --- a/src/backend/distributed/planner/recursive_planning.c +++ b/src/backend/distributed/planner/recursive_planning.c @@ -164,7 +164,7 @@ static bool CteReferenceListWalker(Node *node, CteReferenceWalkerContext *contex static bool ContainsReferencesToOuterQuery(Query *query); static bool ContainsReferencesToOuterQueryWalker(Node *node, VarLevelsUpWalkerContext *context); -static void WrapFunctionsInQuery(Query *query); +static void WrapFunctionsInSubqueries(Query *query); static void TransformFunctionRTE(RangeTblEntry *rangeTblEntry); static bool ShouldTransformRTE(RangeTblEntry *rangeTableEntry); @@ -268,7 +268,7 @@ RecursivelyPlanSubqueriesAndCTEs(Query *query, RecursivePlanningContext *context } /* make sure function calls in joins are executed in the coordinator */ - WrapFunctionsInQuery(query); + WrapFunctionsInSubqueries(query); /* descend into subqueries */ query_tree_walker(query, RecursivelyPlanSubqueryWalker, context, 0); @@ -1313,14 +1313,14 @@ ContainsReferencesToOuterQueryWalker(Node *node, VarLevelsUpWalkerContext *conte /* - * WrapFunctionsInQuery iterates over all the immediate Range Table Entries + * WrapFunctionsInSubqueries iterates over all the immediate Range Table Entries * of a query and wraps the functions inside (SELECT * FROM fnc() f) subqueries. * * We currently wrap only those functions that return a single value. If a * function returns records or a table we leave it as it is * */ static void -WrapFunctionsInQuery(Query *query) +WrapFunctionsInSubqueries(Query *query) { List *rangeTableList = query->rtable; ListCell *rangeTableCell = NULL; @@ -1358,7 +1358,7 @@ TransformFunctionRTE(RangeTblEntry *rangeTblEntry) Var *targetColumn = NULL; TargetEntry *targetEntry = NULL; RangeTblFunction *rangeTblFunction = NULL; - int targetColumnIndex = 0; + AttrNumber targetColumnIndex = 0; TupleDesc tupleDesc = NULL; rangeTblFunction = linitial(rangeTblEntry->functions); @@ -1381,10 +1381,20 @@ TransformFunctionRTE(RangeTblEntry *rangeTblEntry) tupleDesc = (TupleDesc) get_expr_result_tupdesc(rangeTblFunction->funcexpr, true); - /* if tupleDesc is not null, we iterate over all the attributes and - * create targetEntries*/ + /* + * If tupleDesc is not null, we iterate over all the attributes and + * create targetEntries + * */ if (tupleDesc) { + /* + * A sample function join that end up here: + * + * CREATE FUNCTION f(..) RETURNS TABLE(c1 int, c2 text) AS .. ; + * SELECT .. FROM table JOIN f(..) ON ( .. ) ; + * + * We will iterate over Tuple Description attributes. i.e (c1 int, c2 text) + */ for (targetColumnIndex = 0; targetColumnIndex < tupleDesc->natts; targetColumnIndex++) { @@ -1397,7 +1407,7 @@ TransformFunctionRTE(RangeTblEntry *rangeTblEntry) * The indexing of attributes and TupleDesc and varattno differ * * varattno=0 corresponds to whole row - * varattno=1 corrensponds to first column that is stored in tupDesc->attrs[0] */ + * varattno=1 corresponds to first column that is stored in tupDesc->attrs[0] */ targetColumn = makeVar(1, targetColumnIndex + 1, columnType, -1, InvalidOid, 0); targetEntry = makeTargetEntry((Expr *) targetColumn, targetColumnIndex + 1, @@ -1407,13 +1417,13 @@ TransformFunctionRTE(RangeTblEntry *rangeTblEntry) } } /* - * If tupleDesc is NULL we have several cases: + * If tupleDesc is NULL we have 2 different cases: * * 1. The function returns a record but the attributes can not be - * determined before running the query. In this case the column names and - * types must be defined explicitly in the query + * determined just by looking at the function definition. In this case the + * column names and types must be defined explicitly in the query * - * 2. The function returns a non-composite type + * 2. The function returns a non-composite type (e.g. int, text, jsonb ..) * */ else { @@ -1427,24 +1437,56 @@ TransformFunctionRTE(RangeTblEntry *rangeTblEntry) char *columnName = strVal(lfirst(functionColumnName)); Oid columnType = InvalidOid; - /* use explicitly defined types in the query if they are available */ + /* + * If the function returns a set of records, the query needs + * to explicitly name column names and types + * + * Use explicitly defined types in the query if they are + * available + * */ if (list_length(rangeTblFunction->funccoltypes) > 0) { + /* + * A sample function join that end up here: + * + * CREATE FUNCTION get_set_of_records() RETURNS SETOF RECORD AS + * $cmd$ + * SELECT x, x+1 FROM generate_series(0,4) f(x) + * $cmd$ + * LANGUAGE SQL; + * + * SELECT * + * FROM table1 JOIN get_set_of_records() AS t2(x int, y int) + * ON (id = x); + * + * Note that the function definition does not have column + * names and types. Therefore the user needs to explicitly + * state them in the query + * */ columnType = list_nth_oid(rangeTblFunction->funccoltypes, targetColumnIndex); } /* use the types in the function definition otherwise */ else { + /* + * Only functions returning simple types end up here. + * A sample function: + * + * CREATE FUNCTION add(integer, integer) RETURNS integer AS + * 'SELECT $1 + $2;' + * LANGUAGE SQL; + * SELECT * FROM table JOIN add(3,5) sum ON ( .. ) ; + * */ FuncExpr *funcExpr = (FuncExpr *) rangeTblFunction->funcexpr; columnType = funcExpr->funcresulttype; } + /* Note that the column k is associated with varattno/resno of k+1 */ targetColumn = makeVar(1, targetColumnIndex + 1, columnType, -1, InvalidOid, 0); targetEntry = makeTargetEntry((Expr *) targetColumn, - targetColumnIndex + 1, columnName, - false); + targetColumnIndex + 1, columnName, false); subquery->targetList = lappend(subquery->targetList, targetEntry); targetColumnIndex++; @@ -1461,33 +1503,22 @@ TransformFunctionRTE(RangeTblEntry *rangeTblEntry) * ShouldTransformRTE determines whether a given RTE should bne wrapped in a * subquery. * - * Not all functions can be wrapped in a subquery for now. As we support more + * Not all functions should be wrapped in a subquery for now. As we support more * functions to be used in joins, the constraints here will be relaxed. * */ static bool ShouldTransformRTE(RangeTblEntry *rangeTableEntry) { - /* wrap only function rtes */ - if (rangeTableEntry->rtekind != RTE_FUNCTION) - { - return false; - } - /* - * TODO: remove this check once lateral joins are supported - * We do not support lateral joins on functions for now, as referencing - * columns of an outer query is quite tricky */ - if (rangeTableEntry->lateral) + * We should wrap only function rtes that are not LATERAL and + * without WITH CARDINALITY clause + * */ + if (rangeTableEntry->rtekind != RTE_FUNCTION || + rangeTableEntry->lateral || + rangeTableEntry->funcordinality) { return false; } - - /* We do not want to wrap read-intermediate-result function calls */ - if (ContainsReadIntermediateResultFunction(linitial(rangeTableEntry->functions))) - { - return false; - } - return true; } diff --git a/src/include/distributed/version_compat.h b/src/include/distributed/version_compat.h index 44c50d1c0..4803313b5 100644 --- a/src/include/distributed/version_compat.h +++ b/src/include/distributed/version_compat.h @@ -139,7 +139,7 @@ canonicalize_qual_compat(Expr *qual, bool is_check) * A convenient wrapper around get_expr_result_type() that is added on PG11 * * Note that this function ignores the second parameter and behaves - * slightly differently. + * slightly differently than the PG11 version. * * 1. The original function throws errors if noError flag is not set, we ignore * this flag here and return NULL in that case diff --git a/src/test/regress/expected/multi_function_in_join.out b/src/test/regress/expected/multi_function_in_join.out index ef65aac82..d06016cae 100644 --- a/src/test/regress/expected/multi_function_in_join.out +++ b/src/test/regress/expected/multi_function_in_join.out @@ -6,8 +6,8 @@ -- that we wrap those functions inside (SELECT * FROM fnc()) sub queries. -- -- We do not yet support those functions that: --- - return records --- - return tables +-- - have lateral joins +-- - have WITH ORDINALITY clause -- - are user-defined and immutable CREATE SCHEMA functions_in_joins; SET search_path TO 'functions_in_joins'; @@ -202,14 +202,16 @@ SELECT * FROM table1 JOIN the_answer_to_life() the_answer ON (id = the_answer) $cmd$); ERROR: Task failed to execute CONTEXT: PL/pgSQL function public.raise_failed_execution(text) line 6 at RAISE --- WITH ORDINALITY clause forcing the result type to be RECORD/RECORDs +-- WITH ORDINALITY clause +SELECT public.raise_failed_execution($cmd$ SELECT * FROM table1 JOIN next_k_integers(10,5) WITH ORDINALITY next_integers ON (id = next_integers.result) ORDER BY id ASC; -ERROR: attribute 2 of type record has wrong type -DETAIL: Table has type bigint, but query expects integer. +$cmd$); +ERROR: Task failed to execute +CONTEXT: PL/pgSQL function public.raise_failed_execution(text) line 6 at RAISE RESET client_min_messages; DROP SCHEMA functions_in_joins CASCADE; NOTICE: drop cascades to 11 other objects diff --git a/src/test/regress/sql/multi_function_in_join.sql b/src/test/regress/sql/multi_function_in_join.sql index 86fe2a769..61d9700e0 100644 --- a/src/test/regress/sql/multi_function_in_join.sql +++ b/src/test/regress/sql/multi_function_in_join.sql @@ -6,8 +6,8 @@ -- that we wrap those functions inside (SELECT * FROM fnc()) sub queries. -- -- We do not yet support those functions that: --- - return records --- - return tables +-- - have lateral joins +-- - have WITH ORDINALITY clause -- - are user-defined and immutable CREATE SCHEMA functions_in_joins; @@ -132,12 +132,14 @@ SELECT public.raise_failed_execution($cmd$ SELECT * FROM table1 JOIN the_answer_to_life() the_answer ON (id = the_answer) $cmd$); --- WITH ORDINALITY clause forcing the result type to be RECORD/RECORDs +-- WITH ORDINALITY clause +SELECT public.raise_failed_execution($cmd$ SELECT * FROM table1 JOIN next_k_integers(10,5) WITH ORDINALITY next_integers ON (id = next_integers.result) ORDER BY id ASC; +$cmd$); RESET client_min_messages; DROP SCHEMA functions_in_joins CASCADE;