Skip site navigation (1) Skip section navigation (2)

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

From: Yeb Havinga <yebhavinga(at)gmail(dot)com>
To: Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Parameterized aggregate subquery (was: Pull up aggregate subquery)
Date: 2011-06-29 13:44:18
Message-ID: 4E0B2C32.6010808@gmail.com (view raw or flat)
Thread:
Lists: 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


In response to

Responses

pgsql-hackers by date

Next:From: Simon RiggsDate: 2011-06-29 14:09:06
Subject: Re: Process local hint bit cache
Previous:From: Robert HaasDate: 2011-06-29 13:43:57
Subject: Re: Process local hint bit cache

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group