Re: pgindent (was Re: [HACKERS] Header comments in the recently added files)
On Thu, Mar 10, 2011 at 11:25 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, Mar 10, 2011 at 10:59 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Speaking of running scripts, I think we should run pgindent now. Yeah, +1 for doing it as soon as Tom is at a good stopping point. It makes things a lot simpler later on. IIRC the argument for an early pgindent run was to standardize the new code for easier review. I expect to be spending a whole lot of time reading collate and SSI code over the next few weeks, so I'm in favor of pgindent'ing that stuff first. But I guess we need the typedef list update before anything can happen. That's one good reason. Another is that this is presumably the time of the cycle when there are the fewest outstanding patches, making it a good time for changes that are likely to conflict with lots of other things. At any rate, it sounds like Andrew needs a few days to get the typedef list together, so let's wait for that to happen and then we'll see where we are. Andrew, any update on this? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Header comments in the recently added files
Tom Lane wrote: Itagaki Takahiro itagaki.takah...@gmail.com writes: I found trivial mistakes in the recently added files. Will they fixed by some automated batches, or by manual? - Copyright (c) xxx-*2010*, PostgreSQL Global Development Group in pg_collation.h, pg_foreign_table.h, basebackup.h, syncrep.h, pg_backup_directory.c and auth_delay.c. - IDENTIFICATION $PostgreSQL$ in pg_collation.h, syncrep.h, and syncrep.c Other files has their actual paths in the same place. It might be worth Bruce making another run of his copyright-update script to fix the former problems. As for the latter problems, I ran it just now and nothing was changed, so we are OK now. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
pgindent (was Re: [HACKERS] Header comments in the recently added files)
On Thu, Mar 10, 2011 at 10:21 AM, Bruce Momjian br...@momjian.us wrote: Tom Lane wrote: Itagaki Takahiro itagaki.takah...@gmail.com writes: I found trivial mistakes in the recently added files. Will they fixed by some automated batches, or by manual? - Copyright (c) xxx-*2010*, PostgreSQL Global Development Group in pg_collation.h, pg_foreign_table.h, basebackup.h, syncrep.h, pg_backup_directory.c and auth_delay.c. - IDENTIFICATION $PostgreSQL$ in pg_collation.h, syncrep.h, and syncrep.c Other files has their actual paths in the same place. It might be worth Bruce making another run of his copyright-update script to fix the former problems. As for the latter problems, I ran it just now and nothing was changed, so we are OK now. Speaking of running scripts, I think we should run pgindent now. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgindent (was Re: [HACKERS] Header comments in the recently added files)
Robert Haas wrote: On Thu, Mar 10, 2011 at 10:21 AM, Bruce Momjian br...@momjian.us wrote: Tom Lane wrote: Itagaki Takahiro itagaki.takah...@gmail.com writes: I found trivial mistakes in the recently added files. Will they fixed by some automated batches, or by manual? - Copyright (c) xxx-*2010*, PostgreSQL Global Development Group ? in pg_collation.h, pg_foreign_table.h, basebackup.h, syncrep.h, ? pg_backup_directory.c and auth_delay.c. - IDENTIFICATION $PostgreSQL$ in pg_collation.h, syncrep.h, and syncrep.c ? Other files has their actual paths in the same place. It might be worth Bruce making another run of his copyright-update script to fix the former problems. ?As for the latter problems, I ran it just now and nothing was changed, so we are OK now. Speaking of running scripts, I think we should run pgindent now. We usually do it during a late beta. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgindent (was Re: [HACKERS] Header comments in the recently added files)
Bruce Momjian br...@momjian.us writes: Robert Haas wrote: Speaking of running scripts, I think we should run pgindent now. We usually do it during a late beta. Last time we did it early and then again late, and that seemed to work well. I wouldn't object to a pgindent run now, but please sync with me before you do --- I've got some heavy hacking to do on the collations patch, and don't want to find myself trying to merge changes after a pgindent run. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgindent (was Re: [HACKERS] Header comments in the recently added files)
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Robert Haas wrote: Speaking of running scripts, I think we should run pgindent now. We usually do it during a late beta. Last time we did it early and then again late, and that seemed to work well. I wouldn't object to a pgindent run now, but please sync with me before you do --- I've got some heavy hacking to do on the collations patch, and don't want to find myself trying to merge changes after a pgindent run. Andrew needs to update the typedef list in our GIT tree first. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgindent (was Re: [HACKERS] Header comments in the recently added files)
On Thu, Mar 10, 2011 at 10:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Bruce Momjian br...@momjian.us writes: Robert Haas wrote: Speaking of running scripts, I think we should run pgindent now. We usually do it during a late beta. Last time we did it early and then again late, and that seemed to work well. I wouldn't object to a pgindent run now, but please sync with me before you do --- I've got some heavy hacking to do on the collations patch, and don't want to find myself trying to merge changes after a pgindent run. Yeah, +1 for doing it as soon as Tom is at a good stopping point. It makes things a lot simpler later on. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgindent (was Re: [HACKERS] Header comments in the recently added files)
On Thu, Mar 10, 2011 at 16:50, Robert Haas robertmh...@gmail.com wrote: On Thu, Mar 10, 2011 at 10:47 AM, Tom Lane t...@sss.pgh.pa.us wrote: Bruce Momjian br...@momjian.us writes: Robert Haas wrote: Speaking of running scripts, I think we should run pgindent now. We usually do it during a late beta. Last time we did it early and then again late, and that seemed to work well. I wouldn't object to a pgindent run now, but please sync with me before you do --- I've got some heavy hacking to do on the collations patch, and don't want to find myself trying to merge changes after a pgindent run. Yeah, +1 for doing it as soon as Tom is at a good stopping point. It makes things a lot simpler later on. Agreed. With git in play, it should be quite possible to merge with head just before the pgindent run, then run pgindent on your local topic branch, and then merge with head after the pgindent run - that should take care of *most* of the conflicts, I think - as long as you use the same typdef list. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgindent (was Re: [HACKERS] Header comments in the recently added files)
Robert Haas robertmh...@gmail.com writes: Speaking of running scripts, I think we should run pgindent now. Yeah, +1 for doing it as soon as Tom is at a good stopping point. It makes things a lot simpler later on. IIRC the argument for an early pgindent run was to standardize the new code for easier review. I expect to be spending a whole lot of time reading collate and SSI code over the next few weeks, so I'm in favor of pgindent'ing that stuff first. But I guess we need the typedef list update before anything can happen. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgindent (was Re: [HACKERS] Header comments in the recently added files)
On 03/10/2011 10:33 AM, Robert Haas wrote: On Thu, Mar 10, 2011 at 10:21 AM, Bruce Momjianbr...@momjian.us wrote: Tom Lane wrote: Itagaki Takahiroitagaki.takah...@gmail.com writes: I found trivial mistakes in the recently added files. Will they fixed by some automated batches, or by manual? - Copyright (c) xxx-*2010*, PostgreSQL Global Development Group in pg_collation.h, pg_foreign_table.h, basebackup.h, syncrep.h, pg_backup_directory.c and auth_delay.c. - IDENTIFICATION $PostgreSQL$ in pg_collation.h, syncrep.h, and syncrep.c Other files has their actual paths in the same place. It might be worth Bruce making another run of his copyright-update script to fix the former problems. As for the latter problems, I ran it just now and nothing was changed, so we are OK now. Speaking of running scripts, I think we should run pgindent now. Please wait a few days at least. I am just setting up a new server at this very moment (now that SL6 is released) and will get a new FBSD buildfarm member extracting typedefs running. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgindent (was Re: [HACKERS] Header comments in the recently added files)
On Thu, Mar 10, 2011 at 10:59 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Speaking of running scripts, I think we should run pgindent now. Yeah, +1 for doing it as soon as Tom is at a good stopping point. It makes things a lot simpler later on. IIRC the argument for an early pgindent run was to standardize the new code for easier review. I expect to be spending a whole lot of time reading collate and SSI code over the next few weeks, so I'm in favor of pgindent'ing that stuff first. But I guess we need the typedef list update before anything can happen. That's one good reason. Another is that this is presumably the time of the cycle when there are the fewest outstanding patches, making it a good time for changes that are likely to conflict with lots of other things. At any rate, it sounds like Andrew needs a few days to get the typedef list together, so let's wait for that to happen and then we'll see where we are. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: pgindent (was Re: [HACKERS] Header comments in the recently added files)
Tom Lane t...@sss.pgh.pa.us wrote:
I expect to be spending a whole lot of time reading collate and
SSI code over the next few weeks, so I'm in favor of pgindent'ing
that stuff first.
I've been running that throughout development, but it hasn't been
run after the last few changes. If you want the SSI files in
pgindent format, you can get there by applying the attached patch.
-Kevin
*** a/src/backend/storage/lmgr/predicate.c
--- b/src/backend/storage/lmgr/predicate.c
***
*** 746,755 OldSerXidAdd(TransactionId xid, SerCommitSeqNo
minConflictCommitSeqNo)
Assert(TransactionIdIsValid(tailXid));
/*
!* If the SLRU is currently unused, zero out the whole active region
!* from tailXid to headXid before taking it into use. Otherwise zero
!* out only any new pages that enter the tailXid-headXid range as we
!* advance headXid.
*/
if (oldSerXidControl-headPage 0)
{
--- 746,755
Assert(TransactionIdIsValid(tailXid));
/*
!* If the SLRU is currently unused, zero out the whole active region
from
!* tailXid to headXid before taking it into use. Otherwise zero out only
!* any new pages that enter the tailXid-headXid range as we advance
!* headXid.
*/
if (oldSerXidControl-headPage 0)
{
***
*** 855,862 OldSerXidSetActiveSerXmin(TransactionId xid)
/*
* When no sxacts are active, nothing overlaps, set the xid values to
* invalid to show that there are no valid entries. Don't clear
headPage,
!* though. A new xmin might still land on that page, and we don't want
!* to repeatedly zero out the same page.
*/
if (!TransactionIdIsValid(xid))
{
--- 855,862
/*
* When no sxacts are active, nothing overlaps, set the xid values to
* invalid to show that there are no valid entries. Don't clear
headPage,
!* though. A new xmin might still land on that page, and we don't
want to
!* repeatedly zero out the same page.
*/
if (!TransactionIdIsValid(xid))
{
***
*** 901,907 OldSerXidSetActiveSerXmin(TransactionId xid)
void
CheckPointPredicate(void)
{
! int tailPage;
LWLockAcquire(OldSerXidLock, LW_EXCLUSIVE);
--- 901,907
void
CheckPointPredicate(void)
{
! int tailPage;
LWLockAcquire(OldSerXidLock, LW_EXCLUSIVE);
***
*** 1317,1328 SummarizeOldestCommittedSxact(void)
/*
* This function is only called if there are no sxact slots available.
* Some of them must belong to old, already-finished transactions, so
!* there should be something in FinishedSerializableTransactions list
!* that we can summarize. However, there's a race condition: while we
!* were not holding any locks, a transaction might have ended and
cleaned
!* up all the finished sxact entries already, freeing up their sxact
!* slots. In that case, we have nothing to do here. The caller will find
!* one of the slots released by the other backend when it retries.
*/
if (SHMQueueEmpty(FinishedSerializableTransactions))
{
--- 1317,1328
/*
* This function is only called if there are no sxact slots available.
* Some of them must belong to old, already-finished transactions, so
!* there should be something in FinishedSerializableTransactions list
that
!* we can summarize. However, there's a race condition: while we were
not
!* holding any locks, a transaction might have ended and cleaned up all
!* the finished sxact entries already, freeing up their sxact slots. In
!* that case, we have nothing to do here. The caller will find one of
the
!* slots released by the other backend when it retries.
*/
if (SHMQueueEmpty(FinishedSerializableTransactions))
{
***
*** 2207,2213 PredicateLockTuple(const Relation relation, const HeapTuple
tuple)
*/
if (relation-rd_index == NULL)
{
! TransactionId myxid;
targetxmin = HeapTupleHeaderGetXmin(tuple-t_data);
--- 2207,2213
*/
if (relation-rd_index == NULL)
{
! TransactionId myxid;
targetxmin = HeapTupleHeaderGetXmin(tuple-t_data);
***
*** 2217, PredicateLockTuple(const Relation relation, const HeapTuple
tuple)
--- 2217,2223
if (TransactionIdFollowsOrEquals(targetxmin,
TransactionXmin))
{
TransactionId xid =
SubTransGetTopmostTransaction(targetxmin);
+
if (TransactionIdEquals(xid, myxid))
{
[HACKERS] Header comments in the recently added files
I found trivial mistakes in the recently added files. Will they fixed by some automated batches, or by manual? - Copyright (c) xxx-*2010*, PostgreSQL Global Development Group in pg_collation.h, pg_foreign_table.h, basebackup.h, syncrep.h, pg_backup_directory.c and auth_delay.c. - IDENTIFICATION $PostgreSQL$ in pg_collation.h, syncrep.h, and syncrep.c Other files has their actual paths in the same place. -- Itagaki Takahiro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Header comments in the recently added files
On Wed, Mar 9, 2011 at 8:33 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: I found trivial mistakes in the recently added files. Will they fixed by some automated batches, or by manual? - Copyright (c) xxx-*2010*, PostgreSQL Global Development Group in pg_collation.h, pg_foreign_table.h, basebackup.h, syncrep.h, pg_backup_directory.c and auth_delay.c. - IDENTIFICATION $PostgreSQL$ in pg_collation.h, syncrep.h, and syncrep.c Other files has their actual paths in the same place. I think these should be fixed manually. The first might eventually get fixed automatically, but perhaps not until 2012, and I'm not sure the second would ever get fixed automatically. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Header comments in the recently added files
On Thu, Mar 10, 2011 at 12:55, Robert Haas robertmh...@gmail.com wrote: On Wed, Mar 9, 2011 at 8:33 PM, Itagaki Takahiro itagaki.takah...@gmail.com wrote: I found trivial mistakes in the recently added files. Will they fixed by some automated batches, or by manual? I think these should be fixed manually. The first might eventually get fixed automatically, but perhaps not until 2012, and I'm not sure the second would ever get fixed automatically. OK, I'll do that. -- Itagaki Takahiro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Header comments in the recently added files
Itagaki Takahiro itagaki.takah...@gmail.com writes: I found trivial mistakes in the recently added files. Will they fixed by some automated batches, or by manual? - Copyright (c) xxx-*2010*, PostgreSQL Global Development Group in pg_collation.h, pg_foreign_table.h, basebackup.h, syncrep.h, pg_backup_directory.c and auth_delay.c. - IDENTIFICATION $PostgreSQL$ in pg_collation.h, syncrep.h, and syncrep.c Other files has their actual paths in the same place. It might be worth Bruce making another run of his copyright-update script to fix the former problems. As for the latter problems, I have a to-do item to go around and standardize the format of the header comments a bit better --- I'd like to have a uniform convention that we put the full path name of each file at the top, and then get rid of the vestigial IDENTIFICATION sections. Don't know quite when I'll get to it though. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
