On 21 June 2015 at 05:27, Robert Haas <robertmh...@gmail.com> wrote:
> On Sat, Jun 20, 2015 at 6:48 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
>> I propose instead the attached patch, which operates by identifying which
>> of the append_rel_list entries actually contain subquery references, and
>> copying only those; the other ones are just linked into the child's
>> append_rel_list by reference, which is okay because they won't get
>> modified.
>
> Better than unpatched, definitely! Not sure how it compares to my patch.
>
I tested on my machine (optimised build, asserts off). With HEAD I got:
Q1: 8076ms
Q2: 7165ms
Q3: 4027ms
Q4: OOM (killed by kernel, used > 16GB RAM)
Q5: 4131ms
The machine only has 16GB of RAM and almost no swap, so it wasn't able to do Q4.
With Robert's patch:
Q1: 1121ms
Q2: 542ms
Q3: 498ms
Q4: 50763ms (used 3GB RAM)
Q5: 556ms
and with Tom's patch:
Q1: 2264ms
Q2: 3785ms
Q3: 507ms
Q4: 50851ms (used 3GB RAM)
Q5: 558ms
However, there's an obvious improvement that can be made to Tom's
patch -- having computed modifiableARIindexes, you may as well use it
in the innermost loop to only apply ChangeVarNodes() to those
AppendRelInfo's that can actually change, rather than having it trawl
through all the other ones that we know won't be touched.
With that improvement (attached), the timings become:
Q1: 1148ms
Q2: 547ms
Q3: 505ms
Q4: 51325ms
Q5: 544ms
i.e., basically the same as Robert's patch.
Regards,
Dean
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
new file mode 100644
index 8afde2b..c8a3c65
*** a/src/backend/optimizer/plan/planner.c
--- b/src/backend/optimizer/plan/planner.c
*************** inheritance_planner(PlannerInfo *root)
*** 834,840 ****
{
Query *parse = root->parse;
int parentRTindex = parse->resultRelation;
! Bitmapset *resultRTindexes = NULL;
int nominalRelation = -1;
List *final_rtable = NIL;
int save_rel_array_size = 0;
--- 834,842 ----
{
Query *parse = root->parse;
int parentRTindex = parse->resultRelation;
! Bitmapset *resultRTindexes;
! Bitmapset *subqueryRTindexes;
! Bitmapset *modifiableARIindexes;
int nominalRelation = -1;
List *final_rtable = NIL;
int save_rel_array_size = 0;
*************** inheritance_planner(PlannerInfo *root)
*** 845,850 ****
--- 847,853 ----
List *returningLists = NIL;
List *rowMarks;
ListCell *lc;
+ Index rti;
Assert(parse->commandType != CMD_INSERT);
*************** inheritance_planner(PlannerInfo *root)
*** 867,874 ****
* subqueries during planning, and so we must create copies of them too,
* except where they are target relations, which will each only be used in
* a single plan.
*/
! resultRTindexes = bms_add_member(resultRTindexes, parentRTindex);
foreach(lc, root->append_rel_list)
{
AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
--- 870,879 ----
* subqueries during planning, and so we must create copies of them too,
* except where they are target relations, which will each only be used in
* a single plan.
+ *
+ * To begin with, we'll need a bitmapset of the target relation relids.
*/
! resultRTindexes = bms_make_singleton(parentRTindex);
foreach(lc, root->append_rel_list)
{
AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
*************** inheritance_planner(PlannerInfo *root)
*** 878,889 ****
appinfo->child_relid);
}
foreach(lc, root->append_rel_list)
{
AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
PlannerInfo subroot;
Plan *subplan;
! Index rti;
/* append_rel_list contains all append rels; ignore others */
if (appinfo->parent_relid != parentRTindex)
--- 883,937 ----
appinfo->child_relid);
}
+ /*
+ * Now, generate a bitmapset of the relids of the subquery RTEs, including
+ * security-barrier RTEs that will become subqueries, as just explained.
+ */
+ subqueryRTindexes = NULL;
+ rti = 1;
+ foreach(lc, parse->rtable)
+ {
+ RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
+
+ if (rte->rtekind == RTE_SUBQUERY ||
+ (rte->securityQuals != NIL &&
+ !bms_is_member(rti, resultRTindexes)))
+ subqueryRTindexes = bms_add_member(subqueryRTindexes, rti);
+ rti++;
+ }
+
+ /*
+ * Next, we want to identify which AppendRelInfo items contain references
+ * to any of the aforesaid subquery RTEs. These items will need to be
+ * copied and modified to adjust their subquery references; whereas the
+ * other ones need not be touched. It's worth being tense over this
+ * because we can usually avoid processing most of the AppendRelInfo
+ * items, thereby saving O(N^2) space and time when the target is a large
+ * inheritance tree. We can identify AppendRelInfo items by their
+ * child_relid, since that should be unique within the list.
+ */
+ modifiableARIindexes = NULL;
+ foreach(lc, root->append_rel_list)
+ {
+ AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
+
+ if (bms_is_member(appinfo->parent_relid, subqueryRTindexes) ||
+ bms_is_member(appinfo->child_relid, subqueryRTindexes) ||
+ bms_overlap(pull_varnos((Node *) appinfo->translated_vars),
+ subqueryRTindexes))
+ modifiableARIindexes = bms_add_member(modifiableARIindexes,
+ appinfo->child_relid);
+ }
+
+ /*
+ * And now we can get on with generating a plan for each child table.
+ */
foreach(lc, root->append_rel_list)
{
AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
PlannerInfo subroot;
Plan *subplan;
! ListCell *lc2;
/* append_rel_list contains all append rels; ignore others */
if (appinfo->parent_relid != parentRTindex)
*************** inheritance_planner(PlannerInfo *root)
*** 917,925 ****
/*
* The append_rel_list likewise might contain references to subquery
* RTEs (if any subqueries were flattenable UNION ALLs). So prepare
! * to apply ChangeVarNodes to that, too.
*/
! subroot.append_rel_list = (List *) copyObject(root->append_rel_list);
/*
* Add placeholders to the child Query's rangetable list to fill the
--- 965,985 ----
/*
* The append_rel_list likewise might contain references to subquery
* RTEs (if any subqueries were flattenable UNION ALLs). So prepare
! * to apply ChangeVarNodes to that, too. As explained above, we only
! * want to copy items that actually contain such references; the
! * rest can just get linked into the subroot's append_rel_list.
*/
! subroot.append_rel_list = NIL;
! foreach(lc2, root->append_rel_list)
! {
! AppendRelInfo *appinfo2 = (AppendRelInfo *) lfirst(lc2);
!
! if (bms_is_member(appinfo2->child_relid, modifiableARIindexes))
! appinfo2 = (AppendRelInfo *) copyObject(appinfo2);
!
! subroot.append_rel_list = lappend(subroot.append_rel_list,
! appinfo2);
! }
/*
* Add placeholders to the child Query's rangetable list to fill the
*************** inheritance_planner(PlannerInfo *root)
*** 933,945 ****
/*
* If this isn't the first child Query, generate duplicates of all
! * subquery RTEs, and adjust Var numbering to reference the
! * duplicates. To simplify the loop logic, we scan the original rtable
! * not the copy just made by adjust_appendrel_attrs; that should be OK
! * since subquery RTEs couldn't contain any references to the target
! * rel.
*/
! if (final_rtable != NIL)
{
ListCell *lr;
--- 993,1005 ----
/*
* If this isn't the first child Query, generate duplicates of all
! * subquery (or subquery-to-be) RTEs, and adjust Var numbering to
! * reference the duplicates. To simplify the loop logic, we scan the
! * original rtable not the copy just made by adjust_appendrel_attrs;
! * that should be OK since subquery RTEs couldn't contain any
! * references to the target rel.
*/
! if (final_rtable != NIL && !bms_is_empty(subqueryRTindexes))
{
ListCell *lr;
*************** inheritance_planner(PlannerInfo *root)
*** 948,961 ****
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(lr);
! /*
! * Copy subquery RTEs and RTEs with security barrier quals
! * that will be turned into subqueries, except those that are
! * target relations.
! */
! if (rte->rtekind == RTE_SUBQUERY ||
! (rte->securityQuals != NIL &&
! !bms_is_member(rti, resultRTindexes)))
{
Index newrti;
--- 1008,1014 ----
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(lr);
! if (bms_is_member(rti, subqueryRTindexes))
{
Index newrti;
*************** inheritance_planner(PlannerInfo *root)
*** 968,974 ****
newrti = list_length(subroot.parse->rtable) + 1;
ChangeVarNodes((Node *) subroot.parse, rti, newrti, 0);
ChangeVarNodes((Node *) subroot.rowMarks, rti, newrti, 0);
! ChangeVarNodes((Node *) subroot.append_rel_list, rti, newrti, 0);
rte = copyObject(rte);
ChangeVarNodes((Node *) rte->securityQuals, rti, newrti, 0);
subroot.parse->rtable = lappend(subroot.parse->rtable,
--- 1021,1033 ----
newrti = list_length(subroot.parse->rtable) + 1;
ChangeVarNodes((Node *) subroot.parse, rti, newrti, 0);
ChangeVarNodes((Node *) subroot.rowMarks, rti, newrti, 0);
! foreach(lc2, subroot.append_rel_list)
! {
! AppendRelInfo *appinfo2 = (AppendRelInfo *) lfirst(lc2);
!
! if (bms_is_member(appinfo2->child_relid, modifiableARIindexes))
! ChangeVarNodes((Node *) appinfo2, rti, newrti, 0);
! }
rte = copyObject(rte);
ChangeVarNodes((Node *) rte->securityQuals, rti, newrti, 0);
subroot.parse->rtable = lappend(subroot.parse->rtable,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
