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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
