Incorrect comment in tablecmds.c

Started by Etsuro Fujitaabout 11 years ago3 messages

fujita.etsuro@lab.ntt.co.jp

about 11 years ago

1 attachment(s)

I don't think that the lock level mentioned in the following comment in
MergeAttributes() in tablecmds.c is right, since that that function has
opened the relation with ShareUpdateExclusiveLock, not with
AccessShareLock. Patch attached.

1749 /*
1750 * Close the parent rel, but keep our AccessShareLock on it
until xact
1751 * commit. That will prevent someone else from deleting or
ALTERing
1752 * the parent before the child is committed.
1753 */
1754 heap_close(relation, NoLock);

Thanks,

Best regards,
Etsuro Fujita

Attachments:

comment-fix.patchtext/x-diff; name=comment-fix.patchDownload

*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 1747,1755 **** MergeAttributes(List *schema, List *supers, char relpersistence,
  		pfree(newattno);
  
  		/*
! 		 * Close the parent rel, but keep our AccessShareLock on it until xact
! 		 * commit.  That will prevent someone else from deleting or ALTERing
! 		 * the parent before the child is committed.
  		 */
  		heap_close(relation, NoLock);
  	}
--- 1747,1755 ----
  		pfree(newattno);
  
  		/*
! 		 * Close the parent rel, but keep our ShareUpdateExclusiveLock on it
! 		 * until xact commit.  That will prevent someone else from deleting or
! 		 * ALTERing the parent before the child is committed.
  		 */
  		heap_close(relation, NoLock);
  	}

Bruce Momjian

bruce@momjian.us

almost 11 years ago

In reply to: Etsuro Fujita (#1)

Re: Incorrect comment in tablecmds.c

On Thu, Oct 23, 2014 at 06:29:07PM +0900, Etsuro Fujita wrote:

I don't think that the lock level mentioned in the following comment in
MergeAttributes() in tablecmds.c is right, since that that function has
opened the relation with ShareUpdateExclusiveLock, not with
AccessShareLock. Patch attached.

1749 /*
1750 * Close the parent rel, but keep our AccessShareLock on it
until xact
1751 * commit. That will prevent someone else from deleting or
ALTERing
1752 * the parent before the child is committed.
1753 */
1754 heap_close(relation, NoLock);

Agreed, patch applied. Thanks.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

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

Etsuro Fujita

fujita.etsuro@lab.ntt.co.jp

almost 11 years ago

In reply to: Bruce Momjian (#2)

Re: Incorrect comment in tablecmds.c

On 2015/03/20 21:31, Bruce Momjian wrote:

On Thu, Oct 23, 2014 at 06:29:07PM +0900, Etsuro Fujita wrote:

I don't think that the lock level mentioned in the following comment in
MergeAttributes() in tablecmds.c is right, since that that function has
opened the relation with ShareUpdateExclusiveLock, not with
AccessShareLock. Patch attached.

1749 /*
1750 * Close the parent rel, but keep our AccessShareLock on it
until xact
1751 * commit. That will prevent someone else from deleting or
ALTERing
1752 * the parent before the child is committed.
1753 */
1754 heap_close(relation, NoLock);

Agreed, patch applied. Thanks.

Thanks for picking this up!

Best regards,
Etsuro Fujita

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