Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix typo in error message

  2. Make spelling consistent

  1. [PATCH] Clean up property graph error messages

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-05-04T19:57:00Z

    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

  2. Re: [PATCH] Clean up property graph error messages

    Chao Li <li.evan.chao@gmail.com> — 2026-05-05T02:50:09Z

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

  3. Re: [PATCH] Clean up property graph error messages

    Peter Eisentraut <peter@eisentraut.org> — 2026-05-05T07:43:21Z

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

  4. Re: [PATCH] Clean up property graph error messages

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-05-06T06:23:38Z

    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

  5. Re: [PATCH] Clean up property graph error messages

    Chao Li <li.evan.chao@gmail.com> — 2026-05-06T06:45:46Z

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

  6. Re: [PATCH] Clean up property graph error messages

    Peter Eisentraut <peter@eisentraut.org> — 2026-05-07T08:45:13Z

    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.

  7. Re: [PATCH] Clean up property graph error messages

    Peter Eisentraut <peter@eisentraut.org> — 2026-05-07T08:46:04Z

    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.

  8. Re: [PATCH] Clean up property graph error messages

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-05-07T09:04:05Z

    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

  9. Re: [PATCH] Clean up property graph error messages

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-05-11T06:39:24Z

    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