Re: Parameterized aggregate subquery (was: Pull up aggregate subquery)

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