RE: speeding up planning with partitions

Amit-san,

On Tue, Mar 5, 2019 at 10:24 AM, Amit Langote wrote:
> On 2019/03/05 9:50, Amit Langote wrote:
> > I'll post the updated patches after diagnosing what I'm suspecting a
> > memory over-allocation bug in one of the patches.  If you configure
> > build with --enable-cassert, you'll see that throughput decreases as
> > number of partitions run into many thousands, but it doesn't when
> > asserts are turned off.
> 
> Attached an updated version.  This incorporates fixes for both Jesper's
> and Imai-san's review.

Thanks for updating patches!
Here is the code review for previous v26 patches.

[0002]
In expand_inherited_rtentry():

expand_inherited_rtentry()
{
    ...
+   RelOptInfo *rel = NULL;

can be declared at more later:

if (oldrelation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
...
else
{   
    List       *appinfos = NIL;
    RangeTblEntry *childrte;
    Index       childRTindex;
+    RelOptInfo *rel = NULL;


[0003]
In inheritance_planner:

+    rtable_with_target = list_copy(root->parse->rtable);

can be:

+    rtable_with_target = list_copy(parse->rtable);

[0004 or 0005]
There are redundant process in add_appendrel_other_rels (or expand_xxx_rtentry()?).
I modified add_appendrel_other_rels like below and it actually worked.


add_appendrel_other_rels(PlannerInfo *root, RangeTblEntry *rte, Index rti) 
{
    ListCell       *l;
    RelOptInfo     *rel;

    /*   
     * Add inheritance children to the query if not already done. For child
     * tables that are themselves partitioned, their children will be added
     * recursively.
     */
    if (rte->rtekind == RTE_RELATION && !root->contains_inherit_children)
    {    
        expand_inherited_rtentry(root, rte, rti);
        return;
    }    

    rel = find_base_rel(root, rti);

    foreach(l, root->append_rel_list)
    {    
        AppendRelInfo  *appinfo = lfirst(l);
        Index           childRTindex = appinfo->child_relid;
        RangeTblEntry  *childrte;
        RelOptInfo     *childrel;

        if (appinfo->parent_relid != rti) 
                continue;

        Assert(childRTindex < root->simple_rel_array_size);
        childrte = root->simple_rte_array[childRTindex];
        Assert(childrte != NULL);
        build_simple_rel(root, childRTindex, rel);

        /* Child may itself be an inherited relation. */
        if (childrte->inh)
            add_appendrel_other_rels(root, childrte, childRTindex);
    }    
}

> and Imai-san's review.  I haven't been able to pin down the bug (or
> whatever) that makes throughput go down as the partition count increases,
> when tested with a --enable-cassert build.

I didn't investigate that problem, but there is another memory increase issue, which is because of 0003 patch I think.
I'lltry to solve the latter issue.
 

--
Yoshikazu Imai