[PATCH] Clean up property graph error messages

On May 5, 2026, at 03:57, Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:

Hi,

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 think changing “properties names” to “property names” is correct.

I wonder if we can also change “mismatching” to “mismatched”, so the error message is:
```
"mismatched property names in definition of label \"%s\""
```

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.

Agreed.

The regression expected output is updated for the changed error text.

[On a separate note, it might be better to change all "vertexes" to "vertices",
haven't included that in the patch though since the other one is not exactly
wrong?]

Regards,
Ayus
<v1-0001-Clean-up-property-graph-error-messages.patch>

So, v1 LGTM. Only thing is that I think we can also make “mismatching” to “mismatched” in the errmsg.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On 04.05.26 21:57, Ayush Tiwari wrote:

[On a separate note, it might be better to change all "vertexes" to
"vertices",
haven't included that in the patch though since the other one is not exactly
wrong?]

I have fixed that, thanks for pointing it out. We should stick to the
preferred terms.

(I'll look at the rest of your patch in a bit.)

Hi,

On Tue, 5 May 2026 at 08:20, Chao Li <li.evan.chao@gmail.com> wrote:

So, v1 LGTM. Only thing is that I think we can also make “mismatching” to
“mismatched” in the errmsg.

I agree that "mismatched property names" reads better, so I have updated
that in v2. I did not include the vertexes/vertices change here, since
Peter has already fixed that separately.

Regards,
Ayush

On May 6, 2026, at 14:23, Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:

Hi,

On Tue, 5 May 2026 at 08:20, Chao Li <li.evan.chao@gmail.com> wrote:

So, v1 LGTM. Only thing is that I think we can also make “mismatching” to “mismatched” in the errmsg.

I agree that "mismatched property names" reads better, so I have updated
that in v2. I did not include the vertexes/vertices change here, since
Peter has already fixed that separately.

Regards,
Ayush
<v2-0001-Clean-up-property-graph-error-messages.patch>

V2 LGTM.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

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.

On 06.05.26 08:23, Ayush Tiwari wrote:

On Tue, 5 May 2026 at 08:20, Chao Li <li.evan.chao@gmail.com
<mailto:li.evan.chao@gmail.com>> wrote:

So, v1 LGTM. Only thing is that I think we can also make
“mismatching” to “mismatched” in the errmsg.

I agree that "mismatched property names" reads better, so I have updated
that in v2.

I think the existing wording is better. In any case, you only changed a
few places and not others.

Hi,

On Thu, 7 May 2026 at 14:15, Peter Eisentraut <peter@eisentraut.org> wrote:

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.

IIUC that is dead code right now, it never reaches that point to release?

Regards,
Ayush

On Thu, May 7, 2026 at 2:15 PM Peter Eisentraut <peter@eisentraut.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