MIME-Version: 1.0
References: 
 <CAJTYsWXFy1j_T82+M_S9kFxU414tQYnZQD-b82=oL_LbG_5fPQ@mail.gmail.com>
 <459ea164-aa38-43dd-8c93-dbb1063d20b5@eisentraut.org>
In-Reply-To: <459ea164-aa38-43dd-8c93-dbb1063d20b5@eisentraut.org>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Mon, 11 May 2026 12:09:24 +0530
Message-ID: 
 <CAExHW5sG3N8YFY3o4_+0uxpou4Uw6kjz7MefeOBVbDtbh3ahLA@mail.gmail.com>
Subject: Re: [PATCH] Clean up property graph error messages
To: Peter Eisentraut <peter@eisentraut.org>
Cc: Ayush Tiwari <ayushtiwari.slg01@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAExHW5sG3N8YFY3o4_%2B0uxpou4Uw6kjz7MefeOBVbDtbh3ahLA%40mail.gmail.com>
Precedence: bulk

On Thu, May 7, 2026 at 2:15=E2=80=AFPM Peter Eisentraut <peter@eisentraut.o=
rg> 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.

--=20
Best Wishes,
Ashutosh Bapat