Re: [PATCHES] APC/socket fix (final?)
Thanks. I was getting to that, but hadn't started :-) Per our discussion off-list, I agree with this method, and the patch looks fine to me. //Magnus -Original Message- From: Claudio Natoli [mailto:[EMAIL PROTECTED] Sent: Thursday, March 25, 2004 2:07 AM To: '[EMAIL PROTECTED]' Subject: [PATCHES] APC/socket fix (final?) For application to HEAD. This should take care of most, if not all, cases where a backend can be interrupted in a blocking socket operation by a signal which itself performs a socket operation (which interacts badly with our APC-based signal implementation). --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see a href=http://www.memetrics.com/emailpolicy.html;http://www.me metrics.com/em ailpolicy.html/a ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] APC/socket fix (final?)
Claudio Natoli [EMAIL PROTECTED] writes: + #ifdef WIN32 +/* Interrupted by socket/APC interaction? */ +if (n 0 GetLastError() == ERROR_IO_PENDING) +errno = EINTR; + #endif This seems a bit schizophrenic; if you can assign to errno, why can't you read from it? Would look more consistent if the code looked like if (n 0 errno == ERROR_IO_PENDING) errno = EINTR; The problem is that winsock does not *set* the errno variable in the case when it's interrupted by an APC. errno is left at 0. GetLastError() is the win32 replacement for errno. In most cases errno is set correctly when you use the unix api functions, but not in this case (which is arguably a bug, but it's there nevertheless). If you use native win32 functions, you get GetLastError() set only. //Magnus ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] installdir patch for win32
Andrew Dunstan wrote: Every #ifdef WIN32 I see reduces my opinion of the quality of work being done for this port. Tom, I think in defense of Claudio and company, it should be noted that there has been a desire not to disturb existing functionality more than necessary, and that has led fairly obviously to use of #ifdef with some liberality. Agreed. When we have issues with special socket restriction in signal handlers on the platform, and things like that, there isn't much we can do except #ifdef WIN32. I have been surprised how few #ifdef's we have needed for this platform. I do like to see a comment at the top of WIN32-specific code so we know what OS limtiation we are working around, and I think that has been done. In fact, here is a count of WIN32 references in each file. You will find the backend has very few references, with most in interfaces or /bin, which makes sense. --- 1 ./backend/access/transam/slru.c 1 ./backend/access/transam/xlog.c 1 ./backend/commands/copy.c 4 ./backend/commands/dbcommands.c 2 ./backend/libpq/md5.c 1 ./backend/libpq/pqcomm.c 3 ./backend/libpq/pqsignal.c 8 ./backend/main/main.c 5 ./backend/postmaster/pgstat.c 13 ./backend/postmaster/postmaster.c 6 ./backend/utils/fmgr/dfmgr.c 3 ./backend/utils/init/findbe.c 1 ./backend/utils/init/miscinit.c 4 ./backend/postgres 12 ./bin/initdb/initdb.c 1 ./bin/pg_resetxlog/pg_resetxlog.c 1 ./bin/psql/bcc32.mak 10 ./bin/psql/command.c 6 ./bin/psql/common.c 3 ./bin/psql/common.h 2 ./bin/psql/copy.c 5 ./bin/psql/describe.c 8 ./bin/psql/help.c 1 ./bin/psql/input.c 4 ./bin/psql/mainloop.c 1 ./bin/psql/mainloop.h 1 ./bin/psql/mbprint.c 4 ./bin/psql/print.c 1 ./bin/psql/prompt.c 7 ./bin/psql/startup.c 1 ./bin/psql/win32.mak 1 ./bin/scripts/common.c 4 ./bin/scripts/print.c 1 ./bin/scripts/mbprint.c 4 ./include/c.h 1 ./include/miscadmin.h 1 ./include/pg_config.h.win32 3 ./include/pg_config_manual.h 5 ./include/port.h 1 ./include/rusagestub.h 1 ./include/libpq/hba.h 3 ./include/libpq/pqcomm.h 3 ./include/libpq/pqsignal.h 1 ./include/port/win32.h 1 ./include/utils/elog.h 1 ./interfaces/ecpg/include/sqlca.h 1 ./interfaces/ecpg/pgtypeslib/dt.h 2 ./interfaces/libpgtcl/win32.mak 1 ./interfaces/libpq/bcc32.mak 2 ./interfaces/libpq/fe-auth.c 2 ./interfaces/libpq/fe-exec.c 1 ./interfaces/libpq/fe-lobj.c 1 ./interfaces/libpq/fe-misc.c 8 ./interfaces/libpq/fe-print.c 1 ./interfaces/libpq/fe-protocol2.c 1 ./interfaces/libpq/fe-protocol3.c 6 ./interfaces/libpq/fe-connect.c 8 ./interfaces/libpq/fe-secure.c 2 ./interfaces/libpq/md5.c 1 ./interfaces/libpq/libpqdll.c 2 ./interfaces/libpq/win32.c 1 ./interfaces/libpq/pqexpbuffer.c 1 ./interfaces/libpq/pqsignal.c 1 ./interfaces/libpq/win32.mak 3 ./interfaces/libpq/libpq-int.h 2 ./interfaces/libpq/noblock.c 4 ./interfaces/libpq/path.c 2 ./interfaces/libpq/thread.c 4 ./interfaces/libpq/libpq.so.3.2 4 ./interfaces/libpq/libpq.so.3 4 ./interfaces/libpq/libpq.so 1 ./port/crypt.c 3 ./port/dirmod.c 1 ./port/getrusage.c 2 ./port/noblock.c 4 ./port/path.c 1 ./port/pgsleep.c 4 ./port/sprompt.c 2 ./port/thread.c 1 ./port/open.c 2 ./utils/dllinit.c -- 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
Re: [PATCHES] installdir patch for win32
Tom Lane wrote: If the problem is that we need PKGLIBDIR not to be frozen at compile time, then let's fix the problem for everybody, not add a pile of undocumented #ifdef WIN32 hacks. (And it is a problem for everybody; Happy to go with whatever you can suggest. However, will point out that this patch is refactoring for general use code which had already accepted and in the source base, and for which opinions had already been previously canvassed. Every #ifdef WIN32 I see reduces my opinion of the quality of work being done for this port. At risk of getting (further? :-) on your bad side, IMHO that is a specious metric. Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see a href=http://www.memetrics.com/emailpolicy.html;http://www.memetrics.com/em ailpolicy.html/a ---(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] APC/socket fix (final?)
Claudio Natoli [EMAIL PROTECTED] writes: It would be more consistent, but unfortunately GetLastError() != errno. Yeah, I saw Magnus' explanation. So essentially this is a workaround for a bug in Windows' select() emulation. Rather than hoping that we'll remember to decorate every select() call with this workaround, would it make sense to use a wrapper function? I'm loath to invent pg_select() but it might be cleaner than this. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] installdir patch for win32
Claudio Natoli [EMAIL PROTECTED] writes: Tom Lane wrote: Every #ifdef WIN32 I see reduces my opinion of the quality of work being done for this port. At risk of getting (further? :-) on your bad side, IMHO that is a specious metric. Well, it's surely not the only interesting metric, but I don't think it's specious. Every #ifdef poses a continuing load on future maintainers, who have to look at that code and think whether they need to worry about adjusting it when making nearby changes. To the extent that you can avoid ifdefs in favor of cleaner solutions (such as refactoring code, or solving a general problem instead of making a platform-specific change in behavior), you'll have more readable and more maintainable code. I'm not expecting to see zero ifdefs --- certainly not in the port modules ;-). But Bruce's search, further up in the thread, showed that #ifdef WIN32's are sneaking into a lot of modules that probably shouldn't have any platform dependencies. I don't think that's a good sign. We should be working to keep those dependencies localized. 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] APC/socket fix (final?)
Claudio Natoli [EMAIL PROTECTED] writes:
I'm loath to invent pg_select() but it might be cleaner than this.
We'd also need pg_recv() and pg_send(). Chances are it can happen with every
blocking socket call :-(
Ugh. Is there a way we can insert a wrapper layer without modifying the
call sites? I'm thinking of some kind of macro hack, say
#ifdef WIN32
#define select(...) pg_select(...)
#endif
and then provide a port module that goes roughly like
#undef select
pg_select(...)
{
foo = select(...);
// fix errno here;
return foo;
}
The fewer places that have to know about this sort of thing, the better
off we will be.
regards, tom lane
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] APC/socket fix (final?)
Ugh. Is there a way we can insert a wrapper layer without modifying the call sites? I'm thinking of some kind of macro hack, say [snip] Sure. Think we've even done this before (also, prevents developers needing to remember to use pg_*). The reason I think it was avoided for select(), in preference for a thread to invoke the socket op during the signal/APC, was a fear that perhaps the Windows Sockets internals could get mashed. AFAICS, the discussion Magnus had with the Microsoft guys (and, from memory, those I've had with Magnus off-list) suggests this isn't true. If mashing the internals is still a possibility, then clearly the patch I've submitted might do more harm than good. (Magnus, can you confirm?) If so, I'll submit a patch for select/recv/send over the weekend, which will also remove the recent fixes for pgstat. Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see a href=http://www.memetrics.com/emailpolicy.html;http://www.memetrics.com/em ailpolicy.html/a ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] installdir patch for win32
On Thu, Mar 25, 2004 at 07:57:19PM -0500, Tom Lane wrote: I'm not expecting to see zero ifdefs --- certainly not in the port modules ;-). But Bruce's search, further up in the thread, showed that #ifdef WIN32's are sneaking into a lot of modules that probably shouldn't have any platform dependencies. Don't forget about the EXEC_BACKEND businness ... is there any chance those could refactored somehow? -- Alvaro Herrera (alvherre[a]dcc.uchile.cl) Amanece. (Ignacio Reyes) El Cerro San Cristóbal me mira, cínicamente, con ojos de virgen ---(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
[PATCHES] documentation work
I've applied the attached patch to CVS HEAD. It makes a few more SQL statement references into cross-references and makes some other minor improvements to the documentation. -Neil more_xref_work-3.patch Description: Binary data ---(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] installdir patch for win32
I'm not expecting to see zero ifdefs --- certainly not in the port modules ;-). But Bruce's search, further up in the thread, showed that #ifdef WIN32's are sneaking into a lot of modules that probably shouldn't have any platform dependencies. For the most part, I disagree (in fact, I was surprised to see how few there were). The bulk come from include, bin, interfaces, port... with almost all of the first three hits of these existing prior to the port. With regards to the backend files, a healthy number of these are pre-existing, or exist only to include/exclude header files + struct members, or simply avoid things that are unix specific. I recall a few potential culprits lurking in pgstat + postmaster (like the pipe() + win32_fork, which could be shipped off to /port), but nothing that would substantially impact the number of #ifdefs. I don't think that's a good sign. We should be working to keep those dependencies localized. On this, I agree, and if you can point me towards any that you find particularly obnoxious (on-list or otherwise), I'll gladly fix them. Seriously. From my POV, the WIN32 specific areas should be clear and obvious, and present no great maintenance challenge. This attitude, however, does not extend to some of the EXEC_BACKEND areas, but this is also where we are obviously the most hamstrung. As you probably remember, I've already undertaken to refactor this section of the backend startup code when the dust settles, but some degree of pain will be had here for as long as we choose to support platforms without fork(). Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see a href=http://www.memetrics.com/emailpolicy.html;http://www.memetrics.com/em ailpolicy.html/a ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] [NOT] (LIKE|ILIKE) (ANY|ALL) (...)
Fabien COELHO [EMAIL PROTECTED] writes: Please find attached a patch which allows LIKE/ILIKE/NOT LIKE/NOT ILIKE as operators for ANY/SOME/ALL constructs. This seems to allow a whole lot of unintended and probably uncool things as well. ORDER BY NOT LIKE, for instance. Yes. Well, it seemed to me (maybe I'm wrong here/) that ORDER BY !~~ was allowed anyway by the parser, so I cannot see why it should not allow NOT LIKE as well, even if it does not make sense. I guess that it is filtered out later anyway? Or the rule factorization must be changed. It can also be done. -- Fabien Coelho - [EMAIL PROTECTED] ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
