From e30e3fdea873e4e9517c490232ea1d3bcef6c643 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 25 May 2021 12:55:52 -0400 Subject: [PATCH] Fix use of uninitialized variable in inline_function(). Commit e717a9a18 introduced a code path that bypassed the call of get_expr_result_type, which is not good because we need its rettupdesc result to pass to check_sql_fn_retval. We'd failed to notice right away because the code path in which check_sql_fn_retval uses that argument is fairly hard to reach in this context. It's not impossible though, and in any case inline_function would have no business assuming that check_sql_fn_retval doesn't need that value. To fix, move get_expr_result_type out of the if-block, which in turn requires moving the construction of the dummy FuncExpr out of it. Per report from Ranier Vilela. (I'm bemused by the lack of any compiler complaints...) Discussion: https://postgr.es/m/CAEudQAqBqQpQ3HruWAGU_7WaMJ7tntpk0T8k_dVtNB46DqdBgw@mail.gmail.com --- src/backend/optimizer/util/clauses.c | 44 ++++++++++++++-------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index e117ab976e..517712a8f4 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -4317,6 +4317,22 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid, ALLOCSET_DEFAULT_SIZES); oldcxt = MemoryContextSwitchTo(mycxt); + /* + * We need a dummy FuncExpr node containing the already-simplified + * arguments. (In some cases we don't really need it, but building it is + * cheap enough that it's not worth contortions to avoid.) + */ + fexpr = makeNode(FuncExpr); + fexpr->funcid = funcid; + fexpr->funcresulttype = result_type; + fexpr->funcretset = false; + fexpr->funcvariadic = funcvariadic; + fexpr->funcformat = COERCE_EXPLICIT_CALL; /* doesn't matter */ + fexpr->funccollid = result_collid; /* doesn't matter */ + fexpr->inputcollid = input_collid; + fexpr->args = args; + fexpr->location = -1; + /* Fetch the function body */ tmp = SysCacheGetAttr(PROCOID, func_tuple, @@ -4359,32 +4375,11 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid, } else { - /* - * Set up to handle parameters while parsing the function body. We - * need a dummy FuncExpr node containing the already-simplified - * arguments to pass to prepare_sql_fn_parse_info. (In some cases we - * don't really need that, but for simplicity we always build it.) - */ - fexpr = makeNode(FuncExpr); - fexpr->funcid = funcid; - fexpr->funcresulttype = result_type; - fexpr->funcretset = false; - fexpr->funcvariadic = funcvariadic; - fexpr->funcformat = COERCE_EXPLICIT_CALL; /* doesn't matter */ - fexpr->funccollid = result_collid; /* doesn't matter */ - fexpr->inputcollid = input_collid; - fexpr->args = args; - fexpr->location = -1; - + /* Set up to handle parameters while parsing the function body. */ pinfo = prepare_sql_fn_parse_info(func_tuple, (Node *) fexpr, input_collid); - /* fexpr also provides a convenient way to resolve a composite result */ - (void) get_expr_result_type((Node *) fexpr, - NULL, - &rettupdesc); - /* * We just do parsing and parse analysis, not rewriting, because * rewriting will not affect table-free-SELECT-only queries, which is @@ -4434,6 +4429,11 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid, list_length(querytree->targetList) != 1) goto fail; + /* If the function result is composite, resolve it */ + (void) get_expr_result_type((Node *) fexpr, + NULL, + &rettupdesc); + /* * Make sure the function (still) returns what it's declared to. This * will raise an error if wrong, but that's okay since the function would -- 2.39.5