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'll try to solve the latter issue.

--
Yoshikazu Imai