Fix use of uninitialized variable in inline_function().
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 25 May 2021 16:55:52 +0000 (12:55 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 25 May 2021 16:55:55 +0000 (12:55 -0400)
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

index e117ab976e62ba952a90b92b8ebc595b1230bb75..517712a8f4c5cc6eccb345c7f3b8540ae8ae1b1d 100644 (file)
@@ -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