Subject: Re: "SELECT ... FROM DUAL" is not quite as silly as it appears
To: pgsql-hackers@lists.postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>
Cc: Mark Dilger <hornschnorter@gmail.com>
References: <15944.1521127664@sss.pgh.pa.us>
 <154342395130.2093.8013327151440360209.pgcf@coridan.postgresql.org>
 <154342412161.2094.14859341952034622342.pgcf@coridan.postgresql.org>
From: Alexander Kuzmenkov <a.kuzmenkov@postgrespro.ru>
Message-ID: <6d51c9bb-ccc6-92fd-b6ac-05f0d5546d70@postgrespro.ru>
Date: Thu, 29 Nov 2018 15:08:31 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: 
 <154342412161.2094.14859341952034622342.pgcf@coridan.postgresql.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk

I was also looking at this patch, here are some things I noticed:

In remove_useless_results_recurse where it processes JOIN_SEMI there is 
this comment:

                  * However, we can't simplify if there are PHVs to 
evaluate at
                  * the RTE_RESULT ... but that's impossible isn't it?

Is that impossible because the RHS of semi join can't be used above it? 
Let's write this down. There is similar code above for JOIN_LEFT and it 
does have to check for PHVs, so a comment that clarifies the reasons for 
the difference would help.


Also around there:

                 if ((varno = is_result_ref(root, j->rarg)) != 0 &&

I'd expect a function that starts with "is_" to return a bool, so this 
catches the eye. Maybe varno = get_result_relid()?


Looking at the coverage report of regression tests, most of the new code 
is covered except for the aforementioned simplification of JOIN_LEFT and 
JOIN_RIGHT, but it's probably not worth adding a special test. I checked 
these cases manually and they work OK.


I also repeated the benchmark with trivial select and can confirm that 
there is no change in performance.

-- 
Alexander Kuzmenkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company