Re: [PATCH] Clean up property graph error messages

On Thu, May 7, 2026 at 2:15 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 04.05.26 21:57, Ayush Tiwari wrote:
> > While looking at the SQL/PGQ property graph error paths, I noticed a
> > few small cleanups in propgraphcmds.c.
> >
> > The attached patch fixes a user-visible error message from "mismatching
> > properties names" to "mismatching property names",
>
> I have fixed that.
>
> > and moves a
> > ReleaseSysCache() call before an ERROR ereport in
> > check_element_properties().
> >
> > The existing code should be cleaned up by
> > the resource owner on the ERROR path, but the explicit ReleaseSysCache()
> > placed after ereport(ERROR) was unreachable.
>
> I think that's fine. I don't think the change makes this better.
>

Yeah.

If we call ReleaseSysCache before ereport(), we need to add local
variables as you have done, which increases the lines of code. Since
ereport will never return, ReleaseSysCache() is not needed at all. But
leaving it there helps justifying fetching the attributes in the
ereport call. FWIW, it makes the code future proof, in a very rare
case if someone makes ereport conditional.

--
Best Wishes,
Ashutosh Bapat