On Tue, Jan 28, 2014 at 06:53:32PM +0900, Kyotaro HORIGUCHI wrote: > Hello, Thank you, and I' sorry to have kept you waiting.
No hurry. > > > 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? I would still like to know the answer to the above question. > > [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); It's true that each AppendRelInfo is initially created that way, but they need not remain so simple. You can assume ctx->child_appinfo->translated_vars contains only these Var nodes, but parent_appinfo->translated_vars may contain arbitrary expressions. (I think the pullup_replace_vars() call at prepjointree.c:954 is where an AppendRelInfo can get non-Var expressions.) > 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. Here is a test case that provokes an assertion failure under the patch; I believe the cause is oversimplification in transvars_merge_mutator(): create table parent (a int, b int); create table child () inherits (parent); select parent from parent union all select parent from parent; While poking at that test case, I noticed that the following test emits three rows on HEAD and wrongly emits four rows with the patch: create table parent (a int, b int); create table child () inherits (parent); insert into parent values (1,10); insert into child values (2,20); select a, b from parent union all select a, b from child; -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
