From 0339047bc93147c1c6f78f867ae6b0c215406235 Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Fri, 23 Mar 2012 17:29:57 -0400 Subject: [PATCH] Code review for protransform patches. Fix loss of previous expression-simplification work when a transform function fires: we must not simply revert to untransformed input tree. Instead build a dummy FuncExpr node to pass to the transform function. This has the additional advantage of providing a simpler, more uniform API for transform functions. Move documentation to a somewhat less buried spot, relocate some poorly-placed code, be more wary of null constants and invalid typmod values, add an opr_sanity check on protransform function signatures, and some other minor cosmetic adjustments. Note: although this patch touches pg_proc.h, no need for catversion bump, because the changes are cosmetic and don't actually change the intended catalog contents. --- doc/src/sgml/catalogs.sgml | 3 +- doc/src/sgml/xfunc.sgml | 34 +++++++ src/backend/nodes/nodeFuncs.c | 29 +++++- src/backend/optimizer/util/clauses.c | 115 ++++++++++------------- src/backend/parser/parse_clause.c | 22 ----- src/backend/utils/adt/datetime.c | 29 +++--- src/backend/utils/adt/numeric.c | 23 +++-- src/backend/utils/adt/timestamp.c | 30 +++--- src/backend/utils/adt/varbit.c | 23 +++-- src/backend/utils/adt/varchar.c | 26 ++--- src/include/catalog/pg_proc.h | 23 ++--- src/include/nodes/nodeFuncs.h | 1 + src/include/parser/parse_clause.h | 2 - src/test/regress/expected/opr_sanity.out | 10 ++ src/test/regress/sql/opr_sanity.sql | 7 ++ 15 files changed, 205 insertions(+), 172 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 9564e012e66..dcaeda413d3 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -4398,7 +4398,8 @@ <entry><structfield>protransform</structfield></entry> <entry><type>regproc</type></entry> <entry><literal><link linkend="catalog-pg-proc"><structname>pg_proc</structname></link>.oid</literal></entry> - <entry>Calls to function can be simplified by this other function</entry> + <entry>Calls to this function can be simplified by this other function + (see <xref linkend="xfunc-transform-functions">)</entry> </row> <row> diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index e8620ef0738..58df90e30fe 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3177,6 +3177,40 @@ CREATE FUNCTION make_array(anyelement) RETURNS anyarray </para> </sect2> + <sect2 id="xfunc-transform-functions"> + <title>Transform Functions</title> + + <para> + Some function calls can be simplified during planning based on + properties specific to the function. For example, + <literal>int4mul(n, 1)</> could be simplified to just <literal>n</>. + To define such function-specific optimizations, write a + <firstterm>transform function</> and place its OID in the + <structfield>protransform</> field of the primary function's + <structname>pg_proc</> entry. The transform function must have the SQL + signature <literal>protransform(internal) RETURNS internal</>. The + argument, actually <type>FuncExpr *</>, is a dummy node representing a + call to the primary function. If the transform function's study of the + expression tree proves that a simplified expression tree can substitute + for all possible concrete calls represented thereby, build and return + that simplified expression. Otherwise, return a <literal>NULL</> + pointer (<emphasis>not</> a SQL null). + </para> + + <para> + We make no guarantee that <productname>PostgreSQL</> will never call the + primary function in cases that the transform function could simplify. + Ensure rigorous equivalence between the simplified expression and an + actual call to the primary function. + </para> + + <para> + Currently, this facility is not exposed to users at the SQL level + because of security concerns, so it is only practical to use for + optimizing built-in functions. + </para> + </sect2> + <sect2> <title>Shared Memory and LWLocks</title> diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index 51459c4a43f..6f9e0536694 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -17,6 +17,7 @@ #include "catalog/pg_collation.h" #include "catalog/pg_type.h" #include "miscadmin.h" +#include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" #include "nodes/relation.h" #include "utils/builtins.h" @@ -547,6 +548,30 @@ exprIsLengthCoercion(const Node *expr, int32 *coercedTypmod) return false; } +/* + * relabel_to_typmod + * Add a RelabelType node that changes just the typmod of the expression. + * + * This is primarily intended to be used during planning. Therefore, it + * strips any existing RelabelType nodes to maintain the planner's invariant + * that there are not adjacent RelabelTypes, and it uses COERCE_DONTCARE + * which would typically be inappropriate earlier. + */ +Node * +relabel_to_typmod(Node *expr, int32 typmod) +{ + Oid type = exprType(expr); + Oid coll = exprCollation(expr); + + /* Strip any existing RelabelType node(s) */ + while (expr && IsA(expr, RelabelType)) + expr = (Node *) ((RelabelType *) expr)->arg; + + /* Apply new typmod, preserving the previous exposed type and collation */ + return (Node *) makeRelabelType((Expr *) expr, type, typmod, coll, + COERCE_DONTCARE); +} + /* * expression_returns_set * Test whether an expression returns a set result. @@ -2694,7 +2719,9 @@ query_or_expression_tree_mutator(Node *node, * that could appear under it, but not other statement types. */ bool - raw_expression_tree_walker(Node *node, bool (*walker) (), void *context) +raw_expression_tree_walker(Node *node, + bool (*walker) (), + void *context) { ListCell *temp; diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index b14ae2e3670..117c9c0e451 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -107,11 +107,11 @@ static List *simplify_and_arguments(List *args, eval_const_expressions_context *context, bool *haveNull, bool *forceFalse); static Node *simplify_boolean_equality(Oid opno, List *args); -static Expr *simplify_function(Expr *oldexpr, Oid funcid, - Oid result_type, int32 result_typmod, Oid result_collid, - Oid input_collid, List **args, +static Expr *simplify_function(Oid funcid, + Oid result_type, int32 result_typmod, + Oid result_collid, Oid input_collid, List **args, bool has_named_args, - bool allow_inline, + bool allow_non_const, eval_const_expressions_context *context); static List *reorder_function_arguments(List *args, Oid result_type, HeapTuple func_tuple, @@ -2332,8 +2332,7 @@ eval_const_expressions_mutator(Node *node, * length coercion; we want to preserve the typmod in the * eventual Const if so. */ - simple = simplify_function((Expr *) expr, - expr->funcid, + simple = simplify_function(expr->funcid, expr->funcresulttype, exprTypmod(node), expr->funccollid, @@ -2389,8 +2388,7 @@ eval_const_expressions_mutator(Node *node, * Code for op/func reduction is pretty bulky, so split it out * as a separate function. */ - simple = simplify_function((Expr *) expr, - expr->opfuncid, + simple = simplify_function(expr->opfuncid, expr->opresulttype, -1, expr->opcollid, expr->inputcollid, @@ -2491,8 +2489,7 @@ eval_const_expressions_mutator(Node *node, * Code for op/func reduction is pretty bulky, so split it * out as a separate function. */ - simple = simplify_function((Expr *) expr, - expr->opfuncid, + simple = simplify_function(expr->opfuncid, expr->opresulttype, -1, expr->opcollid, expr->inputcollid, @@ -2698,8 +2695,7 @@ eval_const_expressions_mutator(Node *node, getTypeInputInfo(expr->resulttype, &infunc, &intypioparam); - simple = simplify_function(NULL, - outfunc, + simple = simplify_function(outfunc, CSTRINGOID, -1, InvalidOid, InvalidOid, @@ -2728,8 +2724,7 @@ eval_const_expressions_mutator(Node *node, false, true)); - simple = simplify_function(NULL, - infunc, + simple = simplify_function(infunc, expr->resulttype, -1, expr->resultcollid, InvalidOid, @@ -3581,15 +3576,11 @@ simplify_boolean_equality(Oid opno, List *args) * Subroutine for eval_const_expressions: try to simplify a function call * (which might originally have been an operator; we don't care) * - * Inputs are the original expression (can be NULL), function OID, actual - * result type OID (which is needed for polymorphic functions), result typmod, - * result collation, the input collation to use for the function, the - * pre-simplified argument list, and some flags; also the context data for - * eval_const_expressions. In common cases, several of the arguments could be - * derived from the original expression. Sending them separately avoids - * duplicating NodeTag-specific knowledge, and it's necessary for CoerceViaIO. - * A NULL original expression disables use of transform functions while - * retaining all other behaviors. + * Inputs are the function OID, actual result type OID (which is needed for + * polymorphic functions), result typmod, result collation, + * the input collation to use for the function, + * the pre-simplified argument list, and some flags; + * also the context data for eval_const_expressions. * * Returns a simplified expression if successful, or NULL if cannot * simplify the function call. @@ -3601,28 +3592,32 @@ simplify_boolean_equality(Oid opno, List *args) * pass-by-reference, and it may get modified even if simplification fails. */ static Expr * -simplify_function(Expr *oldexpr, Oid funcid, - Oid result_type, int32 result_typmod, Oid result_collid, - Oid input_collid, List **args, +simplify_function(Oid funcid, Oid result_type, int32 result_typmod, + Oid result_collid, Oid input_collid, List **args, bool has_named_args, - bool allow_inline, + bool allow_non_const, eval_const_expressions_context *context) { HeapTuple func_tuple; + Form_pg_proc func_form; Expr *newexpr; - Oid transform; /* * We have three strategies for simplification: execute the function to * deliver a constant result, use a transform function to generate a * substitute node tree, or expand in-line the body of the function * definition (which only works for simple SQL-language functions, but - * that is a common case). Each needs access to the function's pg_proc - * tuple, so fetch it just once. + * that is a common case). Each case needs access to the function's + * pg_proc tuple, so fetch it just once. + * + * Note: the allow_non_const flag suppresses both the second and third + * strategies; so if !allow_non_const, simplify_function can only return + * a Const or NULL. Argument-list rewriting happens anyway, though. */ func_tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid)); if (!HeapTupleIsValid(func_tuple)) elog(ERROR, "cache lookup failed for function %u", funcid); + func_form = (Form_pg_proc) GETSTRUCT(func_tuple); /* * While we have the tuple, reorder named arguments and add default @@ -3631,48 +3626,38 @@ simplify_function(Expr *oldexpr, Oid funcid, if (has_named_args) *args = reorder_function_arguments(*args, result_type, func_tuple, context); - else if (((Form_pg_proc) GETSTRUCT(func_tuple))->pronargs > list_length(*args)) + else if (func_form->pronargs > list_length(*args)) *args = add_function_defaults(*args, result_type, func_tuple, context); newexpr = evaluate_function(funcid, result_type, result_typmod, result_collid, input_collid, *args, func_tuple, context); - /* - * Some functions calls can be simplified at plan time based on properties - * specific to the function. For example, "varchar(s::varchar(4), 8, - * true)" simplifies to "s::varchar(4)", and "int4mul(n, 1)" could - * simplify to "n". To define such function-specific optimizations, write - * a "transform function" and store its OID in the pg_proc.protransform of - * the primary function. Give each transform function the signature - * "protransform(internal) RETURNS internal". The argument, internally an - * Expr *, is the node representing a call to the primary function. If - * the transform function's study of that node proves that a simplified - * Expr substitutes for all possible concrete calls represented thereby, - * return that simplified Expr. Otherwise, return the NULL pointer. - * - * Currently, the specific Expr nodetag can be FuncExpr, OpExpr or - * DistinctExpr. This list may change in the future. The function should - * check the nodetag and return the NULL pointer for unexpected inputs. - * - * We make no guarantee that PostgreSQL will never call the primary - * function in cases that the transform function would simplify. Ensure - * rigorous equivalence between the simplified expression and an actual - * call to the primary function. - * - * Currently, this facility is undocumented and not exposed to users at - * the SQL level. Core length coercion casts use it to avoid calls - * guaranteed to return their input unchanged. This in turn allows ALTER - * TABLE ALTER TYPE to avoid rewriting tables for some typmod changes. In - * the future, this facility may find other applications, like simplifying - * x*0, x*1, and x+0. - */ - transform = ((Form_pg_proc) GETSTRUCT(func_tuple))->protransform; - if (!newexpr && OidIsValid(transform) && oldexpr) - newexpr = (Expr *) DatumGetPointer(OidFunctionCall1(transform, - PointerGetDatum(oldexpr))); + if (!newexpr && allow_non_const && OidIsValid(func_form->protransform)) + { + /* + * Build a dummy FuncExpr node containing the simplified arg list. We + * use this approach to present a uniform interface to the transform + * function regardless of how the function is actually being invoked. + */ + FuncExpr fexpr; + + fexpr.xpr.type = T_FuncExpr; + fexpr.funcid = funcid; + fexpr.funcresulttype = result_type; + fexpr.funcretset = func_form->proretset; + fexpr.funcformat = COERCE_DONTCARE; + fexpr.funccollid = result_collid; + fexpr.inputcollid = input_collid; + fexpr.args = *args; + fexpr.location = -1; + + newexpr = (Expr *) + DatumGetPointer(OidFunctionCall1(func_form->protransform, + PointerGetDatum(&fexpr))); + } - if (!newexpr && allow_inline) + if (!newexpr && allow_non_const) newexpr = inline_function(funcid, result_type, result_collid, input_collid, *args, func_tuple, context); diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 3a23cddd337..97ab9d5581a 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -2271,25 +2271,3 @@ transformFrameOffset(ParseState *pstate, int frameOptions, Node *clause) return node; } - -/* - * relabel_to_typmod - * Add a RelabelType node that changes just the typmod, and remove all - * now-superfluous RelabelType nodes beneath it. - */ -Node * -relabel_to_typmod(Node *expr, int32 typmod) -{ - Oid type = exprType(expr); - Oid coll = exprCollation(expr); - - /* - * Strip any existing RelabelType, then add one. This is to preserve the - * invariant of no redundant RelabelTypes. - */ - while (IsA(expr, RelabelType)) - expr = (Node *) ((RelabelType *) expr)->arg; - - return (Node *) makeRelabelType((Expr *) expr, type, typmod, coll, - COERCE_DONTCARE); -} diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c index 9ceb269b3fc..31ae045da9d 100644 --- a/src/backend/utils/adt/datetime.c +++ b/src/backend/utils/adt/datetime.c @@ -24,7 +24,6 @@ #include "funcapi.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" -#include "parser/parse_clause.h" #include "utils/builtins.h" #include "utils/date.h" #include "utils/datetime.h" @@ -4153,32 +4152,34 @@ CheckDateTokenTables(void) } /* - * Helper for temporal protransform functions. Types time, timetz, timestamp - * and timestamptz each have a range of allowed precisions. An unspecified - * precision is rigorously equivalent to the highest specifiable precision. + * Common code for temporal protransform functions. Types time, timetz, + * timestamp and timestamptz each have a range of allowed precisions. An + * unspecified precision is rigorously equivalent to the highest specifiable + * precision. + * + * Note: timestamp_scale throws an error when the typmod is out of range, but + * we can't get there from a cast: our typmodin will have caught it already. */ Node * TemporalTransform(int32 max_precis, Node *node) { FuncExpr *expr = (FuncExpr *) node; - Node *typmod; Node *ret = NULL; + Node *typmod; - if (!IsA(expr, FuncExpr)) - return ret; + Assert(IsA(expr, FuncExpr)); + Assert(list_length(expr->args) >= 2); - Assert(list_length(expr->args) == 2); - typmod = lsecond(expr->args); + typmod = (Node *) lsecond(expr->args); - if (IsA(typmod, Const)) + if (IsA(typmod, Const) && !((Const *) typmod)->constisnull) { - Node *source = linitial(expr->args); + Node *source = (Node *) linitial(expr->args); int32 old_precis = exprTypmod(source); int32 new_precis = DatumGetInt32(((Const *) typmod)->constvalue); - if (new_precis == -1 || - new_precis == max_precis || - (old_precis != -1 && new_precis >= old_precis)) + if (new_precis < 0 || new_precis == max_precis || + (old_precis >= 0 && new_precis >= old_precis)) ret = relabel_to_typmod(source, new_precis); } diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 627870a4464..5723b9af4ed 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -31,7 +31,6 @@ #include "libpq/pqformat.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" -#include "parser/parse_clause.h" #include "utils/array.h" #include "utils/builtins.h" #include "utils/int8.h" @@ -717,7 +716,7 @@ numeric_send(PG_FUNCTION_ARGS) /* * numeric_transform() - * - * Flatten calls to our length coercion function that solely represent + * Flatten calls to numeric's length coercion function that solely represent * increases in allowable precision. Scale changes mutate every datum, so * they are unoptimizable. Some values, e.g. 1E-1001, can only fit into an * unconstrained numeric, so a change from an unconstrained numeric to any @@ -727,18 +726,17 @@ Datum numeric_transform(PG_FUNCTION_ARGS) { FuncExpr *expr = (FuncExpr *) PG_GETARG_POINTER(0); - Node *typmod; Node *ret = NULL; + Node *typmod; - if (!IsA(expr, FuncExpr)) - PG_RETURN_POINTER(ret); + Assert(IsA(expr, FuncExpr)); + Assert(list_length(expr->args) >= 2); - Assert(list_length(expr->args) == 2); - typmod = lsecond(expr->args); + typmod = (Node *) lsecond(expr->args); - if (IsA(typmod, Const)) + if (IsA(typmod, Const) && !((Const *) typmod)->constisnull) { - Node *source = linitial(expr->args); + Node *source = (Node *) linitial(expr->args); int32 old_typmod = exprTypmod(source); int32 new_typmod = DatumGetInt32(((Const *) typmod)->constvalue); int32 old_scale = (old_typmod - VARHDRSZ) & 0xffff; @@ -748,11 +746,12 @@ numeric_transform(PG_FUNCTION_ARGS) /* * If new_typmod < VARHDRSZ, the destination is unconstrained; that's - * always OK. If old_typmod >= VARHDRSZ, the source is constrained. - * and we're OK if the scale is unchanged and the precison is not + * always OK. If old_typmod >= VARHDRSZ, the source is constrained, + * and we're OK if the scale is unchanged and the precision is not * decreasing. See further notes in function header comment. */ - if (new_typmod < VARHDRSZ || (old_typmod >= VARHDRSZ && + if (new_typmod < (int32) VARHDRSZ || + (old_typmod >= (int32) VARHDRSZ && new_scale == old_scale && new_precision >= old_precision)) ret = relabel_to_typmod(source, new_typmod); } diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 88ff57dd38a..9a7238be38d 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -28,7 +28,6 @@ #include "libpq/pqformat.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" -#include "parser/parse_clause.h" #include "parser/scansup.h" #include "utils/array.h" #include "utils/builtins.h" @@ -316,10 +315,6 @@ timestamptypmodout(PG_FUNCTION_ARGS) Datum timestamp_transform(PG_FUNCTION_ARGS) { - /* - * timestamp_scale throws an error when the typmod is out of range, but we - * can't get there from a cast: our typmodin will have caught it already. - */ PG_RETURN_POINTER(TemporalTransform(MAX_TIMESTAMP_PRECISION, (Node *) PG_GETARG_POINTER(0))); } @@ -937,18 +932,17 @@ Datum interval_transform(PG_FUNCTION_ARGS) { FuncExpr *expr = (FuncExpr *) PG_GETARG_POINTER(0); - Node *typmod; Node *ret = NULL; + Node *typmod; - if (!IsA(expr, FuncExpr)) - PG_RETURN_POINTER(ret); + Assert(IsA(expr, FuncExpr)); + Assert(list_length(expr->args) >= 2); - Assert(list_length(expr->args) == 2); - typmod = lsecond(expr->args); + typmod = (Node *) lsecond(expr->args); - if (IsA(typmod, Const)) + if (IsA(typmod, Const) && !((Const *) typmod)->constisnull) { - Node *source = linitial(expr->args); + Node *source = (Node *) linitial(expr->args); int32 old_typmod = exprTypmod(source); int32 new_typmod = DatumGetInt32(((Const *) typmod)->constvalue); int old_range; @@ -958,7 +952,7 @@ interval_transform(PG_FUNCTION_ARGS) int new_range_fls; int old_range_fls; - if (old_typmod == -1) + if (old_typmod < 0) { old_range = INTERVAL_FULL_RANGE; old_precis = INTERVAL_FULL_PRECISION; @@ -978,11 +972,9 @@ interval_transform(PG_FUNCTION_ARGS) */ new_range_fls = fls(new_range); old_range_fls = fls(old_range); - if (new_typmod == -1 || - ((new_range_fls >= SECOND || - new_range_fls >= old_range_fls) && - (old_range_fls < SECOND || - new_precis >= MAX_INTERVAL_PRECISION || + if (new_typmod < 0 || + ((new_range_fls >= SECOND || new_range_fls >= old_range_fls) && + (old_range_fls < SECOND || new_precis >= MAX_INTERVAL_PRECISION || new_precis >= old_precis))) ret = relabel_to_typmod(source, new_typmod); } @@ -1068,7 +1060,7 @@ AdjustIntervalForTypmod(Interval *interval, int32 typmod) * can't do it consistently. (We cannot enforce a range limit on the * highest expected field, since we do not have any equivalent of * SQL's <interval leading field precision>.) If we ever decide to - * revisit this, interval_transform will likely requite adjusting. + * revisit this, interval_transform will likely require adjusting. * * Note: before PG 8.4 we interpreted a limited set of fields as * actually causing a "modulo" operation on a given value, potentially diff --git a/src/backend/utils/adt/varbit.c b/src/backend/utils/adt/varbit.c index 1227e5a5082..e74e062338d 100644 --- a/src/backend/utils/adt/varbit.c +++ b/src/backend/utils/adt/varbit.c @@ -19,7 +19,6 @@ #include "access/htup.h" #include "libpq/pqformat.h" #include "nodes/nodeFuncs.h" -#include "parser/parse_clause.h" #include "utils/array.h" #include "utils/varbit.h" @@ -649,31 +648,31 @@ varbit_send(PG_FUNCTION_ARGS) /* * varbit_transform() - * Flatten calls to our length coercion function that leave the new maximum - * length >= the previous maximum length. We ignore the isExplicit argument, - * which only affects truncation. + * Flatten calls to varbit's length coercion function that set the new maximum + * length >= the previous maximum length. We can ignore the isExplicit + * argument, since that only affects truncation cases. */ Datum varbit_transform(PG_FUNCTION_ARGS) { FuncExpr *expr = (FuncExpr *) PG_GETARG_POINTER(0); - Node *typmod; Node *ret = NULL; + Node *typmod; - if (!IsA(expr, FuncExpr)) - PG_RETURN_POINTER(ret); + Assert(IsA(expr, FuncExpr)); + Assert(list_length(expr->args) >= 2); - Assert(list_length(expr->args) == 3); - typmod = lsecond(expr->args); + typmod = (Node *) lsecond(expr->args); - if (IsA(typmod, Const)) + if (IsA(typmod, Const) && !((Const *) typmod)->constisnull) { - Node *source = linitial(expr->args); + Node *source = (Node *) linitial(expr->args); int32 new_typmod = DatumGetInt32(((Const *) typmod)->constvalue); int32 old_max = exprTypmod(source); int32 new_max = new_typmod; - if (new_max <= 0 || (old_max >= 0 && old_max <= new_max)) + /* Note: varbit() treats typmod 0 as invalid, so we do too */ + if (new_max <= 0 || (old_max > 0 && old_max <= new_max)) ret = relabel_to_typmod(source, new_typmod); } diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c index 98ec04ef631..5774102bc02 100644 --- a/src/backend/utils/adt/varchar.c +++ b/src/backend/utils/adt/varchar.c @@ -19,7 +19,6 @@ #include "access/tuptoaster.h" #include "libpq/pqformat.h" #include "nodes/nodeFuncs.h" -#include "parser/parse_clause.h" #include "utils/array.h" #include "utils/builtins.h" #include "mb/pg_wchar.h" @@ -551,31 +550,32 @@ varcharsend(PG_FUNCTION_ARGS) /* - * Flatten calls to our length coercion function that leave the new maximum - * length >= the previous maximum length. We ignore the isExplicit argument, - * which only affects truncation. + * varchar_transform() + * Flatten calls to varchar's length coercion function that set the new maximum + * length >= the previous maximum length. We can ignore the isExplicit + * argument, since that only affects truncation cases. */ Datum varchar_transform(PG_FUNCTION_ARGS) { FuncExpr *expr = (FuncExpr *) PG_GETARG_POINTER(0); - Node *typmod; Node *ret = NULL; + Node *typmod; - if (!IsA(expr, FuncExpr)) - PG_RETURN_POINTER(ret); + Assert(IsA(expr, FuncExpr)); + Assert(list_length(expr->args) >= 2); - Assert(list_length(expr->args) == 3); - typmod = lsecond(expr->args); + typmod = (Node *) lsecond(expr->args); - if (IsA(typmod, Const)) + if (IsA(typmod, Const) && !((Const *) typmod)->constisnull) { - Node *source = linitial(expr->args); + Node *source = (Node *) linitial(expr->args); + int32 old_typmod = exprTypmod(source); int32 new_typmod = DatumGetInt32(((Const *) typmod)->constvalue); - int32 old_max = exprTypmod(source) - VARHDRSZ; + int32 old_max = old_typmod - VARHDRSZ; int32 new_max = new_typmod - VARHDRSZ; - if (new_max < 0 || (old_max >= 0 && old_max <= new_max)) + if (new_typmod < 0 || (old_typmod >= 0 && old_max <= new_max)) ret = relabel_to_typmod(source, new_typmod); } diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index 2db848903c4..49b0754a2ca 100644 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -767,7 +767,7 @@ DATA(insert OID = 668 ( bpchar PGNSP PGUID 12 1 0 0 0 f f f f t f i 3 0 10 DESCR("adjust char() to typmod length"); DATA(insert OID = 3097 ( varchar_transform PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 2281 "2281" _null_ _null_ _null_ _null_ varchar_transform _null_ _null_ _null_ )); DESCR("transform a varchar length coercion"); -DATA(insert OID = 669 ( varchar PGNSP PGUID 12 1 0 0 3097 f f f f t f i 3 0 1043 "1043 23 16" _null_ _null_ _null_ _null_ varchar _null_ _null_ _null_ )); +DATA(insert OID = 669 ( varchar PGNSP PGUID 12 1 0 0 varchar_transform f f f f t f i 3 0 1043 "1043 23 16" _null_ _null_ _null_ _null_ varchar _null_ _null_ _null_ )); DESCR("adjust varchar() to typmod length"); DATA(insert OID = 676 ( mktinterval PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 704 "702 702" _null_ _null_ _null_ _null_ mktinterval _null_ _null_ _null_ )); @@ -1267,7 +1267,7 @@ DESCR("date difference preserving months and years"); DATA(insert OID = 3918 ( interval_transform PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 2281 "2281" _null_ _null_ _null_ _null_ interval_transform _null_ _null_ _null_ )); DESCR("transform an interval length coercion"); -DATA(insert OID = 1200 ( interval PGNSP PGUID 12 1 0 0 3918 f f f f t f i 2 0 1186 "1186 23" _null_ _null_ _null_ _null_ interval_scale _null_ _null_ _null_ )); +DATA(insert OID = 1200 ( interval PGNSP PGUID 12 1 0 0 interval_transform f f f f t f i 2 0 1186 "1186 23" _null_ _null_ _null_ _null_ interval_scale _null_ _null_ _null_ )); DESCR("adjust interval precision"); DATA(insert OID = 1215 ( obj_description PGNSP PGUID 14 100 0 0 0 f f f f t f s 2 0 25 "26 19" _null_ _null_ _null_ _null_ "select description from pg_catalog.pg_description where objoid = $1 and classoid = (select oid from pg_catalog.pg_class where relname = $2 and relnamespace = PGNSP) and objsubid = 0" _null_ _null_ _null_ )); @@ -2023,7 +2023,7 @@ DATA(insert OID = 1685 ( bit PGNSP PGUID 12 1 0 0 0 f f f f t f i 3 0 1560 DESCR("adjust bit() to typmod length"); DATA(insert OID = 3158 ( varbit_transform PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 2281 "2281" _null_ _null_ _null_ _null_ varbit_transform _null_ _null_ _null_ )); DESCR("transform a varbit length coercion"); -DATA(insert OID = 1687 ( varbit PGNSP PGUID 12 1 0 0 3158 f f f f t f i 3 0 1562 "1562 23 16" _null_ _null_ _null_ _null_ varbit _null_ _null_ _null_ )); +DATA(insert OID = 1687 ( varbit PGNSP PGUID 12 1 0 0 varbit_transform f f f f t f i 3 0 1562 "1562 23 16" _null_ _null_ _null_ _null_ varbit _null_ _null_ _null_ )); DESCR("adjust varbit() to typmod length"); DATA(insert OID = 1698 ( position PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 23 "1560 1560" _null_ _null_ _null_ _null_ bitposition _null_ _null_ _null_ )); @@ -2156,10 +2156,10 @@ DATA(insert OID = 2917 ( numerictypmodin PGNSP PGUID 12 1 0 0 0 f f f f t f i DESCR("I/O typmod"); DATA(insert OID = 2918 ( numerictypmodout PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 2275 "23" _null_ _null_ _null_ _null_ numerictypmodout _null_ _null_ _null_ )); DESCR("I/O typmod"); -DATA(insert OID = 1703 ( numeric PGNSP PGUID 12 1 0 0 3157 f f f f t f i 2 0 1700 "1700 23" _null_ _null_ _null_ _null_ numeric _null_ _null_ _null_ )); -DESCR("adjust numeric to typmod precision/scale"); DATA(insert OID = 3157 ( numeric_transform PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 2281 "2281" _null_ _null_ _null_ _null_ numeric_transform _null_ _null_ _null_ )); DESCR("transform a numeric length coercion"); +DATA(insert OID = 1703 ( numeric PGNSP PGUID 12 1 0 0 numeric_transform f f f f t f i 2 0 1700 "1700 23" _null_ _null_ _null_ _null_ numeric _null_ _null_ _null_ )); +DESCR("adjust numeric to typmod precision/scale"); DATA(insert OID = 1704 ( numeric_abs PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 1700 "1700" _null_ _null_ _null_ _null_ numeric_abs _null_ _null_ _null_ )); DATA(insert OID = 1705 ( abs PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 1700 "1700" _null_ _null_ _null_ _null_ numeric_abs _null_ _null_ _null_ )); DESCR("absolute value"); @@ -2746,7 +2746,10 @@ DESCR("less-equal-greater"); DATA(insert OID = 3917 ( timestamp_transform PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 2281 "2281" _null_ _null_ _null_ _null_ timestamp_transform _null_ _null_ _null_ )); DESCR("transform a timestamp length coercion"); -DATA(insert OID = 1961 ( timestamp PGNSP PGUID 12 1 0 0 3917 f f f f t f i 2 0 1114 "1114 23" _null_ _null_ _null_ _null_ timestamp_scale _null_ _null_ _null_ )); +DATA(insert OID = 3944 ( time_transform PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 2281 "2281" _null_ _null_ _null_ _null_ time_transform _null_ _null_ _null_ )); +DESCR("transform a time length coercion"); + +DATA(insert OID = 1961 ( timestamp PGNSP PGUID 12 1 0 0 timestamp_transform f f f f t f i 2 0 1114 "1114 23" _null_ _null_ _null_ _null_ timestamp_scale _null_ _null_ _null_ )); DESCR("adjust timestamp precision"); DATA(insert OID = 1965 ( oidlarger PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 26 "26 26" _null_ _null_ _null_ _null_ oidlarger _null_ _null_ _null_ )); @@ -2754,13 +2757,11 @@ DESCR("larger of two"); DATA(insert OID = 1966 ( oidsmaller PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 26 "26 26" _null_ _null_ _null_ _null_ oidsmaller _null_ _null_ _null_ )); DESCR("smaller of two"); -DATA(insert OID = 1967 ( timestamptz PGNSP PGUID 12 1 0 0 3917 f f f f t f i 2 0 1184 "1184 23" _null_ _null_ _null_ _null_ timestamptz_scale _null_ _null_ _null_ )); +DATA(insert OID = 1967 ( timestamptz PGNSP PGUID 12 1 0 0 timestamp_transform f f f f t f i 2 0 1184 "1184 23" _null_ _null_ _null_ _null_ timestamptz_scale _null_ _null_ _null_ )); DESCR("adjust timestamptz precision"); -DATA(insert OID = 3944 ( time_transform PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 2281 "2281" _null_ _null_ _null_ _null_ time_transform _null_ _null_ _null_ )); -DESCR("transform a time length coercion"); -DATA(insert OID = 1968 ( time PGNSP PGUID 12 1 0 0 3944 f f f f t f i 2 0 1083 "1083 23" _null_ _null_ _null_ _null_ time_scale _null_ _null_ _null_ )); +DATA(insert OID = 1968 ( time PGNSP PGUID 12 1 0 0 time_transform f f f f t f i 2 0 1083 "1083 23" _null_ _null_ _null_ _null_ time_scale _null_ _null_ _null_ )); DESCR("adjust time precision"); -DATA(insert OID = 1969 ( timetz PGNSP PGUID 12 1 0 0 3944 f f f f t f i 2 0 1266 "1266 23" _null_ _null_ _null_ _null_ timetz_scale _null_ _null_ _null_ )); +DATA(insert OID = 1969 ( timetz PGNSP PGUID 12 1 0 0 time_transform f f f f t f i 2 0 1266 "1266 23" _null_ _null_ _null_ _null_ timetz_scale _null_ _null_ _null_ )); DESCR("adjust time with time zone precision"); DATA(insert OID = 2003 ( textanycat PGNSP PGUID 14 1 0 0 0 f f f f t f s 2 0 25 "25 2776" _null_ _null_ _null_ _null_ "select $1 || $2::pg_catalog.text" _null_ _null_ _null_ )); diff --git a/src/include/nodes/nodeFuncs.h b/src/include/nodes/nodeFuncs.h index def4f313ab7..e609e4bae20 100644 --- a/src/include/nodes/nodeFuncs.h +++ b/src/include/nodes/nodeFuncs.h @@ -29,6 +29,7 @@ extern Oid exprType(const Node *expr); extern int32 exprTypmod(const Node *expr); extern bool exprIsLengthCoercion(const Node *expr, int32 *coercedTypmod); +extern Node *relabel_to_typmod(Node *expr, int32 typmod); extern bool expression_returns_set(Node *clause); extern Oid exprCollation(const Node *expr); diff --git a/src/include/parser/parse_clause.h b/src/include/parser/parse_clause.h index f8eb41aad45..fd3fc8f5709 100644 --- a/src/include/parser/parse_clause.h +++ b/src/include/parser/parse_clause.h @@ -44,6 +44,4 @@ extern List *transformDistinctOnClause(ParseState *pstate, List *distinctlist, extern Index assignSortGroupRef(TargetEntry *tle, List *tlist); extern bool targetIsInSortList(TargetEntry *tle, Oid sortop, List *sortList); -extern Node *relabel_to_typmod(Node *expr, int32 typmod); - #endif /* PARSE_CLAUSE_H */ diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out index 7815b0d7be4..110ea4111f9 100644 --- a/src/test/regress/expected/opr_sanity.out +++ b/src/test/regress/expected/opr_sanity.out @@ -330,6 +330,16 @@ WHERE proargmodes IS NOT NULL AND proargnames IS NOT NULL AND -----+--------- (0 rows) +-- Check for protransform functions with the wrong signature +SELECT p1.oid, p1.proname, p2.oid, p2.proname +FROM pg_proc AS p1, pg_proc AS p2 +WHERE p2.oid = p1.protransform AND + (p2.prorettype != 'internal'::regtype OR p2.proretset OR p2.pronargs != 1 + OR p2.proargtypes[0] != 'internal'::regtype); + oid | proname | oid | proname +-----+---------+-----+--------- +(0 rows) + -- Insist that all built-in pg_proc entries have descriptions SELECT p1.oid, p1.proname FROM pg_proc as p1 LEFT JOIN pg_description as d diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql index 3ed6366bfc0..a3be0c1114f 100644 --- a/src/test/regress/sql/opr_sanity.sql +++ b/src/test/regress/sql/opr_sanity.sql @@ -256,6 +256,13 @@ FROM pg_proc as p1 WHERE proargmodes IS NOT NULL AND proargnames IS NOT NULL AND array_length(proargmodes,1) <> array_length(proargnames,1); +-- Check for protransform functions with the wrong signature +SELECT p1.oid, p1.proname, p2.oid, p2.proname +FROM pg_proc AS p1, pg_proc AS p2 +WHERE p2.oid = p1.protransform AND + (p2.prorettype != 'internal'::regtype OR p2.proretset OR p2.pronargs != 1 + OR p2.proargtypes[0] != 'internal'::regtype); + -- Insist that all built-in pg_proc entries have descriptions SELECT p1.oid, p1.proname FROM pg_proc as p1 LEFT JOIN pg_description as d -- GitLab