On Tue, Jan 14, 2014 at 06:04:47PM +0900, Kyotaro HORIGUCHI wrote:
> I proposed 3 types of solution but the one of them - tweaking
> Equivalence Class (Type 1)- was dismissed because of
> inappropriate manipulation on Equivalence Class. The rest do
> essentially the same thing - flattening appendrels - at different
> timings.
>
> In type 2, the process is implemented as a generic appendrel
> flattening function and applied after expand_inherited_tables()
> in subquery_planner.
>
> In type 3, the equivelant process is combined in
> expand_inherited_rtentry().
>
> Type 2 loops once for whole appendrel list (in most cases) so it
> might be more effective than type 3 which searches the parent
> appendrel for each appended ones. Type 3 is more approprieate
> design assuming that appendrel tree deeper than 2 levels will be
> generated only by expand_inherited_tables().
Let's focus on type 3; Tom and I both found it most promising.
> 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
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.
...
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
...
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.
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com