Re: Incorrect calculation of path fraction value in MergeAppend

On Thu, May 8, 2025 at 2:42 AM Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:
>
> On 7/5/2025 12:45, Álvaro Herrera wrote:
> > On 2025-May-07, Andrei Lepikhov wrote:
> >> The magic is how it finds a link to the thread. This time it doesn't
> > Weird, that doesn't seem to work very well if you try to search for
> > stuff. But if you just specify the Message-Id without clicking
> > "search" then it works. So here you go:
> > https://commitfest.postgresql.org/patch/5742/
> Thanks! I think this memo should be pinned at the top of the page.
>
> On 7/5/2025 17:35, Junwang Zhao wrote:
> > + /* Convert absolute limit to a path fraction */
> > + if (path_fraction >= 1.)
> > + path_fraction /= childrel->rows;
> >
> > maybe add `&& childrel->rows > 0.` to the if condition to
> > avoid potential crash. I'm not sure if `childrel->rows` will
> > be 0 but I see get_cheapest_fractional_path did this.
> You may look at the 76281aa for the reason. Also, looking into the code,
> I see that it is impossible now to find more places with rows == 0
> except for the Dummy result node. Moreover, it just doesn't make sense
> in the case of append nodes: why should the optimiser spend resources
> and consider paths if it is impossible to pull any tuples from the subplan?
>
> So, I think checking assertion instead would be the better option. See
> new version in the attachment.

Agree, assertion is better, the new version LGTM.

>
> --
> regards, Andrei Lepikhov

--
Regards
Junwang Zhao