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

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index e117ab976e62ba952a90b92b8ebc595b1230bb75..517712a8f4c5cc6eccb345c7f3b8540ae8ae1b1d 100644 (file)
--- 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