Out of date comment in predicate.c

Started by Thomas Munroabout 9 years ago4 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t37014
psql -h localhost -U postgres

Built from patchset v3 (message #3), July 28, 2026 at 05:56 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t37014_3 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t37014_3 && git checkout t37014_3

Patchset v3 (message #3) is on t37014_3

Jump to latest
#1Thomas Munro
thomas.munro@gmail.com

Hi hackers,

Commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f got rid of
FirstPredicateLockMgrLock, but it's still referred to in a comment in
predicate.c where the locking protocol is documented. I think it's
probably best to use the name of the macro that's usually used to
access the lock array in the code. Please see attached.

--
Thomas Munro
http://www.enterprisedb.com

Attachments:

fix-comments.patchapplication/octet-stream; name=fix-comments.patchDownload+2-2
#2Peter Eisentraut
peter_e@gmx.net
In reply to: Thomas Munro (#1)
Re: Out of date comment in predicate.c

On 6/27/17 01:21, Thomas Munro wrote:

Commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f got rid of
FirstPredicateLockMgrLock, but it's still referred to in a comment in
predicate.c where the locking protocol is documented. I think it's
probably best to use the name of the macro that's usually used to
access the lock array in the code. Please see attached.

Does this apply equally to PredicateLockHashPartitionLock() and
PredicateLockHashPartitionLockByIndex()? Should the comment mention or
imply both?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Thomas Munro
thomas.munro@gmail.com
In reply to: Peter Eisentraut (#2)
Re: Out of date comment in predicate.c

On Sat, Jul 1, 2017 at 6:38 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 6/27/17 01:21, Thomas Munro wrote:

Commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f got rid of
FirstPredicateLockMgrLock, but it's still referred to in a comment in
predicate.c where the locking protocol is documented. I think it's
probably best to use the name of the macro that's usually used to
access the lock array in the code. Please see attached.

Does this apply equally to PredicateLockHashPartitionLock() and
PredicateLockHashPartitionLockByIndex()? Should the comment mention or
imply both?

Yeah, I guess so. How about listing the hashcode variant, as it's the
more commonly used and important for a reader to understand of the
two, but mentioning the ByIndex variant in a bullet point below? Like
this.

--
Thomas Munro
http://www.enterprisedb.com

Attachments:

t37014_3
fix-comments-v2.patchapplication/octet-stream; name=fix-comments-v2.patchDownload+5-3
#4Peter Eisentraut
peter_e@gmx.net
In reply to: Thomas Munro (#3)
Re: Out of date comment in predicate.c

On 7/6/17 21:06, Thomas Munro wrote:

On Sat, Jul 1, 2017 at 6:38 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 6/27/17 01:21, Thomas Munro wrote:

Commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f got rid of
FirstPredicateLockMgrLock, but it's still referred to in a comment in
predicate.c where the locking protocol is documented. I think it's
probably best to use the name of the macro that's usually used to
access the lock array in the code. Please see attached.

Does this apply equally to PredicateLockHashPartitionLock() and
PredicateLockHashPartitionLockByIndex()? Should the comment mention or
imply both?

Yeah, I guess so. How about listing the hashcode variant, as it's the
more commonly used and important for a reader to understand of the
two, but mentioning the ByIndex variant in a bullet point below? Like
this.

Committed and backpatched to 9.4.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers