RE: speeding up planning with partitions

Amit-san,

On Fri, Mar 8, 2019 at 9:18 AM, Amit Langote wrote:
> On 2019/03/08 16:16, Imai, Yoshikazu wrote:
> > So I modified the code and did test to confirm memory increasement don't
> happen. The test and results are below.
> >
> > [test]
> > * Create partitioned table with 1536 partitions.
> > * Execute "update rt set a = random();"
> >
> > [results]
> > A backend uses below amount of memory in update transaction:
> >
> > HEAD: 807MB
> > With v26-0001, 0002: 790MB
> > With v26-0001, 0002, 0003: 860MB
> > With v26-0003 modified: 790MB
> 
> Can you measure with v28, or better attached v29 (about which more below)?
> 
> > I attached the diff of modification for v26-0003 patch which also
> contains some refactoring.
> > Please see if it is ok.
> 
> I like the is_first_child variable which somewhat improves readability,
> so updated the patch to use it.
> 
> Maybe you know that range_table_mutator() spends quite a long time if
> there are many target children, but I realized there's no need for
> range_table_mutator() to copy/mutate child target RTEs.  First, there's
> nothing to translate in their case.  Second, copying them is not necessary
> too, because they're not modified across different planning cycles.  If
> we pass to adjust_appendrel_attrs only the RTEs in the original range
> table (that is, before any child target RTEs were added), then
> range_table_mutator() has to do significantly less work and allocates
> lot less memory than before.  I've broken this change into its own patch;
> see patch 0004.

Cool!
I tested with v29 patches and checked it saved a lot of memory..

HEAD: 807MB
With v29-0001, 0002, 0003, 0004: 167MB

Maybe planning time in this case is also improved, but I didn't check it.


> but I realized there's no need for
> range_table_mutator() to copy/mutate child target RTEs.  First, there's
> nothing to translate in their case.  Second, copying them is not necessary
> too, because they're not modified across different planning cycles.

Yeah, although I couldn't check the codes in detail, but from the below comments in inheritance_planner(), ISTM we need
copiesof subquery RTEs but need not copies of other RTEs in each planning.
 

    /*   
     * We generate a modified instance of the original Query for each target
     * relation, plan that, and put all the plans into a list that will be
     * controlled by a single ModifyTable node.  All the instances share the
     * same rangetable, but each instance must have its own set of subquery
     * RTEs within the finished rangetable because (1) they are likely to get
     * scribbled on during planning, and (2) it's not inconceivable that
     * subqueries could get planned differently in different cases.  We need
     * not create duplicate copies of other RTE kinds, in particular not the
     * target relations, because they don't have either of those issues.  Not
     * having to duplicate the target relations is important because doing so
     * (1) would result in a rangetable of length O(N^2) for N targets, with
     * at least O(N^3) work expended here; and (2) would greatly complicate
     * management of the rowMarks list.


Thanks 
--
Yoshikazu Imai