Re: UNION ALL on partitioned tables won't use indices.

Hello, Thank you, and I' sorry to have kept you waiting.

> Let's focus on type 3; Tom and I both found it most promising.

Agreed.

> > The attached two patches are rebased to current 9.4dev HEAD and
> > make check at the topmost directory and src/test/isolation are
> > passed without error. One bug was found and fixed on the way. It
> > was an assertion failure caused by probably unexpected type
> > conversion introduced by collapse_appendrels() which leads
> > implicit whole-row cast from some valid reltype to invalid
> > reltype (0) into adjust_appendrel_attrs_mutator().
>
> What query demonstrated this bug in the previous type 2/3 patches?
>
> > - unionall_inh_idx_typ3_v4_20140114.patch
>
> You have not addressed my review comments from November:
> http://www.postgresql.org/message-id/20131123073913.GA1008138@tornado.leadboat.com

mmm. Sorry, I've overlooked most of it..

em_is_child is no more a issue, and the rest seem to cited below, thanks.

> Specifically, these:
>
> [transvar_merge_mutator()] has roughly the same purpose as
> adjust_appendrel_attrs_mutator(), but it propagates the change to far fewer
> node types. Why this case so much simpler? The parent translated_vars of
> parent_appinfo may bear mostly-arbitrary expressions.

There are only two places where AppendRelInfo node is generated,
expand_inherited_rtentry and
pull_up_union_leaf_queries.(_copyAppendrelInfo is irrelevant to
this discussion.) The core part generating translated_vars for
them are make_inh_translation_list and
make_setop_translation_list respectively. That's all. And they
are essentially works on the same way, making a Var for every
referred target entry of children like following.

| makeVar(varno,
| tle->resno,
| exprType((Node *) tle->expr),
| exprTypmod((Node *) tle->expr),
| exprCollation((Node *) tle->expr),
| 0);

So all we should do to collapse nested appendrels is simplly
connecting each RTEs directly to the root (most ancient?) RTE in
the relationship, resolving Vars like above, as seen in patched
expand_inherited_rtentry.

# If translated_vars are generated always in the way shown above,
# using tree walker might be no use..

This can be done apart from all other stuffs compensating
translation skew done in adjust_appendrel_attrs. I believe they
are in orthogonal relationship.

> Approaches (2) and (3) leave the inheritance parent with rte->inh == true
> despite no AppendRelInfo pointing to it as their parent. Until now,
> expand_inherited_rtentry() has been careful to clear rte->inh in such cases.

Thank you. I missed that point. rte->inh at first works as a
trigger to try expand inheritance and create append_rel_list
entries, and later works to say "dig me through appinfos". From
that point of view, inh of the RTEs whose children took away
should be 0. The two meanings of inh are now become different
from each other so I suppose it'd better rename it, but I don't
come up with good alternatives..

Anyway this is corrected in the patch attached and works as
follows,

BEFORE:
rte[1] Subquery "SELECT*1", inh = 1
+- appinfo[0] - rte[4] Relation "p1", inh = 1
| +- appinfo[2] - rte[6] Relation "p1", inh = 0
| +- appinfo[3] - rte[7] Relation "c11", inh = 0
| +- appinfo[4] - rte[8] Relation "c12", inh = 0
+- appinfo[1] - rte[5] Relation "p2", inh = 1
+- appinfo[5] - rte[9] Relation "p1", inh = 0
+- appinfo[6] - rte[10] Relation "c11", inh = 0
+- appinfo[7] - rte[11] Relation "c12", inh = 0

COLLAPSED:
rte[1] Subquery "SELECT*1", inh = 1
+- appinfo[0] - rte[4] Relation "p1", inh = 1 => 0
+- appinfo[2] - rte[6] Relation "p1", inh = 0
+- appinfo[3] - rte[7] Relation "c11", inh = 0
+- appinfo[4] - rte[8] Relation "c12", inh = 0
+- appinfo[1] - rte[5] Relation "p2", inh = 1 => 0
+- appinfo[5] - rte[9] Relation "p1", inh = 0
+- appinfo[6] - rte[10] Relation "c11", inh = 0
+- appinfo[7] - rte[11] Relation "c12", inh = 0

> I get this warning:
>
> prepunion.c: In function `expand_inherited_rtentry':
> prepunion.c:1450: warning: passing argument 1 of `expression_tree_mutator' from incompatible pointer type

Sorry, I forgot to put a casting to generic type. It is fixed in
the attached version.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center