[PATCHES] fork/exec patch: CreateProcess calls for Win32

2004-01-08 Thread Claudio Natoli
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

Re: [PATCHES] SIGPIPE handling

2004-01-08 Thread Bruce Momjian
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. +*

Re: [PATCHES] SIGPIPE handling

2004-01-08 Thread Bruce Momjian
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()? +*/

Re: [PATCHES] psql \i handling ~ in specified file name

2004-01-08 Thread Bruce Momjian
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

Re: [PATCHES] psql \i handling ~ in specified file name

2004-01-08 Thread Bruce Momjian
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

Re: [PATCHES] psql \i handling ~ in specified file name

2004-01-08 Thread Joshua D. Drake
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 '~'

[PATCHES] Win32 signal code - first try

2004-01-08 Thread Magnus Hagander
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Jan Wieck
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

Re: [PATCHES] psql \i handling ~ in specified file name

2004-01-08 Thread Bruce Momjian
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

Re: [PATCHES] Patch of intarray module in v7.4.1

2004-01-08 Thread Tom Lane
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

Re: [PATCHES] [pgsql-hackers-win32] Win32 signal code - first try

2004-01-08 Thread Claudio Natoli
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!

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Tom Lane
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 =

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Bruce Momjian
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:

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Bruce Momjian
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Bruce Momjian
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Claudio Natoli
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Tom Lane
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Claudio Natoli
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Tom Lane
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Bruce Momjian
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Jan Wieck
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Tom Lane
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Claudio Natoli
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Bruce Momjian
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Claudio Natoli
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.

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Jan Wieck
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Bruce Momjian
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.

Re: [PATCHES] Proposed replacement for pipe under Win32

2004-01-08 Thread Bruce Momjian
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.

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Claudio Natoli
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;

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Bruce Momjian
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 {

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Tom Lane
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Tom Lane
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Claudio Natoli
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Claudio Natoli
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Bruce Momjian
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Tom Lane
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

Re: [PATCHES] fork/exec patch: pre-CreateProcess finalization

2004-01-08 Thread Claudio Natoli
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.