Re: Parameterized aggregate subquery (was: Pull up aggregate subquery) - Mailing list pgsql-hackers

On 2011-06-17 09:54, Hitoshi Harada wrote:
> While reviewing the gist/box patch, I found some planner APIs that can
> replace parts in my patch. Also, comments in includes wasn't updated
> appropriately. Revised patch attached.

Hello Hitoshi-san,

I read your latest patch implementing parameterizing subquery scans.

1)
In the email from june 9 with the patch You wrote: "While IndexScan
is simple since its information like costs are well known by the base
relation, SubqueryScan should re-plan its Query to gain that, which is
expensive."

Initial concerns I had were caused by misinterpreting 'replanning' as: 
for each outer tuple, replan the subquery (it sounds a bit like 
'ReScan'). Though the general comments in the patch are helpful, I think 
it would be an improvement to describe why subqueries are planned more 
than once, i.e. something like
"best_inner_subqueryscan   Try to find a better subqueryscan path and its associated plan for 
each join clause that can be pushed down, in addition to the path that 
is already calculated by set_subquery_pathlist."

The consequences of having multiple subquery paths and plans for each 
new subquery path makes the bulk of the remainder of the patch.

2)
Since 'subquery_is_pushdown_safe' is invariant under join clause push 
down, it might be possible to have it called only once in 
set_subquery_pathlist, i.e. by only attaching rel->preprocessed_subquery 
if the subquery_is_pushdown_safe.

3)
/* * set_subquery_pathlist *        Build the (single) access path for a subquery RTE */
This unchanged comment is still correct, but 'the (single) access path' 
might have become a bit misleading now there are multiple paths 
possible, though not by set_subquery_pathlist.


4) somewhere in the patch
s/regsitered/registered/

5) Regression tests are missing; I think at this point they'd aid in 
speeding up development/test cycles.

6) Before patching Postgres, I could execute the test with the size_l 
and size_m tables, after patching against current git HEAD (patch 
without errors), I get the following error when running the example query:
ERROR:  plan should not reference subplan's variable

I get the same error with the version from june 9 with current git HEAD.

7) Since both set_subquery_pathlist and best_inner_subqueryscan push 
down clauses, as well as add a path and subplan, could a generalized 
function be made to support both, to reduce duplicate code?

regards,
Yeb Havinga

-- 
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data