Re: ExecTypeSetColNames is fundamentally broken

2022-03-17 Thread Tom Lane
Aleksander Alekseev writes: > I understand the concern expressed by Robert in respect of backward > compatibility. From the user's perspective, personally I would prefer > a fixed bug over backward compatibility. Especially if we consider the > fact that the current behaviour of cases like

Re: ExecTypeSetColNames is fundamentally broken

2022-03-16 Thread Aleksander Alekseev
Hi hackers, > Anyone else have thoughts? I came across this thread while looking for the patches that need review. My understanding of the code is limited, but I can say that I don't see anything particularly wrong with it. I can also confirm that it fixes the problem reported by the user while

Re: ExecTypeSetColNames is fundamentally broken

2022-03-15 Thread Robert Haas
On Tue, Dec 7, 2021 at 1:19 PM Tom Lane wrote: > So the alternatives I see are to revert what bf7ca1587 tried to do > here, or to try to make it work that way across-the-board, which > implies (a) a very much larger amount of work, and (b) breaking > important behaviors that are decades older

Re: ExecTypeSetColNames is fundamentally broken

2021-12-07 Thread Tom Lane
Robert Haas writes: > On Tue, Dec 7, 2021 at 12:30 PM Tom Lane wrote: >> If we consider that the alias renames the columns "for all purposes", >> how is it okay for f() to select the "a" column? > I'd say it isn't. In a green field I'd probably agree with you, but IMO that will break far too

Re: ExecTypeSetColNames is fundamentally broken

2021-12-07 Thread Robert Haas
On Tue, Dec 7, 2021 at 12:30 PM Tom Lane wrote: > If we consider that the alias renames the columns "for all purposes", > how is it okay for f() to select the "a" column? I'd say it isn't. > Another way to phrase the issue is that the column names seen > by f() are currently different from

Re: ExecTypeSetColNames is fundamentally broken

2021-12-07 Thread Tom Lane
Robert Haas writes: > On Mon, Dec 6, 2021 at 4:05 PM Tom Lane wrote: >> select f(t) from t(x,y); >> >> If we adopt the "rename for all purposes" interpretation, then >> the second SELECT must fail, because what f() is being passed is >> no longer of type t. > For me, the second SELECT does

Re: ExecTypeSetColNames is fundamentally broken

2021-12-07 Thread Robert Haas
On Mon, Dec 6, 2021 at 4:05 PM Tom Lane wrote: > Well, that was what I thought when I wrote bf7ca1587, but it leads > to logical contradictions. Consider > > create table t (a int, b int); > > create function f(t) returns ... ; > > select f(t) from t; > > select f(t) from t(x,y); > > If we adopt

Re: ExecTypeSetColNames is fundamentally broken

2021-12-06 Thread Tom Lane
Robert Haas writes: > I don't understand the code so I can't comment on the code, but I find > the regression test changes pretty suspect. Attaching any alias list > to the RTE ought to rename the output columns for all purposes, not > just the ones we as implementers find convenient. Well, that

Re: ExecTypeSetColNames is fundamentally broken

2021-12-06 Thread Robert Haas
On Sun, Dec 5, 2021 at 1:46 PM Tom Lane wrote: > So 0001 attached fixes this by revoking the decision to apply > ExecTypeSetColNames in cases where a Var or RowExpr is declared > to return a named composite type. This causes a couple of regression > test results to change, but they are ones that

ExecTypeSetColNames is fundamentally broken

2021-12-05 Thread Tom Lane
ExecTypeSetColNames would do the wrong thing. I made several attempts to work around this problem, but I've now concluded that changing the type OID in ExecTypeSetColNames is just fundamentally broken. It can't be okay to decide that a Var that claims to emit type A actually emits type B