diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 52dcc72..129354a 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -1223,8 +1223,17 @@ expand_inherited_tables(PlannerInfo *root) * table, but with inh = false, to represent the parent table in its role * as a simple member of the inheritance set. * + * Most commonly, the added AppendRelInfo nodes name the original RTE as their + * parent_relid, making the original RTE an appendrel parent. An exception + * arises when the original RTE is itself an appendrel member, as with + * "... UNION ALL SELECT * FROM inhparent" syntax. In such situations, + * preserve appendrel flatness by detaching the original RTE's AppendRelInfo + * and pointing each new parent_relid to the existing UNION ALL appendrel. + * The original RTE remains in the range table for permission-checking + * purposes only. + * * A childless table is never considered to be an inheritance set; therefore - * a parent RTE must always have at least two associated AppendRelInfos. + * a parent RTE must never have exactly one associated AppendRelInfo. */ static void expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) @@ -1237,6 +1246,8 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) List *inhOIDs; List *appinfos; ListCell *l; + ListCell *prev; + AppendRelInfo *parent_appinfo = NULL; /* Does RT entry allow inheritance? */ if (!rte->inh) @@ -1300,6 +1311,27 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) if (oldrc) oldrc->isParent = true; + /* Find and detach from an existing UNION ALL parent appendrel, if any. */ + prev = NULL; + foreach(l, root->append_rel_list) + { + AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(l); + + if (appinfo->child_relid == rti) + { + /* + * A PlanRowMark should have blocked pull-up of the UNION ALL + * subquery referencing this inheritance parent table. + */ + Assert(!oldrc); + + parent_appinfo = appinfo; + list_delete_cell(root->append_rel_list, l, prev); + break; + } + prev = l; + } + /* * Must open the parent relation to examine its tupdesc. We need not lock * it; we assume the rewriter already did. @@ -1361,9 +1393,10 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) appinfos = lappend(appinfos, appinfo); /* - * Translate the column permissions bitmaps to the child's attnums (we - * have to build the translated_vars list before we can do this). But - * if this is the parent table, leave copyObject's result alone. + * Translate the column permissions bitmaps to the child's attnums. We + * must do this after building the initial translated_vars and before + * replacing it with translated_vars derived from a parent appendrel. + * But if this is the parent table, leave copyObject's result alone. * * Note: we need to do this even though the executor won't run any * permissions checks on the child RTE. The modifiedCols bitmap may @@ -1377,6 +1410,30 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) appinfo->translated_vars); } + if (parent_appinfo) + { + Node *parent_tv = (Node *) parent_appinfo->translated_vars; + Node *child_tv; + + /* + * Adjust the inheritance parent table's UNION ALL translated_vars + * to suit this child. Drive it using the appinfo we constructed + * based on the inheritance parent table being the appendrel + * parent; this is like copying a parent targetlist to a child. + */ + child_tv = adjust_appendrel_attrs(root, parent_tv, appinfo); + + /* + * Adapt appinfo to its final appendrel parent. We could leave + * child_reltype valid, but no code would presently benefit. + */ + Assert(!OidIsValid(parent_appinfo->parent_reltype)); + appinfo->parent_relid = parent_appinfo->parent_relid; + appinfo->parent_reltype = InvalidOid; + appinfo->child_reltype = InvalidOid; + appinfo->translated_vars = (List *) child_tv; + } + /* * Build a PlanRowMark if parent is marked FOR UPDATE/SHARE. */ @@ -1580,8 +1637,10 @@ translate_col_privs(const Bitmapset *parent_privs, * child rel instead. We also update rtindexes appearing outside Vars, * such as resultRelation and jointree relids. * - * Note: this is only applied after conversion of sublinks to subplans, - * so we don't need to cope with recursion into sub-queries. + * Note: this is applied after conversion of sublinks to subplans. It is also + * applied shortly before that conversion, at which point every sublink is + * destined to produce a subplan. Therefore, we don't need to cope with + * recursion into sub-queries. * * Note: this is not hugely different from what pullup_replace_vars() does; * maybe we should try to fold the two routines together. diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 8aa40d0..4f738d8 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -1409,13 +1409,15 @@ typedef struct LateralJoinInfo * will not have many different append parents, it doesn't seem worthwhile * to complicate things. * - * Note: after completion of the planner prep phase, any given RTE is an - * append parent having entries in append_rel_list if and only if its - * "inh" flag is set. We clear "inh" for plain tables that turn out not - * to have inheritance children, and (in an abuse of the original meaning - * of the flag) we set "inh" for subquery RTEs that turn out to be - * flattenable UNION ALL queries. This lets us avoid useless searches - * of append_rel_list. + * Note: after completion of the planner prep phase, every RTE having entries + * in append_rel_list has its "inh" flag set. This lets us avoid useless + * searches of append_rel_list. We clear "inh" for plain tables that turn out + * not to have inheritance children. In an abuse of the original meaning of + * the flag, we set "inh" for subquery RTEs that turn out to be flattenable + * UNION ALL queries. After attaching inheritance children to an independent + * UNION ALL, we leave "inh" set on the original inheritance parent RTE + * despite that RTE having no entries in append_rel_list; see + * expand_inherited_rtentry(). * * Note: the data structure assumes that append-rel members are single * baserels. This is OK for inheritance, but it prevents us from pulling diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out index 6f9ee5e..24a13bc 100644 --- a/src/test/regress/expected/union.out +++ b/src/test/regress/expected/union.out @@ -502,9 +502,56 @@ explain (costs off) Index Cond: (ab = 'ab'::text) (7 rows) +-- +-- Test that ORDER BY for UNION ALL can be pushed down on inheritance +-- tables. +-- +CREATE TEMP TABLE t1c (b text, a text); +ALTER TABLE t1c INHERIT t1; +CREATE TEMP TABLE t2c (primary key (ab)) INHERITS (t2); +INSERT INTO t1c VALUES ('v', 'w'), ('c', 'd'); +INSERT INTO t2c VALUES ('vw'), ('cd'); +CREATE INDEX t1c_ab_idx on t1c ((a || b)); +set enable_seqscan = on; +set enable_indexonlyscan = off; +explain (costs off) + SELECT * FROM + (SELECT a || b AS ab, t1 FROM t1 + UNION ALL + SELECT ab, null FROM t2) t + ORDER BY 1 LIMIT 10; + QUERY PLAN +------------------------------------------------ + Limit + -> Merge Append + Sort Key: ((t1.a || t1.b)) + -> Index Scan using t1_ab_idx on t1 + -> Index Scan using t1c_ab_idx on t1c + -> Index Scan using t2_pkey on t2 + -> Index Scan using t2c_pkey on t2c +(7 rows) + + SELECT * FROM + (SELECT a || b AS ab, t1 FROM t1 + UNION ALL + SELECT ab, null FROM t2) t + ORDER BY 1,2 LIMIT 10; + ab | t1 +----+------- + ab | (a,b) + ab | + cd | + dc | (d,c) + vw | + wv | (w,v) + xy | (x,y) + xy | +(8 rows) + reset enable_seqscan; reset enable_indexscan; reset enable_bitmapscan; +reset enable_indexonlyscan; -- Test constraint exclusion of UNION ALL subqueries explain (costs off) SELECT * FROM diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql index d567cf1..13f29ab 100644 --- a/src/test/regress/sql/union.sql +++ b/src/test/regress/sql/union.sql @@ -198,9 +198,37 @@ explain (costs off) SELECT * FROM t2) t WHERE ab = 'ab'; +-- +-- Test that ORDER BY for UNION ALL can be pushed down on inheritance +-- tables. +-- + +CREATE TEMP TABLE t1c (b text, a text); +ALTER TABLE t1c INHERIT t1; +CREATE TEMP TABLE t2c (primary key (ab)) INHERITS (t2); +INSERT INTO t1c VALUES ('v', 'w'), ('c', 'd'); +INSERT INTO t2c VALUES ('vw'), ('cd'); +CREATE INDEX t1c_ab_idx on t1c ((a || b)); +set enable_seqscan = on; +set enable_indexonlyscan = off; + +explain (costs off) + SELECT * FROM + (SELECT a || b AS ab, t1 FROM t1 + UNION ALL + SELECT ab, null FROM t2) t + ORDER BY 1 LIMIT 10; + + SELECT * FROM + (SELECT a || b AS ab, t1 FROM t1 + UNION ALL + SELECT ab, null FROM t2) t + ORDER BY 1,2 LIMIT 10; + reset enable_seqscan; reset enable_indexscan; reset enable_bitmapscan; +reset enable_indexonlyscan; -- Test constraint exclusion of UNION ALL subqueries explain (costs off)