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' <Langote_Amit_f8@lab.ntt.co.jp>
Cc: 'David Rowley' <david.rowley@2ndquadrant.com>, 'Alvaro Herrera' <alvherre@2ndquadrant.com>, 'Pg Hackers' <pgsql-hackers@postgresql.org>
Date: 2019-01-10T06:37:27Z
Lists: pgsql-hackers 
Hi Amit,

On Mon, Jan 7, 2019 at 6:30 PM, Amit Langote wrote:
> On 2018/12/27 20:25, Amit Langote wrote:
> > Here's the v10 of the patch.  I didn't get chance to do more changes
> > than those described above and address Imai-san's review comments
> yesterday.
> >
> > Have a wonderful new year!  I'll be back on January 7 to reply on this
> thread.
> 
> Rebased due to changed copyright year in prepunion.c.  Also realized that
> the new files added by patch 0004 still had 2018 in them.

Thank you for new patches.


I also have some comments on 0001, set_inherit_target_rel_sizes().

In set_inherit_target_rel_sizes():

Some codes are placed not the same order as set_append_rel_size().

0001: at line 325-326,
+	ListCell   *l;
+	bool		has_live_children;

In set_append_rel_size(), "has_live_children" is above of the "ListCell *l";

0001: at line 582-603
+		if (IS_DUMMY_REL(childrel))
+			continue;
+
...
+		Assert(childrel->rows > 0);
+
+		/* We have at least one live child. */
+		has_live_children = true;

In set_append_rel_size(), 
+		/* We have at least one live child. */
+		has_live_children = true;
is directly under of
+		if (IS_DUMMY_REL(childrel))
+			continue;

0001: at line 606-622,
+	if (!has_live_children)
+	{
+		/*
+		 * All children were excluded by constraints, so mark the relation
+		 * ass dummy.  We must do this in this phase so that the rel's
+		 * dummy-ness is visible when we generate paths for other rels.
+		 */
+		set_dummy_rel_pathlist(rel);
+	}
+	else
+		/*
+		 * Set a non-zero value here to cope with the caller's requirement
+		 * that non-dummy relations are actually not empty.  We don't try to
+		 * be accurate here, because we're not going to create a path that
+		 * combines the children outputs.
+		 */
+		rel->rows = 1;

In set_append_rel_size(), a condition of if clause is not !has_live_children but 
has_live_children.

I also noticed there isn't else block while there is if block.

--
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.