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
