On Fri, Jun 22, 2018 at 2:26 AM, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com> wrote: > I have applied patch and checked reported issue. Patch applied cleanly and > issues not reproducible any more.
Committed, with a few changes:
Thanks Robert.
- Renamed found_partially_grouped_child to partial_grouping_valid. The old name seemed to me to be a poor choice, because it sounds from the name like it gets set to true whenever we've found at least one partially grouped child, whereas really it gets set to false whenever we've failed to find at least one partially grouped child. The new name is also more like the names in add_paths_to_append_rel.
- Modified the wording of the comment.
- Omitted the new assertion in add_paths_to_append_rel. I doubt whether that's correct. I don't see any obvious reason why live_childrels can't be NIL there, and further down I see this:
/* * If we found unparameterized paths for all children, build an unordered, * unparameterized Append path for the rel. (Note: this is correct even * if we have zero or one live subpath due to constraint exclusion.) */
If it's not possible to have no live_childrels, then that comment is highly suspect.
OK, do these comments also holds true for partial_subpaths?
If we have NIL live_childrels, then the Assert() present inside the "if (partial_subpaths_valid)" block will hit. Which I think is wrong.
We either need to remove these Asserts altogether or we should enter inside this block only when we have non-NIL partial_subpaths. Also, if we don't have any partial_subpaths, then variable partial_subpaths_valid should be false. Currently, by default, it is set to true due to which we are ending up inside that if block and then hitting an Assert.
Also, even if this assertion *is* correct, I think it needs a better comment explaining why it's correct, because there isn't anything obvious in set_append_rel_pathlist that keeps IS_DUMMY_REL() from being true for every child. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
--
Jeevan Chalke Technical Architect, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company