Subject: Re: Allow an alias to be attached directly to a JOIN ... USING
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Steele <david@pgmasters.net>,
 Georgios Kokolatos <gkokolatos@protonmail.com>,
 pgsql-hackers@lists.postgresql.org
References: <454638cf-d563-ab76-a585-2564428062af@2ndquadrant.com>
 <88018eae-d949-be66-7fec-924213213b49@technowledgy.de>
 <160502135578.7362.9015197269135176959.pgcf@coridan.postgresql.org>
 <4c4727e2-2797-7f02-3529-f504e843d2ea@2ndquadrant.com>
 <19cab8ea-7106-2017-e018-d65feffb90ad@pgmasters.net>
 <64f2b50d-861f-ee4f-4272-e668cee5a154@2ndquadrant.com>
 <785329.1616455091@sss.pgh.pa.us>
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Message-ID: <3d945d71-25d8-22b9-cc4e-aaad817eeda3@2ndquadrant.com>
Date: Wed, 31 Mar 2021 17:49:01 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.9.0
MIME-Version: 1.0
In-Reply-To: <785329.1616455091@sss.pgh.pa.us>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/3d945d71-25d8-22b9-cc4e-aaad817eeda3%402ndquadrant.com>
Precedence: bulk


On 23.03.21 00:18, Tom Lane wrote:
> However, ParseNamespaceItem as it stands needs some help for this.
> It has a wired-in assumption that p_rte->eref describes the table
> and column aliases exposed by the nsitem.  0001 below fixes this by
> creating a separate p_names field in an nsitem.  (There are some
> comments in 0001 referencing JOIN USING aliases, but no actual code
> for the feature.)  That saves one indirection in common code paths,
> so it's possibly a win on its own.  Then 0002 is your patch rebased
> onto that infrastructure, and with some cleanup of my own.

Makes sense.  I've committed it based on that.

> Speaking of decompiled views, I feel like ruleutils.c could do with
> a little more work to teach it that these aliases are available.
> Right now, it resorts to ugly workarounds:

Yeah, the whole has_dangerous_join_using() can probably be unwound and 
removed with this.  But it's a bit of work.

> One other cosmetic thing is that this:
> 
> regression=# select tu.* from (t1 join t2 using(a) as tu) tx;
> ERROR:  missing FROM-clause entry for table "tu"
> LINE 1: select tu.* from (t1 join t2 using(a) as tu) tx;
>                 ^
> 
> is a relatively dumb error message, compared to
> 
> regression=# select t1.* from (t1 join t2 using(a) as tu) tx;
> ERROR:  invalid reference to FROM-clause entry for table "t1"
> LINE 1: select t1.* from (t1 join t2 using(a) as tu) tx;
>                 ^
> HINT:  There is an entry for table "t1", but it cannot be referenced from this part of the query.
> 
> I didn't look into why that isn't working, but maybe errorMissingRTE
> needs to trawl all of the ParseNamespaceItems not just the RTEs.

Yes, I've prototyped that and it would have the desired effect.  Might 
need some code rearranging, like either change searchRangeTableForRel() 
to not return an RTE or make a similar function for ParseNamespaceItem 
search.  Needs some more thought.  I have left a test case in that would 
show any changes here.