Horton

Thread

Commits

  1. Fix incorrect logic in plan dependency recording

  1. Surely this code in setrefs.c is wrong?

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-09-09T23:22:52Z

    I happened to notice this bit in fix_expr_common's processing
of ScalarArrayOpExprs:

        set_sa_opfuncid(saop);
        record_plan_function_dependency(root, saop->opfuncid);

        if (!OidIsValid(saop->hashfuncid))
            record_plan_function_dependency(root, saop->hashfuncid);

        if (!OidIsValid(saop->negfuncid))
            record_plan_function_dependency(root, saop->negfuncid);

Surely those if-conditions are exactly backward, and we should be
recording nonzero hashfuncid and negfuncid entries, not zero ones.
As-is, the code's a no-op because record_plan_function_dependency
will ignore OIDs less than FirstUnpinnedObjectId, including zero.

"git blame" blames 50e17ad28 and 29f45e299 for these, so v14
has only half the problem of later branches.

			regards, tom lane

  2. Re: Surely this code in setrefs.c is wrong?

    David Rowley <dgrowleyml@gmail.com> — 2023-09-10T09:07:53Z

    On Sun, 10 Sept 2023 at 11:22, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>         if (!OidIsValid(saop->hashfuncid))
>             record_plan_function_dependency(root, saop->hashfuncid);
>
>         if (!OidIsValid(saop->negfuncid))
>             record_plan_function_dependency(root, saop->negfuncid);
>
> Surely those if-conditions are exactly backward, and we should be
> recording nonzero hashfuncid and negfuncid entries, not zero ones.

That's certainly not coded as I intended. Perhaps I got my wires
crossed and mixed up OidIsValid and InvalidOid and without reading
correctly somehow thought OidIsValid was for the inverse case.

I'll push fixes once the 16.0 release is out of the way.

David

  3. Re: Surely this code in setrefs.c is wrong?

    David Rowley <dgrowleyml@gmail.com> — 2023-09-13T23:57:11Z

    On Sun, 10 Sept 2023 at 21:07, David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Sun, 10 Sept 2023 at 11:22, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >         if (!OidIsValid(saop->hashfuncid))
> >             record_plan_function_dependency(root, saop->hashfuncid);
> >
> >         if (!OidIsValid(saop->negfuncid))
> >             record_plan_function_dependency(root, saop->negfuncid);
> >
> > Surely those if-conditions are exactly backward, and we should be
> > recording nonzero hashfuncid and negfuncid entries, not zero ones.
>

> I'll push fixes once the 16.0 release is out of the way.

Fixed in ee3a551e9.

David