I think that's mostly a kluge. It seems to me that the actual problem is that 2ed8f9a01 missed some places that need fixing. The policy that that commit intended to institute is "if a RangeTblFunction has a coldeflist, then the function return type is certainly RECORD, and we don't need to try to dig the column specifications out of the function expression" (which dodges the problem that the function expression might not produce what we're expecting). However, expandRTE didn't get that memo, and would produce the wrong tupdesc in an example such as this one. IMO, it ought to produce the query-specified columns in all cases, even when the function expression doesn't match.
+1. This seems a more principled fix.
I dug around looking at other callers of get_expr_result_type and get_expr_result_tupdesc, and found a couple other places that aren't actively buggy but can be made faster and more robust by adding this same rule.
I wonder why the v2 patch does not apply this same rule in process_function_rte_ref(), by checking if the 'rtfunc' has a coldeflist before it calls get_expr_result_tupdesc.
BTW, I wondered why the test cases we added for 2ed8f9a01 didn't catch this, because they sure look superficially similar. The reason seems to be that we fail because this query produces a join plan in which we invoke build_physical_tlist for the FunctionScan, and that's what's calling expandRTE and hitting the assertion. The older test cases get simplified sufficiently that there's no join but just a FunctionScan, and that plan node will be required to produce the query-specified tlist not a physical tlist, so we accidentally avoid reaching the assertion.
