From 9ea14ef56ab8b7d22a4148c4e6765a7874d968a4 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 6 Oct 2005 19:51:16 +0000 Subject: [PATCH] When a function not returning RECORD has a single OUT parameter, use the parameter's name (if any) as the default column name for SELECT FROM the function, rather than the function name as previously. I still think this is a bad idea, but I lost the argument. Force decompilation of function RTEs to specify full aliases always, to reduce the odds of this decision breaking dumped views. --- src/backend/parser/parse_relation.c | 68 ++++++++++++--- src/backend/utils/adt/ruleutils.c | 48 ++++++++--- src/backend/utils/fmgr/funcapi.c | 104 ++++++++++++++++++++++- src/include/funcapi.h | 4 +- src/test/regress/expected/plpgsql.out | 6 +- src/test/regress/expected/rangefuncs.out | 26 +++--- 6 files changed, 215 insertions(+), 41 deletions(-) diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 85984cb1d9..196936bb09 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_relation.c,v 1.113 2005/08/01 20:31:10 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_relation.c,v 1.114 2005/10/06 19:51:13 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -505,6 +505,59 @@ buildRelationAliases(TupleDesc tupdesc, Alias *alias, Alias *eref) eref->aliasname, maxattrs - numdropped, numaliases))); } +/* + * buildScalarFunctionAlias + * Construct the eref column name list for a function RTE, + * when the function returns a scalar type (not composite or RECORD). + * + * funcexpr: transformed expression tree for the function call + * funcname: function name (used only for error message) + * alias: the user-supplied alias, or NULL if none + * eref: the eref Alias to store column names in + * + * eref->colnames is filled in. + */ +static void +buildScalarFunctionAlias(Node *funcexpr, char *funcname, + Alias *alias, Alias *eref) +{ + char *pname; + + Assert(eref->colnames == NIL); + + /* Use user-specified column alias if there is one. */ + if (alias && alias->colnames != NIL) + { + if (list_length(alias->colnames) != 1) + ereport(ERROR, + (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), + errmsg("too many column aliases specified for function %s", + funcname))); + eref->colnames = copyObject(alias->colnames); + return; + } + + /* + * If the expression is a simple function call, and the function has a + * single OUT parameter that is named, use the parameter's name. + */ + if (funcexpr && IsA(funcexpr, FuncExpr)) + { + pname = get_func_result_name(((FuncExpr *) funcexpr)->funcid); + if (pname) + { + eref->colnames = list_make1(makeString(pname)); + return; + } + } + + /* + * Otherwise use the previously-determined alias (not necessarily the + * function name!) + */ + eref->colnames = list_make1(makeString(eref->aliasname)); +} + /* * Add an entry for a relation to the pstate's range table (p_rtable). * @@ -776,18 +829,7 @@ addRangeTableEntryForFunction(ParseState *pstate, else if (functypclass == TYPEFUNC_SCALAR) { /* Base data type, i.e. scalar */ - /* Just add one alias column named for the function. */ - if (alias && alias->colnames != NIL) - { - if (list_length(alias->colnames) != 1) - ereport(ERROR, - (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), - errmsg("too many column aliases specified for function %s", - funcname))); - eref->colnames = copyObject(alias->colnames); - } - else - eref->colnames = list_make1(makeString(eref->aliasname)); + buildScalarFunctionAlias(funcexpr, funcname, alias, eref); } else if (functypclass == TYPEFUNC_RECORD) { diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index bf9ebfefaa..1a226bd49c 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -3,7 +3,7 @@ * back to source text * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.205 2005/08/01 20:31:12 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.206 2005/10/06 19:51:14 tgl Exp $ * * This software is copyrighted by Jan Wieck - Hamburg. * @@ -204,8 +204,8 @@ static void get_from_clause(Query *query, const char *prefix, deparse_context *context); static void get_from_clause_item(Node *jtnode, Query *query, deparse_context *context); -static void get_from_clause_alias(Alias *alias, int varno, - Query *query, deparse_context *context); +static void get_from_clause_alias(Alias *alias, RangeTblEntry *rte, + deparse_context *context); static void get_from_clause_coldeflist(List *coldeflist, deparse_context *context); static void get_opclass_name(Oid opclass, Oid actual_datatype, @@ -4113,16 +4113,15 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) elog(ERROR, "unrecognized RTE kind: %d", (int) rte->rtekind); break; } + if (rte->alias != NULL) { appendStringInfo(buf, " %s", quote_identifier(rte->alias->aliasname)); gavealias = true; - if (coldeflist == NIL) - get_from_clause_alias(rte->alias, varno, query, context); } else if (rte->rtekind == RTE_RELATION && - strcmp(rte->eref->aliasname, get_rel_name(rte->relid)) != 0) + strcmp(rte->eref->aliasname, get_rel_name(rte->relid)) != 0) { /* * Apparently the rel has been renamed since the rule was @@ -4134,12 +4133,40 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) quote_identifier(rte->eref->aliasname)); gavealias = true; } + else if (rte->rtekind == RTE_FUNCTION) + { + /* + * For a function RTE, always give an alias. + * This covers possible renaming of the function and/or + * instability of the FigureColname rules for things that + * aren't simple functions. + */ + appendStringInfo(buf, " %s", + quote_identifier(rte->eref->aliasname)); + gavealias = true; + } + if (coldeflist != NIL) { if (!gavealias) appendStringInfo(buf, " AS "); get_from_clause_coldeflist(coldeflist, context); } + else + { + /* + * For a function RTE, always emit a complete column alias list; + * this is to protect against possible instability of the default + * column names (eg, from altering parameter names). Otherwise + * just report whatever the user originally gave as column + * aliases. + */ + if (rte->rtekind == RTE_FUNCTION) + get_from_clause_alias(rte->eref, rte, context); + else + get_from_clause_alias(rte->alias, rte, context); + } + } else if (IsA(jtnode, JoinExpr)) { @@ -4273,7 +4300,9 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) { appendStringInfo(buf, " %s", quote_identifier(j->alias->aliasname)); - get_from_clause_alias(j->alias, j->rtindex, query, context); + get_from_clause_alias(j->alias, + rt_fetch(j->rtindex, query->rtable), + context); } } else @@ -4287,11 +4316,10 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) * This is tricky because we must ignore dropped columns. */ static void -get_from_clause_alias(Alias *alias, int varno, - Query *query, deparse_context *context) +get_from_clause_alias(Alias *alias, RangeTblEntry *rte, + deparse_context *context) { StringInfo buf = context->buf; - RangeTblEntry *rte = rt_fetch(varno, query->rtable); ListCell *col; AttrNumber attnum; bool first = true; diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c index b6ccd63d4d..598168a70a 100644 --- a/src/backend/utils/fmgr/funcapi.c +++ b/src/backend/utils/fmgr/funcapi.c @@ -7,7 +7,7 @@ * Copyright (c) 2002-2005, PostgreSQL Global Development Group * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/fmgr/funcapi.c,v 1.24 2005/07/03 21:14:18 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/fmgr/funcapi.c,v 1.25 2005/10/06 19:51:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -627,6 +627,108 @@ get_type_func_class(Oid typid) } +/* + * get_func_result_name + * + * If the function has exactly one output parameter, and that parameter + * is named, return the name (as a palloc'd string). Else return NULL. + * + * This is used to determine the default output column name for functions + * returning scalar types. + */ +char * +get_func_result_name(Oid functionId) +{ + char *result; + HeapTuple procTuple; + Datum proargmodes; + Datum proargnames; + bool isnull; + ArrayType *arr; + int numargs; + char *argmodes; + Datum *argnames; + int numoutargs; + int nargnames; + int i; + + /* First fetch the function's pg_proc row */ + procTuple = SearchSysCache(PROCOID, + ObjectIdGetDatum(functionId), + 0, 0, 0); + if (!HeapTupleIsValid(procTuple)) + elog(ERROR, "cache lookup failed for function %u", functionId); + + /* If there are no named OUT parameters, return NULL */ + if (heap_attisnull(procTuple, Anum_pg_proc_proargmodes) || + heap_attisnull(procTuple, Anum_pg_proc_proargnames)) + result = NULL; + else + { + /* Get the data out of the tuple */ + proargmodes = SysCacheGetAttr(PROCOID, procTuple, + Anum_pg_proc_proargmodes, + &isnull); + Assert(!isnull); + proargnames = SysCacheGetAttr(PROCOID, procTuple, + Anum_pg_proc_proargnames, + &isnull); + Assert(!isnull); + + /* + * We expect the arrays to be 1-D arrays of the right types; verify + * that. For the char array, we don't need to use deconstruct_array() + * since the array data is just going to look like a C array of + * values. + */ + arr = DatumGetArrayTypeP(proargmodes); /* ensure not toasted */ + numargs = ARR_DIMS(arr)[0]; + if (ARR_NDIM(arr) != 1 || + numargs < 0 || + ARR_ELEMTYPE(arr) != CHAROID) + elog(ERROR, "proargmodes is not a 1-D char array"); + argmodes = (char *) ARR_DATA_PTR(arr); + arr = DatumGetArrayTypeP(proargnames); /* ensure not toasted */ + if (ARR_NDIM(arr) != 1 || + ARR_DIMS(arr)[0] != numargs || + ARR_ELEMTYPE(arr) != TEXTOID) + elog(ERROR, "proargnames is not a 1-D text array"); + deconstruct_array(arr, TEXTOID, -1, false, 'i', + &argnames, &nargnames); + Assert(nargnames == numargs); + + /* scan for output argument(s) */ + result = NULL; + numoutargs = 0; + for (i = 0; i < numargs; i++) + { + if (argmodes[i] == PROARGMODE_IN) + continue; + Assert(argmodes[i] == PROARGMODE_OUT || + argmodes[i] == PROARGMODE_INOUT); + if (++numoutargs > 1) + { + /* multiple out args, so forget it */ + result = NULL; + break; + } + result = DatumGetCString(DirectFunctionCall1(textout, + argnames[i])); + if (result == NULL || result[0] == '\0') + { + /* Parameter is not named, so forget it */ + result = NULL; + break; + } + } + } + + ReleaseSysCache(procTuple); + + return result; +} + + /* * build_function_result_tupdesc_t * diff --git a/src/include/funcapi.h b/src/include/funcapi.h index 8acbf3aa3b..ddeec2930f 100644 --- a/src/include/funcapi.h +++ b/src/include/funcapi.h @@ -9,7 +9,7 @@ * * Copyright (c) 2002-2005, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/include/funcapi.h,v 1.18 2005/05/30 23:09:07 tgl Exp $ + * $PostgreSQL: pgsql/src/include/funcapi.h,v 1.19 2005/10/06 19:51:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -167,6 +167,8 @@ extern TypeFuncClass get_func_result_type(Oid functionId, Oid *resultTypeId, TupleDesc *resultTupleDesc); +extern char *get_func_result_name(Oid functionId); + extern bool resolve_polymorphic_argtypes(int numargs, Oid *argtypes, char *argmodes, Node *call_expr); diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 170669c62b..aef73e1eee 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -1762,7 +1762,7 @@ select f1(42); (1 row) select * from f1(42); - f1 + j ---- 43 (1 row) @@ -1778,7 +1778,7 @@ select f1(42); (1 row) select * from f1(42); - f1 + i ---- 43 (1 row) @@ -1793,7 +1793,7 @@ begin return; end$$ language plpgsql; select * from f1(42); - f1 + j ---- 43 44 diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out index 42f770ff55..49fc121f04 100644 --- a/src/test/regress/expected/rangefuncs.out +++ b/src/test/regress/expected/rangefuncs.out @@ -1,15 +1,15 @@ SELECT name, setting FROM pg_settings WHERE name LIKE 'enable%'; - name | setting + name | setting -------------------+--------- - enable_bitmapscan | on - enable_hashagg | on - enable_hashjoin | on - enable_indexscan | on - enable_mergejoin | on - enable_nestloop | on - enable_seqscan | on - enable_sort | on - enable_tidscan | on + enable_bitmapscan | on + enable_hashagg | on + enable_hashjoin | on + enable_indexscan | on + enable_mergejoin | on + enable_nestloop | on + enable_seqscan | on + enable_sort | on + enable_tidscan | on (9 rows) CREATE TABLE foo2(fooid int, f2 int); @@ -409,9 +409,9 @@ SELECT foo(42); (1 row) SELECT * FROM foo(42); - foo ------ - 43 + f2 +---- + 43 (1 row) SELECT * FROM foo(42) AS p(x); -- 2.40.0