Horton

Thread

Commits

  1. Use LOCKMODE in parse_relation.c/.h

  2. Don't include low level locking code from frontend code.

  3. Add a bunch of new error location reports to parse-analysis error messages.

  1. Use LOCKMODE in parse_relation.c/.h

    Peter Eisentraut <peter@eisentraut.org> — 2026-02-18T09:50:38Z

    There are a couple of comments in parse_relation.c

 > Note: properly, lockmode should be declared LOCKMODE not int, but that
 > would require importing storage/lock.h into parse_relation.h.  Since
 > LOCKMODE is typedef'd as int anyway, that seems like overkill.

but actually LOCKMODE has been in storage/lockdefs.h for a while,
which is intentionally a more narrow header.  So we can include that
one in parse_relation.h and just use LOCKMODE normally.

An alternative would be to add a duplicate typedef into 
parse_relation.h, but that doesn't seem necessary here.

  2. Re: Use LOCKMODE in parse_relation.c/.h

    Andreas Karlsson <andreas@proxel.se> — 2026-02-18T17:19:57Z

    On 2/18/26 10:50 AM, Peter Eisentraut wrote:
> There are a couple of comments in parse_relation.c
> 
>  > Note: properly, lockmode should be declared LOCKMODE not int, but that
>  > would require importing storage/lock.h into parse_relation.h.  Since
>  > LOCKMODE is typedef'd as int anyway, that seems like overkill.
> 
> but actually LOCKMODE has been in storage/lockdefs.h for a while,
> which is intentionally a more narrow header.  So we can include that
> one in parse_relation.h and just use LOCKMODE normally.
> 
> An alternative would be to add a duplicate typedef into 
> parse_relation.h, but that doesn't seem necessary here.

Looks like a nice change and I did not find any more cases where we 
should fix this.

But when I was looking I found a case where BufferLockMode could be used 
in the gin code (ginStepRight() and the brin (brinGetTupleForHeapBlock) 
code but I am not sure there are worth fixing.

Andreas

  3. Re: Use LOCKMODE in parse_relation.c/.h

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-02-19T09:03:53Z

    On Wed, Feb 18, 2026 at 3:20 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> There are a couple of comments in parse_relation.c
>
>  > Note: properly, lockmode should be declared LOCKMODE not int, but that
>  > would require importing storage/lock.h into parse_relation.h.  Since
>  > LOCKMODE is typedef'd as int anyway, that seems like overkill.
>
> but actually LOCKMODE has been in storage/lockdefs.h for a while,
> which is intentionally a more narrow header.  So we can include that
> one in parse_relation.h and just use LOCKMODE normally.

lockdefs.h is much younger (4eda0a64705763854225a29703b606692af50e77)
than the comment (b153c0920960a6059b67969469166fb29c0105d7) mentioned
above. The commit changed some #include "lock.h" to use lockdefs.h. I
guess it didn't notice that parse_relation.h can use it because it
didn't include lock.h and didn't define LOCKMODE. The change looks
good to me.

>
> An alternative would be to add a duplicate typedef into
> parse_relation.h, but that doesn't seem necessary here.

Not needed.

-- 
Best Wishes,
Ashutosh Bapat

  4. Re: Use LOCKMODE in parse_relation.c/.h

    Peter Eisentraut <peter@eisentraut.org> — 2026-02-23T20:26:29Z

    On 19.02.26 10:03, Ashutosh Bapat wrote:
> On Wed, Feb 18, 2026 at 3:20 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>>
>> There are a couple of comments in parse_relation.c
>>
>>   > Note: properly, lockmode should be declared LOCKMODE not int, but that
>>   > would require importing storage/lock.h into parse_relation.h.  Since
>>   > LOCKMODE is typedef'd as int anyway, that seems like overkill.
>>
>> but actually LOCKMODE has been in storage/lockdefs.h for a while,
>> which is intentionally a more narrow header.  So we can include that
>> one in parse_relation.h and just use LOCKMODE normally.
> 
> lockdefs.h is much younger (4eda0a64705763854225a29703b606692af50e77)
> than the comment (b153c0920960a6059b67969469166fb29c0105d7) mentioned
> above. The commit changed some #include "lock.h" to use lockdefs.h. I
> guess it didn't notice that parse_relation.h can use it because it
> didn't include lock.h and didn't define LOCKMODE. The change looks
> good to me.

committed

  5. Re: Use LOCKMODE in parse_relation.c/.h

    Peter Eisentraut <peter@eisentraut.org> — 2026-02-23T20:27:30Z

    On 18.02.26 18:19, Andreas Karlsson wrote:
> On 2/18/26 10:50 AM, Peter Eisentraut wrote:
>> There are a couple of comments in parse_relation.c
>>
>>  > Note: properly, lockmode should be declared LOCKMODE not int, but that
>>  > would require importing storage/lock.h into parse_relation.h.  Since
>>  > LOCKMODE is typedef'd as int anyway, that seems like overkill.
>>
>> but actually LOCKMODE has been in storage/lockdefs.h for a while,
>> which is intentionally a more narrow header.  So we can include that
>> one in parse_relation.h and just use LOCKMODE normally.
>>
>> An alternative would be to add a duplicate typedef into 
>> parse_relation.h, but that doesn't seem necessary here.
> 
> Looks like a nice change and I did not find any more cases where we 
> should fix this.
> 
> But when I was looking I found a case where BufferLockMode could be used 
> in the gin code (ginStepRight() and the brin (brinGetTupleForHeapBlock) 
> code but I am not sure there are worth fixing.

I think these could be worth improving, if only to make the function 
signatures more clear (there are different kinds of enums for different 
kinds of lock modes).