On Thu, Feb 1, 2024 at 11:37 AM David Rowley <dgrowle...@gmail.com> wrote:
> On Thu, 1 Feb 2024 at 16:29, Richard Guo <guofengli...@gmail.com> wrote: > > On Thu, Feb 1, 2024 at 10:04 AM James Coleman <jtc...@gmail.com> wrote: > >> I don't see any inherent reason why we must always assume that > >> gather_grouping_paths will always result in having at least one entry > >> in pathlist. If, for example, we've disabled incremental sort and the > >> cheapest partial path happens to already be sorted, then I don't > >> believe we'll add any paths. And if that happens then set_cheapest > >> will error with the message "could not devise a query plan for the > >> given query". So I propose we add a similar guard to this call point. > > > > > > I don't believe that would happen. It seems to me that we should, at > > the very least, have a path which is Gather on top of the cheapest > > partial path (see generate_gather_paths), as long as the > > partially_grouped_rel has partial paths. > > It will have partial paths because it's nested inside "if > (partially_grouped_rel && partially_grouped_rel->partial_pathlist)" Right. And that leads to the conclusion that gather_grouping_paths will always generate at least one path for partially_grouped_rel. So it seems to me that the check added in the patch is not necessary. But maybe we can add an Assert or a comment clarifying that the pathlist of partially_grouped_rel cannot be empty here. Thanks Richard
