RE: speeding up planning with partitions

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