Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli <[EMAIL PROTECTED]> writes: > Actually, if I was going to argue anything, I'd say that if a backend goes > nuts and zeroes the whole shmem segment you've probably some bigger things > to worry about (Aside: Would postgres actually recover from such an > occurence? BTW, I'd be pretty impressed if it did, but not all that > surprised ;-). It should, although there are limits (for instance, if someone is actively writing out a page of WAL at the same time the bogus backend comes by and zeroes that buffer, you might lose WAL entries for already-committed transactions, which would be unhappy-making). As a developer, though, I crash backends all the time, and I can say that this mechanism is indeed pretty robust. The postmaster never goes down (what, never? well, hardly ever) and it's *extremely* seldom that a crash results in on-disk corruption, because the postmaster generally manages to kill the other backends before any corruption in shared memory gets propagated to disk. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Tom Lane wrote: > What it comes down to is I don't want the postmaster to be keeping its > own state in shared memory --- that is, the array must be write-only > memory as far as the postmaster is concerned. If we eliminate the Ok. Well, I can appreciate things from that point of view. > postmaster's private DLList of backends, then the postmaster becomes > that much more vulnerable to corruption from a backend bug that leads to > trashing shared memory. To take just one example: backend A goes nuts, > zeroes the whole shmem segment, and then dumps core. How is the > postmaster going to kill the other backends so that it can begin the > recovery process? If there's no record of their PIDs left anywhere, > it's got a problem. The postmaster *needs* its own copy of that DLList. > > You might object that backend bugs could clobber the array and thus > interfere with cancel request processing, but that's not nearly as > critical a function. Actually, if I was going to argue anything, I'd say that if a backend goes nuts and zeroes the whole shmem segment you've probably some bigger things to worry about (Aside: Would postgres actually recover from such an occurence? BTW, I'd be pretty impressed if it did, but not all that surprised ;-). But I'll happily concede the point, and prove it by knocking up a patch for it over the weekend (unless anyone else particularly wants to). Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli <[EMAIL PROTECTED]> writes: > I wasn't suggesting that we somehow put the Dllist in shared memory. I was > advocating completely replacing the BackendList dlllist with this array, > under both Win32 and *nix case. Right, that's what I understood you to be proposing, and I dislike it on robustness grounds. One of the most important functions of the postmaster is to survive backend crashes --- as soon as you put any of its state where backends could clobber it, you compromise that function. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> I think the simplest way to make this work is to use an array that's >> 2*MaxBackend items long (corresponding to the max number of children the >> postmaster will fork). > Actually, now that I think about it, is even that big enough. There is a > reason BackendList is a list. In pathological situations, the postmaster > could be made to fork a much larger number than 2*MaxBackend simultaneous > children, although only this many will be allowed to become backends. > (I guess we could check the port->canAcceptConnections value, and not add > the backend to the array when == CAC_TOOMANY). Probably we could rearrange the logic to test for too-many-children before we even do the fork. It'd be a little uglier that way, but not out of the question. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli <[EMAIL PROTECTED]> writes: > Tom Lane: >> Per Jan's comment, there is no need to mess with the existing >> datastructure. I'd think more of *copying* the dllist into some array >> in shared memory. This is code that would only need to exist in the >> Windows port. > Also, let's get back to why we want this: to handle processCancelRequest in > the Win32 case. If this array is in Windows only, then we'll obviously need > two implementations of the processCancelRequest logic. [ shrug... ] If that's the only redundant code we end up with in this port, we'll have done pretty durn well. What it comes down to is I don't want the postmaster to be keeping its own state in shared memory --- that is, the array must be write-only memory as far as the postmaster is concerned. If we eliminate the postmaster's private DLList of backends, then the postmaster becomes that much more vulnerable to corruption from a backend bug that leads to trashing shared memory. To take just one example: backend A goes nuts, zeroes the whole shmem segment, and then dumps core. How is the postmaster going to kill the other backends so that it can begin the recovery process? If there's no record of their PIDs left anywhere, it's got a problem. The postmaster *needs* its own copy of that DLList. You might object that backend bugs could clobber the array and thus interfere with cancel request processing, but that's not nearly as critical a function. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Bruce Momjian: > > Tom Lane: > > > Per Jan's comment, there is no need to mess with the existing > > > datastructure. I'd think more of *copying* the dllist into some array > > > in shared memory. This is code that would only need to exist in the > > > Windows port. > > > > (I thought Jan was referring to the PGPROC struct) > > > > This just seems a little odd to me. I mean, they are going to be basically > > identical (they'll even use the same struct!). > > Seems you should use an array of "struct bkend" or "Backend" as storage > in shared memory. I think the problem with Dllist is that it is > dynamically allocated and I don't see how that could be done cleanly in > shared memory. I think that's why the array idea seems best. I wasn't suggesting that we somehow put the Dllist in shared memory. I was advocating completely replacing the BackendList dlllist with this array, under both Win32 and *nix case. (Is what I've been saying sound different in this light?) AFAICS, dumping the BackendList dlllist, and just completely replacing it with "PidCancel[]" in shared memory would be a pretty small hit, and give us the same code under *nix + Win32. > Looking up they key would have to be different for fork and fork/exec. Only if we choose to have this new PidCancel array, in addition to the existing BackendList, which looks to me to be entirely redundant. Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli wrote: > > Tom Lane: > > Per Jan's comment, there is no need to mess with the existing > > datastructure. I'd think more of *copying* the dllist into some array > > in shared memory. This is code that would only need to exist in the > > Windows port. > > (I thought Jan was referring to the PGPROC struct) > > This just seems a little odd to me. I mean, they are going to be basically > identical (they'll even use the same struct!). Seems you should use an array of "struct bkend" or "Backend" as storage in shared memory. I think the problem with Dllist is that it is dynamically allocated and I don't see how that could be done cleanly in shared memory. I think that's why the array idea seems best. > Also, let's get back to why we want this: to handle processCancelRequest in > the Win32 case. If this array is in Windows only, then we'll obviously need > two implementations of the processCancelRequest logic. > > Or I'm missing something... Looking up they key would have to be different for fork and fork/exec. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Tom Lane wrote: > I think the simplest way to make this work is to use an array that's > 2*MaxBackend items long (corresponding to the max number of children the > postmaster will fork). Actually, now that I think about it, is even that big enough. There is a reason BackendList is a list. In pathological situations, the postmaster could be made to fork a much larger number than 2*MaxBackend simultaneous children, although only this many will be allowed to become backends. (I guess we could check the port->canAcceptConnections value, and not add the backend to the array when == CAC_TOOMANY). > There is no race condition possible for the ProcessCancelRequest case --- > the sub-postmaster that spawned an active backend must have found its > entry before it could have sent the cancel key to the client, so any > valid cancel request from a client must reference an already-existing > entry in the array. Make sense. Great. Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Tom Lane: > Per Jan's comment, there is no need to mess with the existing > datastructure. I'd think more of *copying* the dllist into some array > in shared memory. This is code that would only need to exist in the > Windows port. (I thought Jan was referring to the PGPROC struct) This just seems a little odd to me. I mean, they are going to be basically identical (they'll even use the same struct!). Also, let's get back to why we want this: to handle processCancelRequest in the Win32 case. If this array is in Windows only, then we'll obviously need two implementations of the processCancelRequest logic. Or I'm missing something... Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Bruce Momjian <[EMAIL PROTECTED]> writes: > Uh, why would you need more than maxbackends? Can't it be indexed by > slot number --- each backend has a slot? Maybe I am missing something. The postmaster has no way to know what slot number each backend will get. For that matter, a sub-postmaster doesn't know yet either. I think the simplest way to make this work is to use an array that's 2*MaxBackend items long (corresponding to the max number of children the postmaster will fork). Establish the convention that unused entries are zero. Then: 1. On forking a child, the postmaster scans the array for a free (zero) slot, and stashes the cancel key and PID there (in that order). 2. On receiving a child-termination report, the postmaster scans the array for the corresponding entry, and zeroes it out (PID first). (Obviously these algorithms could be improved if they turn out to be bottlenecks, but for the first cut KISS is applicable.) 3. To find or check a cencel key, a sub-postmaster scans the array looking for the desired PID (either its own, or the one it got from an incoming cancel request message). There is a potential race condition if a sub-postmaster scans the array before the postmaster has been able to store its PID there. I think it is sufficient for the sub-postmaster to sleep a few milliseconds and try again if it can't find its own PID in the array. There is no race condition possible for the ProcessCancelRequest case --- the sub-postmaster that spawned an active backend must have found its entry before it could have sent the cancel key to the client, so any valid cancel request from a client must reference an already-existing entry in the array. regards, tom lane ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli <[EMAIL PROTECTED]> writes: > Just to be clear, this would involve turning the BackendList dlllist into an > array in shared memory, right? If so, a couple of questions: Per Jan's comment, there is no need to mess with the existing datastructure. I'd think more of *copying* the dllist into some array in shared memory. This is code that would only need to exist in the Windows port. > - the postmaster makes all calls referencing this list, with the exception > of processCancelRequest, correct? The postmaster writes the array (and really would have no need to read it). Sub-postmasters would need to read the array, either to check an incoming cancel request or to get the current-session key value to pass back to the client. I don't think that PostgresMain or any subsidiary routine would ever need to touch it. regards, tom lane ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli wrote:
>
> I wrote:
> > Just to be clear, this would involve turning the BackendList dlllist into
> an
> > array in shared memory, right? If so, a couple of questions:
>
> Bruce Momjian wrote:
> > I assumed a much simpler solution. I thought we would just have:
> >
> > struct {
> > pid_t pid;
> > int cancel_key;
> > } PidCancel[maxbackend];
> >
> > in shared memory and we would just sequentially scan looking for a pid
> > match? Is that wrong?
>
> Isn't that basically "turning the BackendList dlllist into an array in
> shared memory"? And I don't think that an array length of maxbackend is
> enough.
Uh, why would you need more than maxbackends? Can't it be indexed by
slot number --- each backend has a slot? Maybe I am missing something.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
I wrote:
> Just to be clear, this would involve turning the BackendList dlllist into
an
> array in shared memory, right? If so, a couple of questions:
Bruce Momjian wrote:
> I assumed a much simpler solution. I thought we would just have:
>
> struct {
> pid_t pid;
> int cancel_key;
> } PidCancel[maxbackend];
>
> in shared memory and we would just sequentially scan looking for a pid
> match? Is that wrong?
Isn't that basically "turning the BackendList dlllist into an array in
shared memory"? And I don't think that an array length of maxbackend is
enough.
Cheers,
Claudio
---
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see
http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em
ailpolicy.html
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] Proposed replacement for pipe under Win32
I have applied your new pipe.c file, the macros you listed (with
modification), and the configure glue to make it work.
Patch and new file attached and applied.
---
Claudio Natoli wrote:
>
> To go in src/port/ directory.
>
> This is to allow the handles returned by pipe to be used in select() calls
> (which Win32 doesn't allow with the native pipe() call), thereby obviating
> the need for reworking of the select() mechanisms in pgstat.c.
>
> Additionally, something of the sort would be required at the top of pgstat.c
>
> #ifdef WIN32
> #define pipe(a) pgpipe(a)
> #define write(a,b,c) send(a,b,c,0)
> #define read(a,b,c) recv(a,b,c,0)
> #endif
>
> For reference, see this thread:
> http://archives.postgresql.org/pgsql-hackers/2003-12/msg00650.php
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: configure
===
RCS file: /cvsroot/pgsql-server/configure,v
retrieving revision 1.319
diff -c -c -r1.319 configure
*** configure 23 Dec 2003 18:40:51 - 1.319
--- configure 9 Jan 2004 03:48:13 -
***
*** 12227,12233
case $host_os in mingw*)
LIBOBJS="$LIBOBJS dirmod.$ac_objext"
LIBOBJS="$LIBOBJS copydir.$ac_objext"
! LIBOBJS="$LIBOBJS gettimeofday.$ac_objext" ;;
esac
if test "$with_readline" = yes; then
--- 12227,12234
case $host_os in mingw*)
LIBOBJS="$LIBOBJS dirmod.$ac_objext"
LIBOBJS="$LIBOBJS copydir.$ac_objext"
! LIBOBJS="$LIBOBJS gettimeofday.$ac_objext"
! LIBOBJS="$LIBOBJS pipe.$ac_objext" ;;
esac
if test "$with_readline" = yes; then
Index: configure.in
===
RCS file: /cvsroot/pgsql-server/configure.in,v
retrieving revision 1.309
diff -c -c -r1.309 configure.in
*** configure.in23 Dec 2003 18:40:52 - 1.309
--- configure.in9 Jan 2004 03:48:14 -
***
*** 924,930
case $host_os in mingw*)
AC_LIBOBJ(dirmod)
AC_LIBOBJ(copydir)
! AC_LIBOBJ(gettimeofday) ;;
esac
if test "$with_readline" = yes; then
--- 924,931
case $host_os in mingw*)
AC_LIBOBJ(dirmod)
AC_LIBOBJ(copydir)
! AC_LIBOBJ(gettimeofday)
! AC_LIBOBJ(pipe) ;;
esac
if test "$with_readline" = yes; then
Index: src/backend/postmaster/pgstat.c
===
RCS file: /cvsroot/pgsql-server/src/backend/postmaster/pgstat.c,v
retrieving revision 1.51
diff -c -c -r1.51 pgstat.c
*** src/backend/postmaster/pgstat.c 6 Jan 2004 23:15:22 - 1.51
--- src/backend/postmaster/pgstat.c 9 Jan 2004 03:48:16 -
***
*** 135,140
--- 135,153
static void pgstat_recv_dropdb(PgStat_MsgDropdb *msg, int len);
static void pgstat_recv_resetcounter(PgStat_MsgResetcounter *msg, int len);
+ /*
+ *WIN32 doesn't allow descriptors returned by pipe() to be used in select(),
+ *so for that platform we use socket() instead of pipe().
+ */
+ #ifndef WIN32
+ #define pgpipe(a) pipe(a)
+ #define piperead(a,b,c) read(a,b,c)
+ #define pipewrite(a,b,c) write(a,b,c)
+ #else
+ /* pgpipe() is in /src/port */
+ #define piperead(a,b,c) recv(a,b,c,0)
+ #define pipewrite(a,b,c) send(a,b,c,0)
+ #endif
/*
* Public functions called from postmaster follow
***
*** 1380,1386
* two buffer processes competing to read from the UDP socket --- not
* good.
*/
! if (pipe(pgStatPipe) < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
--- 1393,1399
* two buffer processes competing to read from the UDP socket --- not
* good.
*/
! if (pgpipe(pgStatPipe) < 0)
{
ereport(LOG,
(errcode_for_socket_access(),
***
*** 1595,1601
while (nread < targetlen)
{
! len = read(readPipe,
((char *) &msg) + nread,
targetlen - nread);
if (len < 0)
--- 1608,1614
while (nread < targetlen)
{
! len = piperead(readPipe,
((char *) &msg) + nread,
targetlen - nread);
if (len < 0)
***
*** 1920,1926
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli wrote:
>
> > I think they are saying put the cancel key inside the existing shared
> > memory segment.
>
> Ok. Thanks.
>
> > I don't know when we actually attach to the main shared
> > memory sigment in the child, but it would have to be sooner than when we
> > need the cancel key.
>
> Yes. Currently, it happens too late.
>
> I'll put my hand up to have a go at this fixing this (and getting
> processCancelRequest to work under Win32/EXEC_BACKEND at the same time), if
> no one else particularly cares to.
>
> Just to be clear, this would involve turning the BackendList dlllist into an
> array in shared memory, right? If so, a couple of questions:
> - what is a suitably large size for this array (2 * MaxBackends, ala
> canAcceptConnections?)
> - the postmaster makes all calls referencing this list, with the exception
> of processCancelRequest, correct? So, as Tom was alluding to, no locking is
> required (or desired!), and we'll just need to be careful not to introduce a
> race condition into processCancelRequest.
I assumed a much simpler solution. I thought we would just have:
struct {
pid_t pid;
int cancel_key;
} PidCancel[maxbackend];
in shared memory and we would just sequentially scan looking for a pid
match? Is that wrong?
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Bruce Momjian wrote: Claudio Natoli wrote: Tom Lane writes: > Actually, on further reflection a separate array to store PIDs and cancel keys is probably a better idea. [snip] > I still think it's unnecessary to make a separate shmem segment for it, though. Why is that? Don't we need the backends to have access to it to do a cancel request? I think I've missed something here... I think they are saying put the cancel key inside the existing shared memory segment. I don't know when we actually attach to the main shared memory sigment in the child, but it would have to be sooner than when we need the cancel key. I said move it into the PGPROC structure. And keep the pid in both, the PGPROC structure and postmaster local memory. The backend attaches to the shared memory during AttachSharedMemoryAndSemaphores() ... where else? Jan -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
> I think they are saying put the cancel key inside the existing shared > memory segment. Ok. Thanks. > I don't know when we actually attach to the main shared > memory sigment in the child, but it would have to be sooner than when we > need the cancel key. Yes. Currently, it happens too late. I'll put my hand up to have a go at this fixing this (and getting processCancelRequest to work under Win32/EXEC_BACKEND at the same time), if no one else particularly cares to. Just to be clear, this would involve turning the BackendList dlllist into an array in shared memory, right? If so, a couple of questions: - what is a suitably large size for this array (2 * MaxBackends, ala canAcceptConnections?) - the postmaster makes all calls referencing this list, with the exception of processCancelRequest, correct? So, as Tom was alluding to, no locking is required (or desired!), and we'll just need to be careful not to introduce a race condition into processCancelRequest. Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli wrote: > > Tom Lane writes: > > Actually, on further reflection a separate array to store PIDs and > cancel keys is probably a better idea. > [snip] > > I still think it's unnecessary to make a separate shmem segment for it, > though. > > Why is that? Don't we need the backends to have access to it to do a cancel > request? I think I've missed something here... I think they are saying put the cancel key inside the existing shared memory segment. I don't know when we actually attach to the main shared memory sigment in the child, but it would have to be sooner than when we need the cancel key. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Tom Lane writes: > Actually, on further reflection a separate array to store PIDs and cancel keys is probably a better idea. [snip] > I still think it's unnecessary to make a separate shmem segment for it, though. Why is that? Don't we need the backends to have access to it to do a cancel request? I think I've missed something here... Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Tom Lane wrote: Jan Wieck <[EMAIL PROTECTED]> writes: It doesn't hurt to keep the locations and code as much in sync as possible. I think Tom's idea to move the information into the PGPROC entry is the winner and does not need any OS specific handling. Actually, on further reflection a separate array to store PIDs and cancel keys is probably a better idea. If we put this stuff in PGPROC then the postmaster will need to be able to obtain the ProcStructLock (or whatever it's called this week) in order to examine/modify that data structure. That gets us into the usual concerns about backend bugs locking up the postmaster, etc. But if it's a separate array then we can just have the rule that no one but the postmaster gets to write in it. I still think it's unnecessary to make a separate shmem segment for it, though. I'd like to avoid the additional shmem segment if possible. The postmaster can keep the stuff he needs in local memory. I did not mean to rip everything out of postmaster local memory, and that little bit of redundancy does not hurt. The pid's of processes aren't likely to change very often. Jan -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Jan Wieck <[EMAIL PROTECTED]> writes: > It doesn't hurt to keep the locations and code as much in sync as > possible. I think Tom's idea to move the information into the PGPROC > entry is the winner and does not need any OS specific handling. Actually, on further reflection a separate array to store PIDs and cancel keys is probably a better idea. If we put this stuff in PGPROC then the postmaster will need to be able to obtain the ProcStructLock (or whatever it's called this week) in order to examine/modify that data structure. That gets us into the usual concerns about backend bugs locking up the postmaster, etc. But if it's a separate array then we can just have the rule that no one but the postmaster gets to write in it. I still think it's unnecessary to make a separate shmem segment for it, though. regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Bruce Momjian wrote: Tom Lane wrote: > Claudio Natoli wrote: >> The only things I've thought of so far are: >> a) sticking the PID/cancel key list in shared mem [yeech] >> b) rearranging the entire cancel handling to occur in the postmaster [double >> yeech] (a) seems like the lesser of the available evils (unless someone has a bright idea for a plan C). Bruce Momjian <[EMAIL PROTECTED]> writes: > I think we need to move in the direction of a separate fork/exec-only > shared memory segment that holds the pids and cancel keys for all the > backends. That doesn't seem worth the trouble. I'd be inclined to just stick the cancel keys in the PGPROC structures (I believe the PIDs are already in there). The original motivation for keeping them only in postmaster local memory was to keep backend A's cancel key away from the prying eyes of backend B, but is there really any security added? Anyone who can inspect PGPROC hardly needs to know the cancel key --- he can just issue a SIGINT (or worse) directly to the target backend. Agreed. I was going for a separate one just to be paranoid. This will only be done for exec(), so I don't see a problem for normal Unix use anyway. It doesn't hurt to keep the locations and code as much in sync as possible. I think Tom's idea to move the information into the PGPROC entry is the winner and does not need any OS specific handling. Jan -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Tom Lane wrote: > > Claudio Natoli wrote: > >> The only things I've thought of so far are: > >> a) sticking the PID/cancel key list in shared mem [yeech] > >> b) rearranging the entire cancel handling to occur in the postmaster [double > >> yeech] > > (a) seems like the lesser of the available evils (unless someone has a > bright idea for a plan C). > > Bruce Momjian <[EMAIL PROTECTED]> writes: > > I think we need to move in the direction of a separate fork/exec-only > > shared memory segment that holds the pids and cancel keys for all the > > backends. > > That doesn't seem worth the trouble. I'd be inclined to just stick the > cancel keys in the PGPROC structures (I believe the PIDs are already in > there). The original motivation for keeping them only in postmaster > local memory was to keep backend A's cancel key away from the prying > eyes of backend B, but is there really any security added? Anyone who > can inspect PGPROC hardly needs to know the cancel key --- he can just > issue a SIGINT (or worse) directly to the target backend. Agreed. I was going for a separate one just to be paranoid. This will only be done for exec(), so I don't see a problem for normal Unix use anyway. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
> Claudio Natoli wrote: >> The only things I've thought of so far are: >> a) sticking the PID/cancel key list in shared mem [yeech] >> b) rearranging the entire cancel handling to occur in the postmaster [double >> yeech] (a) seems like the lesser of the available evils (unless someone has a bright idea for a plan C). Bruce Momjian <[EMAIL PROTECTED]> writes: > I think we need to move in the direction of a separate fork/exec-only > shared memory segment that holds the pids and cancel keys for all the > backends. That doesn't seem worth the trouble. I'd be inclined to just stick the cancel keys in the PGPROC structures (I believe the PIDs are already in there). The original motivation for keeping them only in postmaster local memory was to keep backend A's cancel key away from the prying eyes of backend B, but is there really any security added? Anyone who can inspect PGPROC hardly needs to know the cancel key --- he can just issue a SIGINT (or worse) directly to the target backend. regards, tom lane ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli wrote: > > > BTW, how are we going to do cancels in Windows-land? The sub-postmaster > > isn't gonna have access to the postmaster's list of child PIDs and > > cancel keys ... > > Good question (the Win32/EXEC_BACKEND case is #def'd out to issue an > altogether unhelpful abort(), so I know it is there). > > The only things I've thought of so far are: > a) sticking the PID/cancel key list in shared mem [yeech] > b) rearranging the entire cancel handling to occur in the postmaster [double > yeech] As I remember, the only per-backend value to be passed is the cancel key, and seeing that this is going to be a problem for postmaster too, I think we need to move in the direction of a separate fork/exec-only shared memory segment that holds the pids and cancel keys for all the backends. I will update the Win32 TODO list to mention this issue in more detail. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Tom Lane wrote: > I've just noticed another serious bit of breakage in CVS tip (though > this might be fixed by Claudio's pending patch that reverts a lot of > the code rearrangements). It's already been applied. Your push in the SubPostmaster direction was really useful; a lot cleaner, and fixed things that I wasn't even familiar enough with the code to know were broken. Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
> BTW, how are we going to do cancels in Windows-land? The sub-postmaster > isn't gonna have access to the postmaster's list of child PIDs and > cancel keys ... Good question (the Win32/EXEC_BACKEND case is #def'd out to issue an altogether unhelpful abort(), so I know it is there). The only things I've thought of so far are: a) sticking the PID/cancel key list in shared mem [yeech] b) rearranging the entire cancel handling to occur in the postmaster [double yeech] Any better ideas? Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Bruce Momjian <[EMAIL PROTECTED]> writes: > When you say sub-postmaster, you mean we fork, then process the cancel > request? Seems we might need special handling in there, yea. We fork before we even read the request. Otherwise an attacker can trivially lock up the postmaster by sending a partial startup packet. I've just noticed another serious bit of breakage in CVS tip (though this might be fixed by Claudio's pending patch that reverts a lot of the code rearrangements). There is a reason why the 7.4 code does on_exit_reset *immediately* after fork(), and it's not acceptable to rearrange the code so that that happens at some random later time. Otherwise, any problem in between leads to the child process executing the postmaster's on_exit routines, with such pleasant side effects as destroying the shmem segment. For similar reasons, you don't get to postpone setting IsUnderPostmaster true --- elog pays attention to the value of that flag, and will do the wrong thing if called in a child process that doesn't yet have it set. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
> Whoever changed this: My fault. Thanks for the catch Jan. >proc_exit(status); Should be proc_exit(0). My apologies, Claudio PS. I take it there is no regression test for this. Would this be a difficult one to knock up? --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > My guess is that this used to proc_exit the postgres backend, but now > > proc_exits the postmaster, but that is only a guess. > > No. This is post-fork (and had better remain so). The change causes > the sub-postmaster that has just finished handling a cancel request > to exit with nonzero status, which the postmaster then interprets > (correctly) as a child process crash. > > BTW, how are we going to do cancels in Windows-land? The sub-postmaster > isn't gonna have access to the postmaster's list of child PIDs and > cancel keys ... When you say sub-postmaster, you mean we fork, then process the cancel request? Seems we might need special handling in there, yea. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > My guess is that this used to proc_exit the postgres backend, but now > > proc_exits the postmaster, but that is only a guess. > > No. This is post-fork (and had better remain so). The change causes > the sub-postmaster that has just finished handling a cancel request > to exit with nonzero status, which the postmaster then interprets > (correctly) as a child process crash. > > BTW, how are we going to do cancels in Windows-land? The sub-postmaster > isn't gonna have access to the postmaster's list of child PIDs and > cancel keys ... I think the postmaster will have access to the cancel key and pid. It should work similar to Unix. However, I do have a win32 TODO item on "test cancel key", so we will not forget about it. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Bruce Momjian <[EMAIL PROTECTED]> writes: > My guess is that this used to proc_exit the postgres backend, but now > proc_exits the postmaster, but that is only a guess. No. This is post-fork (and had better remain so). The change causes the sub-postmaster that has just finished handling a cancel request to exit with nonzero status, which the postmaster then interprets (correctly) as a child process crash. BTW, how are we going to do cancels in Windows-land? The sub-postmaster isn't gonna have access to the postmaster's list of child PIDs and cancel keys ... regards, tom lane ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Tom Lane wrote:
> Jan Wieck <[EMAIL PROTECTED]> writes:
> > Something after 2003/11/20 enhanced the query cancel handling. Namely,
> > CVS tip now responds to a query cancel with a postmaster restart
> > canceling all queries. Could the fork/exec stuff be responsible for this?
>
> Whoever changed this:
>
> status = ProcessStartupPacket(port, false);
>
> if (status != STATUS_OK)
> return 0;/* cancel request processed, or error */
>
> to this:
>
> status = ProcessStartupPacket(port, false);
>
> if (status != STATUS_OK)
> {
> ereport(LOG,
> (errmsg("connection startup failed")));
> proc_exit(status);
> }
>
> is responsible. May we have an explanation of the thought process,
> if any?
My guess is that this used to proc_exit the postgres backend, but now
proc_exits the postmaster, but that is only a guess.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Jan Wieck <[EMAIL PROTECTED]> writes:
> Something after 2003/11/20 enhanced the query cancel handling. Namely,
> CVS tip now responds to a query cancel with a postmaster restart
> canceling all queries. Could the fork/exec stuff be responsible for this?
Whoever changed this:
status = ProcessStartupPacket(port, false);
if (status != STATUS_OK)
return 0;/* cancel request processed, or error */
to this:
status = ProcessStartupPacket(port, false);
if (status != STATUS_OK)
{
ereport(LOG,
(errmsg("connection startup failed")));
proc_exit(status);
}
is responsible. May we have an explanation of the thought process,
if any?
regards, tom lane
---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [PATCHES] [pgsql-hackers-win32] Win32 signal code - first try
> Also, do we want this one #ifdef:ed in every place, or just > #define it to nothing at all on non-windows platforms? I imagine the latter will be cleaner. > Anyway. Comments on these points in particular, and in the whole thing in general? Workable path? I think it looks great. Well done! Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Patch of intarray module in v7.4.1
"Korea PostgreSQL Users' Group" <[EMAIL PROTECTED]> writes:
> In 7.4.1, intarray module have a problme about equal operator (=).
> select * from table where intarray_column = '{}';
> above query make error.
Good catch. Patch applied --- thanks!
regards, tom lane
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match
Re: [PATCHES] psql \i handling ~ in specified file name
Andrew Dunstan wrote: > >>>As I remember, MSDOS uses the "~" to specify short versions of long file > >>>names. > >>> > >>> > >>> > >>I is not just msdos, it is cmd.exe which exists on (to my knowledge) all > >>versions of windows. For example: > >>Program Files == progra~1 > >> > >> > > > >Yes, I meant it is a hold-over from dealing with MS-DOS style 8.3 file > >names. > > > > > > This pattern should not cause tilde expansion on any platform, I > believe. Only a *leading* ~/ or ~username/ should be expanded. > > The normal "home" directory on a (modern) Windows machine is usually > "C:\Documents and Settings\username", IIRC. Yes, I understand, but on an OS that uses tilde so much inside the file name, do we want special meaning when it is leading the file name? -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization
Claudio Natoli wrote: Something after 2003/11/20 enhanced the query cancel handling. Namely, CVS tip now responds to a query cancel with a postmaster restart canceling all queries. Could the fork/exec stuff be responsible for this? Jan Hi Bruce, Claudio, where are we on this patch? I see an even newer version in the archives: http://archives.postgresql.org/pgsql-patches/2003-12/msg00361.php [Weird you and Google groups "missed" it!] Anyway, Tom has looked at your newest patch and it looks good to him. I'm happy with the patch from the link above being committed if Tom has no more comments. Was only waiting for a final nod from him. Once it is in the source tree, give me a couple days and I'll fire off a patch with the actual CreateProcess calls... and then we are off into Win32 signal land [shudder]. Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see href="http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
[PATCHES] Win32 signal code - first try
Hi! Here is a first sketch at Win32 signal handling. First a couple of comments: * This is just two files. It is not integrated with postgresql yet. * Uses named pipes. Shared mem was slightly faster, named pipes a lot cleaner. And the signal handlers themselves should not be performance critical, AFAICS. * Does *not* use user APCs. Why? Well, we had to use polling. And with user APCs we'd have to go into kernel mode (however briefly) on every poll. I replaced that with a simple counter that is checked. Thast way we don't slow down the main path of the program much. * It is then checked again while protected with a critical section, to make sure we don't setp on our own toes from two threads. * A user APC is queued on every signal anyway. This is only to break out of SleepEx/WaitForMultipleObjectsEx etc functions. Normally, we pick the signals up with polling. A couple of questions that need answering: * Where to put the polling. We need to find a reasonable limited number of places. They just need PG_POLL_SIGNALS() there to go. Also, do we want this one #ifdef:ed in every place, or just #define it to nothing at all on non-windows platforms? * Where to put the functions. Right now, it's just a separate file. Do we want to keep it in separate files, or put it in existing files with #ifdef:s? Or combination? * Function names - I haven't looked at naming conventions at all :-) * We need to replace any blocking waits (such as select() and sleep()) with "Ex functions" if we want signals to fire while waiting. Probably a bunch more things I forgot to write right now. Oh, and I've included two very small testfiles - "pgsignal_srvtest.c" which is a sample server and "pgkill.c" which is a sample client. Very ugly code, but it should show how it's intended to be used :-) Anyway. Comments on these points in particular, and in the whole thing in general? Workable path? //Magnus pgsignal_w32.h Description: pgsignal_w32.h pgsignal_w32.c Description: pgsignal_w32.c pgsignal_srvtest.c Description: pgsignal_srvtest.c pgkill.c Description: pgkill.c ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] psql \i handling ~ in specified file name
Bruce Momjian wrote: Joshua D. Drake wrote: As I remember, MSDOS uses the "~" to specify short versions of long file names. I is not just msdos, it is cmd.exe which exists on (to my knowledge) all versions of windows. For example: Program Files == progra~1 Yes, I meant it is a hold-over from dealing with MS-DOS style 8.3 file names. This pattern should not cause tilde expansion on any platform, I believe. Only a *leading* ~/ or ~username/ should be expanded. The normal "home" directory on a (modern) Windows machine is usually "C:\Documents and Settings\username", IIRC. cheers andrew ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] psql \i handling ~ in specified file name
As I remember, MSDOS uses the "~" to specify short versions of long file names. I is not just msdos, it is cmd.exe which exists on (to my knowledge) all versions of windows. For example: Program Files == progra~1 I think that is enough for us to say that we are best leaving '~' expansion only for Unix. We already dump COPY in a native Win32 format, so it seems we should handle special characters in a similar native way. I will add a comment to this affect in the source code. -- Command Prompt, Inc., home of Mammoth PostgreSQL - S/ODBC and S/JDBC Postgresql support, programming shared hosting and dedicated hosting. +1-503-222-2783 - [EMAIL PROTECTED] - http://www.commandprompt.com
Re: [PATCHES] psql \i handling ~ in specified file name
Zach Irmen wrote:
> Bruce Momjian wrote:
> > Here is a patch that handles "~" in all the file cases.
>
> Beat me to it. :)
>
> I do have a few issues that I was trying to sort out myself
> regarding this, but I guess now is as good a time as any to ask
> them here.
>
> First off, there should be a check after the malloc to make sure
> NULL wasn't returned in the expand_tilde function. I missed that
> one.
OK, test added. I see no way to recover from a malloc failure in this
case because we can't honor their specification of file name, so we have
to exit.
> Secondly, there are a couple of SQL commands (like COPY and
> LOAD) and psql commands handled outside command.c (like \copy)
> which also take filenames. I'm guessing that eventually you'll
> want substitution in those cases as well. So does this mean that
> the expand_tilde function probably should not be in command.c?
> Placing it in common.c seems the logical place to make it at
> least available to all the psql commands (\copy included).
Yes, seems like that will be required. Please use my attached version
to make the adjustments.
> And finally, I was wondering if arguments with leading pipes
> (e.g. "|~/file") should also get substituted.
Yep, that too.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/bin/psql/command.c
===
RCS file: /cvsroot/pgsql-server/src/bin/psql/command.c,v
retrieving revision 1.108
diff -c -c -r1.108 command.c
*** src/bin/psql/command.c 1 Dec 2003 22:21:54 - 1.108
--- src/bin/psql/command.c 8 Jan 2004 18:01:55 -
***
*** 65,70
--- 65,72
static bool do_connect(const char *new_dbname, const char *new_user);
static bool do_shell(const char *command);
+ static char *expand_tilde(char **filename);
+
/*--
* HandleSlashCmds:
*
***
*** 413,418
--- 415,421
else
{
fname = scan_option(&string, OT_NORMAL, NULL, true);
+ expand_tilde(&fname);
status = do_edit(fname, query_buf) ? CMD_NEWEDIT : CMD_ERROR;
free(fname);
}
***
*** 494,500
--- 497,506
if (!fname)
pset.gfname = NULL;
else
+ {
+ expand_tilde(&fname);
pset.gfname = xstrdup(fname);
+ }
free(fname);
status = CMD_SEND;
}
***
*** 531,536
--- 537,543
}
else
{
+ expand_tilde(&fname);
success = (process_file(fname) == EXIT_SUCCESS);
free(fname);
}
***
*** 602,607
--- 609,615
{
char *fname = scan_option(&string, OT_FILEPIPE, NULL, true);
+ expand_tilde(&fname);
success = setQFout(fname);
free(fname);
}
***
*** 653,658
--- 661,667
{
char *fname = scan_option(&string, OT_NORMAL, NULL, true);
+ expand_tilde(&fname);
success = saveHistory(fname ? fname : "/dev/tty");
if (success && !quiet && fname)
***
*** 771,776
--- 780,786
else
{
fname = scan_option(&string, OT_FILEPIPE, NULL, true);
+ expand_tilde(&fname);
if (!fname)
{
***
*** 1678,1683
--- 1688,1753
}
+ /* expand_tilde
+ *
+ * substitute '~' with HOME or '~username' with username's home dir
+ *
+ */
+ static char *
+ expand_tilde(char **filename)
+ {
+ if (!filename || !(*filename))
+ return NULL;
+
+ /* MSDOS uses tilde for short versions of long file names, so skip it. */
+ #ifndef WIN32
+
+ /* try tilde expansion */
+ if (**filename == '~')
+ {
+ char *fn;
+ char *home;
+ charoldp,
+ *p;
+ struct passwd *pw;
+
+ fn = *filename;
+ home = NULL;
+
+ p = fn + 1;
+ while (*p != '/' && *p != '\0')
+ p++;
+
+ oldp = *p;
+ *p = '\0';
+
+ if (*(fn + 1) == '\0')
+ home = getenv("HOME");
+ else if ((pw = getpwnam(fn + 1)) != NULL)
+ home = pw->pw_dir;
+
+
Re: [PATCHES] psql \i handling ~ in specified file name
Joshua D. Drake wrote: > > > > >As I remember, MSDOS uses the "~" to specify short versions of long file > >names. > > > I is not just msdos, it is cmd.exe which exists on (to my knowledge) all > versions of windows. For example: > Program Files == progra~1 Yes, I meant it is a hold-over from dealing with MS-DOS style 8.3 file names. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] psql \i handling ~ in specified file name
Zach Irmen wrote: > "Andrew Dunstan" <[EMAIL PROTECTED]> writes: > > Zach Irmen said: > > > Can I just ifndef WIN32 and not think about it? I'm not sure how that > > > would work either. > > > > > > > If we are going to have a Windows port I don't think we should treat it as > > a poor cousin. > > I guess I was thinking more about if it should be done as opposed to how it > would be done. On the one hand, I think '~' by itself has no meaning in a > normal Windows environment, so why should psql on Windows give it one? The > readline library on unix, which can be used by psql, interprets the tilde > and is the big reason why psql on unix should interpret the tilde as well. > On the other hand however, I can see consistency being important in that > giving '~' a meaning in psql should give it the same meaning regardless of > platform. As I remember, MSDOS uses the "~" to specify short versions of long file names. I think that is enough for us to say that we are best leaving '~' expansion only for Unix. We already dump COPY in a native Win32 format, so it seems we should handle special characters in a similar native way. I will add a comment to this affect in the source code. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] SIGPIPE handling
Manfred Spraul wrote:
> Bruce Momjian wrote:
>
> >>
> >>+ /*
> >>+* We could lose a signal during this test.
> >>+* In a multi-threaded application, this might
> >>+* be a problem. Do any non-threaded platforms
> >>
> Threaded or non-threaded?
>
> >>+* lack sigaction()?
> >>+*/
> >>
> Additionally, the problem is not restricted to multithreaded apps:
> signal(,SIG_IGN) clears all pending signals.
OK, new function using sigblock():
pqsigfunc
pqsignalinquire(int signo)
{
#if !defined(HAVE_POSIX_SIGNALS)
pqsigfunc old_sigfunc;
int old_sigmask;
/* Prevent signal handler calls during test */
old_sigmask = sigblock(sigmask(signo));
old_sigfunc = signal(signo, SIG_DFL);
signal(signo, old_sigfunc);
sigblock(old_sigmask);
return old_sigfunc;
#else
struct sigaction oact;
if (sigaction(signo, NULL, &oact) < 0)
return SIG_ERR;
return oact.sa_handler;
#endif /* !HAVE_POSIX_SIGNALS */
}
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match
Re: [PATCHES] SIGPIPE handling
Manfred Spraul wrote: > Bruce Momjian wrote: > > >> > >>+ /* > >>+* We could lose a signal during this test. > >>+* In a multi-threaded application, this might > >>+* be a problem. Do any non-threaded platforms > >> > Threaded or non-threaded? OK, yea, I will use threaded. > >>+* lack sigaction()? > >>+*/ > >> > Additionally, the problem is not restricted to multithreaded apps: > signal(,SIG_IGN) clears all pending signals. Oh, yuck. Would SIG_DFL be better here? I am thinking of adding sigblock into that code on the assumption that if they have signal(), they have sigblock(). Should we disable threaded builds unless they have sigaction()? I suppose the sigblock() would take care of the pending signal problem too. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
[PATCHES] fork/exec patch: CreateProcess calls for Win32
For application to HEAD, pending community review. Drops in the CreateProcess calls for Win32 (essentially wrapping up the fork/exec portion of the port), and fixes a handful of whitespace issues (that no doubt I'm to blame for; my apologies) in the source base. Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see http://www.memetrics.com/emailpolicy.html";>http://www.memetrics.com/em ailpolicy.html diff6c.out Description: Binary data ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
