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
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-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

Browse pgsql-hackers by date

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

Browse pgsql-patches by date

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