Skip site navigation (1) Skip section navigation (2)

Re: Concurrent psql patch

From: "Pavan Deolasee" <pavan(dot)deolasee(at)gmail(dot)com>
To: "Bruce Momjian" <bruce(at)momjian(dot)us>
Cc: "Gregory Stark" <stark(at)enterprisedb(dot)com>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "David Fetter" <david(at)fetter(dot)org>, "Jim Nasby" <decibel(at)decibel(dot)org>, pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Concurrent psql patch
Date: 2007-05-18 11:59:44
Message-ID: 2e78013d0705180459o27d6c412xeb8c14c1bfcb2f53@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
Hi Greg,

I looked at the patch briefly. I couldn't spot any issues and it looks good
to me. I've just couple of comments here.

The mvcc regression test files are missing in the patch.

--- 1179,1189 ----
                              dbname, user, password);

        /* We can immediately discard the password -- no longer needed */
!       if (password)
!       {
!           memset(password, '\0', strlen(password));
            free(password);
+       }


Any reason why we do this ? "password" is anyways freed.  I think you
might have left it behind after some debugging exercise.


--- 25,37 ----
  #include "mb/pg_wchar.h"
  #include "mbprint.h"

+ #if 0
+ #include "libpq-int.h" /* For PG_ASYNC */
+ #endif
+

This looks redundant..

Apart from that I really like consistent coding style. For example, to me,
"for (i = 0; i < 10; i++)" looks much better than "for (i=0;i<10; i++)"

This is not comment on your patch  and neither I am saying
we should follow a specific coding style (though I wish we could have done
so) because we have already so many different styles. So its best to
stick to the coding style already followed in that particular file. But few
simple rules like having a single space around operators like '<', '+', '='
etc really makes the code more readable. Other examples are using
parenthesis in a right manner to improve code readability.

flag = (pointer == NULL); is more readable than
flag = pointer == NULL;

Thanks,
Pavan

-- 
Pavan Deolasee
EnterpriseDB     http://www.enterprisedb.com

In response to

Responses

pgsql-hackers by date

Next:From: Heikki LinnakangasDate: 2007-05-18 12:06:46
Subject: Re: Maintaining cluster order on insert
Previous:From: Martijn van OosterhoutDate: 2007-05-18 11:29:23
Subject: Re: [GSOC] - I ntegrity check algorithm for data files

pgsql-patches by date

Next:From: Heikki LinnakangasDate: 2007-05-18 12:06:46
Subject: Re: Maintaining cluster order on insert
Previous:From: Heikki LinnakangasDate: 2007-05-18 09:47:19
Subject: Re: Updated bitmap index patch

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group