Subject: [PATCH] Clean up property graph error messages

From: Ayush Tiwari <[email-masked]>
Date: Tue, 5 May 2026 01:27:00 +0530

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", 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.

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

---

From: Chao Li <[email-masked]>
Date: Tue, 5 May 2026 10:50:09 +0800

> On May 5, 2026, at 03:57, Ayush Tiwari <[email-masked]> wrote:
> 
> Hi,
> 
> 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

Agreed.

> [On a separate note, it might be better to change all "vertexes" to "vertices",
> <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/

---

From: Peter Eisentraut <[email-masked]>
Date: Tue, 5 May 2026 09:43:21 +0200

> haven't included that in the patch though since the other one is not exactly

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.)

---

From: Ayush Tiwari <[email-masked]>
Date: Wed, 6 May 2026 11:53:38 +0530

Hi,

> “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

---

From: Chao Li <[email-masked]>
Date: Wed, 6 May 2026 14:45:46 +0800

> On May 6, 2026, at 14:23, Ayush Tiwari <[email-masked]> wrote:
> 
> Hi,
> 
> 
> <v2-0001-Clean-up-property-graph-error-messages.patch>

V2 LGTM.

Best regards,

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 7 May 2026 10:45:13 +0200

On 04.05.26 21:57, Ayush Tiwari wrote:
> properties names" to "mismatching property names",

I have fixed that.

> placed after ereport(ERROR) was unreachable.

I think that's fine.  I don't think the change makes this better.

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 7 May 2026 10:46:04 +0200

On 06.05.26 08:23, Ayush Tiwari wrote:
> On Tue, 5 May 2026 at 08:20, Chao Li <[email-masked] 
> <mailto:[email-masked]>> wrote:
> 
> 
>     So, v1 LGTM. Only thing is that I think we can also make
>     “mismatching” to “mismatched” in the errmsg.
> 
> that in v2.

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

---

From: Ayush Tiwari <[email-masked]>
Date: Thu, 7 May 2026 14:34:05 +0530

Hi,

> 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

---

From: Ashutosh Bapat <[email-masked]>
Date: Mon, 11 May 2026 12:09:24 +0530

> 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