Re: [HACKERS] gettext calls in pgport
Andrew Dunstan wrote: Tom Lane wrote: Somebody just yesterday stuck an fprintf(stderr,...); exit(1) into one of the pgport routines. This sucks, but there is not a lot else that can be done if the code needs to exist in both backend and clients. It'd be better to propagate the error condition back to the caller. An alternative possibility is to stop pretending that pgport is agnostic about whether it is in backend or frontend. This might mean some duplication of code between src/port/ and src/backend/port/, but if that's what it takes to have sane error handling, that's what we should do. Maybe you're referring to the patch I sent in to strip the .exe suffix in get_progname? ;-) I wondered about that. The choices on strdup() error seemed to be: . ignore the error and return the unstripped path, knowing the program would fail in a minute on the next malloc call anyway . return NULL and patch the code in about 20 places (of which one is the backend) where get_progname() is called . print a message and exit I can see arguments for all of these ;-) I added a comment to the exit() call: exit(1);/* This could exit the postmaster */ This clearly marks that this could be a postmaster issue someday. -- 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: [HACKERS] gettext calls in pgport
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: Somebody just yesterday stuck an fprintf(stderr,...); exit(1) into one of the pgport routines. This sucks, but there is not a lot else that can be done if the code needs to exist in both backend and clients. It'd be better to propagate the error condition back to the caller. Maybe you're referring to the patch I sent in to strip the .exe suffix in get_progname? ;-) Yeah, that was it. I wondered about that. The choices on strdup() error seemed to be: . ignore the error and return the unstripped path, knowing the program would fail in a minute on the next malloc call anyway . return NULL and patch the code in about 20 places (of which one is the backend) where get_progname() is called . print a message and exit Given the limited uses of get_progname, I think that print a message and exit is fine; the problem is that the correct implementation of that differs between backend and clients. The only really correct way to log a message in the backend is elog/ereport --- there's no guarantee that stderr connects to anything but /dev/null. Going directly to exit() instead of proc_exit() is simply broken (although perhaps the distinction does not matter, since the postmaster will treat exit(1) as a backend crash and force a database-wide reset). If I thought that this code path was ever likely to actually be taken in the field, I'd be hollering much more loudly about it. Actually the backend never calls get_progname(), only the postmaster does. I added a comment in path.c and postmaster.c about the possible call to exit. I think this cleans it up as well as possible. -- 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: [HACKERS] gettext calls in pgport
Peter Eisentraut wrote: Am Montag, 18. Oktober 2004 19:43 schrieb Tom Lane: An alternative possibility is to stop pretending that pgport is agnostic about whether it is in backend or frontend. This might mean some duplication of code between src/port/ and src/backend/port/, but if that's what it takes to have sane error handling, that's what we should do. The original plan for libpgport was to be a repository of functions that replace missing operating system functionality, like libiberty. I would have have liked to be able to lift these functions into other projects without complications. That implies that these functions should certainly not care about anything that by definition goes on above the operating system level. Now the directory has grown into a sort of general repository of code that is shared between more than one part of the PostgreSQL source tree, without any regard for well-defined interfaces. If you need to do that, please put it elsewhere, where only the involved parts see it. Not now, but in the future. So we will have a pgport and a pgshare? I don't see a huge value in that. Ideally, yea, they are different concepts but practially it seems like a waste to make the distinction. -- 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: [HACKERS] gettext calls in pgport
Am Montag, 18. Oktober 2004 19:43 schrieb Tom Lane: An alternative possibility is to stop pretending that pgport is agnostic about whether it is in backend or frontend. This might mean some duplication of code between src/port/ and src/backend/port/, but if that's what it takes to have sane error handling, that's what we should do. The original plan for libpgport was to be a repository of functions that replace missing operating system functionality, like libiberty. I would have have liked to be able to lift these functions into other projects without complications. That implies that these functions should certainly not care about anything that by definition goes on above the operating system level. Now the directory has grown into a sort of general repository of code that is shared between more than one part of the PostgreSQL source tree, without any regard for well-defined interfaces. If you need to do that, please put it elsewhere, where only the involved parts see it. Not now, but in the future. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
[HACKERS] gettext calls in pgport
As has previously been pointed out, the strings marked up for gettext translation in the pgport library don't work and need to be moved back to where they once came from, unless someone wants to add gettext and locale setup in pgport. (That might be silly, because in theory locale and gettext functions could one day become part of pgport.) I claim that pgport is a low-layer library that works on the operating system level and should communicate with its callers via error codes. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] gettext calls in pgport
Peter Eisentraut wrote: As has previously been pointed out, the strings marked up for gettext translation in the pgport library don't work and need to be moved back to where they once came from, unless someone wants to add gettext and locale setup in pgport. (That might be silly, because in theory locale and gettext functions could one day become part of pgport.) I claim that pgport is a low-layer library that works on the operating system level and should communicate with its callers via error codes. Error codes seem like a lot more work than it is worth. I vote for adding gettext support to /port. Also adding error codes duplicates all the error strings in the call sites. Added to open items list: * Add gettext support to src/port -- 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: [HACKERS] gettext calls in pgport
Bruce Momjian [EMAIL PROTECTED] writes: Error codes seem like a lot more work than it is worth. I vote for adding gettext support to /port. Also adding error codes duplicates all the error strings in the call sites. Added to open items list: * Add gettext support to src/port He who controls the TODO list dictates the solutions, eh? I tend to agree with Peter, actually: it would be better to pull error reporting issues out of pgport. Somebody just yesterday stuck an fprintf(stderr,...); exit(1) into one of the pgport routines. This sucks, but there is not a lot else that can be done if the code needs to exist in both backend and clients. It'd be better to propagate the error condition back to the caller. An alternative possibility is to stop pretending that pgport is agnostic about whether it is in backend or frontend. This might mean some duplication of code between src/port/ and src/backend/port/, but if that's what it takes to have sane error handling, that's what we should do. 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: [HACKERS] gettext calls in pgport
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Error codes seem like a lot more work than it is worth. I vote for adding gettext support to /port. Also adding error codes duplicates all the error strings in the call sites. Added to open items list: * Add gettext support to src/port He who controls the TODO list dictates the solutions, eh? And I can update it too. New text is: * fix gettext support to src/port We need to fix it somehow. I tend to agree with Peter, actually: it would be better to pull error reporting issues out of pgport. Somebody just yesterday stuck an fprintf(stderr,...); exit(1) into one of the pgport routines. This sucks, but there is not a lot else that can be done if the code needs to exist in both backend and clients. It'd be better to propagate the error condition back to the caller. I have no problem propogating the error condition back to the caller, but to propogate the error code/reason back means you would need -1 == no file, -2 == permission violation, etc and you then have those strings back in the client. That was Peter's goal, I thought, but it goes against the idea of centralizing those strings. I suppose you could #define all the needed strings in an include file and use those defines when checking for the error return code, but you have to make sure you get them all right and adjust for new strings, and it doesn't seem worth it. In fact one interesting idea would be to pass a constant error string back to the caller to fprintf/elog if they want, but that doesn't get the strings out of /port, and it doesn't allow easy use of passing fprintf strings with arguments back to the caller. An alternative possibility is to stop pretending that pgport is agnostic about whether it is in backend or frontend. This might mean some duplication of code between src/port/ and src/backend/port/, but if that's what it takes to have sane error handling, that's what we should do. I tried that but the fix caused more uglyness than it prevented. -- 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: [HACKERS] gettext calls in pgport
Tom Lane wrote: Somebody just yesterday stuck an fprintf(stderr,...); exit(1) into one of the pgport routines. This sucks, but there is not a lot else that can be done if the code needs to exist in both backend and clients. It'd be better to propagate the error condition back to the caller. An alternative possibility is to stop pretending that pgport is agnostic about whether it is in backend or frontend. This might mean some duplication of code between src/port/ and src/backend/port/, but if that's what it takes to have sane error handling, that's what we should do. Maybe you're referring to the patch I sent in to strip the .exe suffix in get_progname? ;-) I wondered about that. The choices on strdup() error seemed to be: . ignore the error and return the unstripped path, knowing the program would fail in a minute on the next malloc call anyway . return NULL and patch the code in about 20 places (of which one is the backend) where get_progname() is called . print a message and exit I can see arguments for all of these ;-) cheers andrew ---(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: [HACKERS] gettext calls in pgport
Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: Somebody just yesterday stuck an fprintf(stderr,...); exit(1) into one of the pgport routines. This sucks, but there is not a lot else that can be done if the code needs to exist in both backend and clients. It'd be better to propagate the error condition back to the caller. Maybe you're referring to the patch I sent in to strip the .exe suffix in get_progname? ;-) Yeah, that was it. I wondered about that. The choices on strdup() error seemed to be: . ignore the error and return the unstripped path, knowing the program would fail in a minute on the next malloc call anyway . return NULL and patch the code in about 20 places (of which one is the backend) where get_progname() is called . print a message and exit Given the limited uses of get_progname, I think that print a message and exit is fine; the problem is that the correct implementation of that differs between backend and clients. The only really correct way to log a message in the backend is elog/ereport --- there's no guarantee that stderr connects to anything but /dev/null. Going directly to exit() instead of proc_exit() is simply broken (although perhaps the distinction does not matter, since the postmaster will treat exit(1) as a backend crash and force a database-wide reset). If I thought that this code path was ever likely to actually be taken in the field, I'd be hollering much more loudly about it. 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])
