SEO

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------


Gregory Stark wrote:
>
> So based on the feedback and suggestions here this is the interface I suggest:
>
> \connect& - to open a new connection keeping the existing one
> \g& - to submit a command asynchronously (like & in the shell)
> \S [Sess#] - to _S_witch to a different _S_ession
> - if no connection # specified list available _S_essions
> \D - _D_isconnect from current session (like ^D in the shell)
>
> This leaves no way to submit an asynchronous command without using \g but I'm
> really not too concerned with that. I don't want to start messing with psql's
> semicolon parsing behaviour and I'm mainly only concerned with this for
> regression tests.
>
> Another thought I had for the future is a \C command to simulate C-c and send
> a query cancel. That would let us have regression tests that query
> cancellation worked. The tests would presumably have to be written using
> pg_sleep() to ensure they ran for long enough but even then there would be no
> way to control exactly when the interrupt arrived.
>
> Attached is an updated patch.
>
> I also found and fixed some missing ResetCancelConn()s. I think I got them all
> and the behaviour seems correct in practice when cancelling various
> combinations of synchronous queries, asynchronous queries, and backslash
> commands. The one thing I wonder about is that I'm a bit concerned I may have
> introduced an assumption about how many resultsets arrive from a single query.
>
> I'll be offline for a few days but I'll be back Monday.
>

[ Attachment, skipping... ]

>
>
> --
> Gregory Stark
> EnterpriseDB http://www.enterprisedb.com
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org

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

+ If your life is a hard drive, Christ can be your backup. +

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

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

Pavan Deolasee wrote:
> --- 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.

I believe it's for security reasons. If that memory page is for example
swapped to disk after freeing the field, the password would be written
to the swapfile. Someone who steals your laptop would be able to recover
it from there. Clearing passwords from memory when they're no longer
needed is a good practice.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend