Re: Explicit NULL dereference (src/backend/utils/adt/ruleutils.c)

At Mon, 02 Nov 2020 14:49:50 -0500, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in
> Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> writes:
> > Em seg., 2 de nov. de 2020 às 01:36, Kyotaro Horiguchi <
> > horikyota(dot)ntt(at)gmail(dot)com> escreveu:
> >> Doesn't seem that ev_qual and ev_action can be NULL. The same
> >> function in the current converts action list to string using
> >> nodeToSTring so NIL is converted into '<>', which is not NULL.
>
> > I think that Assert is not the right solution here.
>
> I think there's some confusion here: whether the ev_actions column can
> contain a SQL NULL is a very different thing from whether the result of
> stringToNode() on it can be a NIL list. The latter should also not
> happen, but it's not enforced by low-level code in the same way that
> the NOT NULL property is. So in my judgment, it's okay for us to just
> Assert that we got a not-null datum, but it's probably worth expending
> an actual test-and-elog for the NIL-list case.
>
> Of course, someone could put a string into that column that doesn't
> read out as a List at all, or it does but the elements aren't Query
> nodes, etc etc. There's a very finite limit to how much code I'm
> willing to expend on such scenarios. But a test for NIL list seems
> reasonable, since this function previously had what looked like sane
> handling for that case.
>
> Anyway, I adjusted Kyotaro-san's patch a bit (including fixing the
> near identical code in make_viewdef) and pushed it.

Thanks!

--
Kyotaro Horiguchi
NTT Open Source Software Center