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
