Horton

RE: speeding up planning with partitions

Imai, Yoshikazu <imai.yoshikazu@jp.fujitsu.com>

View thread View on postgr.es
From: "Imai, Yoshikazu" <imai.yoshikazu@jp.fujitsu.com>
To: 'Amit Langote' <amitlangote09@gmail.com>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>, Tom Lane <tgl@sss.pgh.pa.us>, David Rowley <david.rowley@2ndquadrant.com>, "Alvaro Herrera" <alvherre@2ndquadrant.com>, Justin Pryzby <pryzby@telsasoft.com>, "Pg Hackers" <pgsql-hackers@postgresql.org>, Jesper Pedersen <jesper.pedersen@redhat.com>
Date: 2019-03-13T10:34:50Z
Lists: pgsql-hackers 
Amit-san,

On Tue, Mar 12, 2019 at 2:34 PM, Amit Langote wrote:
> Thanks for the heads up.
> 
> On Tue, Mar 12, 2019 at 10:23 PM Jesper Pedersen
> <jesper.pedersen@redhat.com> wrote:
> > After applying 0004 check-world fails with the attached. CFBot agrees
> [1].
> 
> Fixed.  I had forgotten to re-run postgres_fdw tests after making a change
> just before submitting.

I have done code review of v31 patches from 0001 to 0004.
I described below what I confirmed or thoughts.

0001: This seems ok.

0002:
* I don't really know that delaying adding resjunk output columns to the plan doesn't affect any process in the planner. From the comments of PlanRowMark, those columns are used in only the executor so I think delaying adding junk Vars wouldn't be harm, is that right?

0003:
* Is there need to do CreatePartitionDirectory() if rte->inh becomes false?

+		else if (rte->rtekind == RTE_RELATION && rte->inh)
+		{
+			rte->inh = has_subclass(rte->relid);
+
+			/*
+			 * While at it, initialize the PartitionDesc infrastructure for
+			 * this query, if not done yet.
+			 */
+			if (root->glob->partition_directory == NULL)
+				root->glob->partition_directory =
+					CreatePartitionDirectory(CurrentMemoryContext);
+		}

0004:
* In addition to commit message, this patch also changes the method of adjusting AppendRelInfos which reference to the subquery RTEs, and new one seems logically correct.

* In inheritance_planner(), we do ChangeVarNodes() only for orig_rtable, so the codes concatenating lists of append_rel_list may be able to be moved before doing ChangeVarNodes() and then the codes concatenating lists of rowmarks, rtable and append_rel_list can be in one block (or one bunch).

--
Yoshikazu Imai

Commits

  1. Clean up handling of constraint_exclusion and enable_partition_pruning.

  2. Add test case exercising formerly-unreached code in inheritance_planner.

  3. Speed up planning when partitions can be pruned at plan time.

  4. Avoid crash in partitionwise join planning under GEQO.

  5. Avoid passing query tlist around separately from root->processed_tlist.

  6. Build "other rels" of appendrel baserels in a separate step.

  7. Get rid of duplicate child RTE for a partitioned table.

  8. Rearrange make_partitionedrel_pruneinfo to avoid work when we can't prune.

  9. Don't copy PartitionBoundInfo in set_relation_partition_info.

  10. Move building of child base quals out into a new function

  11. Reorganize planner code moved in b60c39759908

  12. Move inheritance expansion code into its own file

  13. Fix inherited UPDATE/DELETE with UNION ALL subqueries.

  14. Rearrange planner to save the whole PlannerInfo (subroot) for a subquery.