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 copies of 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