[PATCHES] entab vs. gcc4 again
There was a patch applied against entab/halt.c that converts it away from KR varargs. But it's buggy, its makes halt() use second argment as format string, not first. Following patch fixes it, but also converts entab/entab.c away from KR. That is from my own patch against entab for the same problem, which got lost somewhere in -patches. -- marko Index: src/tools/entab/entab.c === RCS file: /opt/arc/cvs2/pgsql/src/tools/entab/entab.c,v retrieving revision 1.13 diff -u -c -r1.13 entab.c *** src/tools/entab/entab.c 7 Oct 2003 17:40:09 - 1.13 --- src/tools/entab/entab.c 5 May 2005 12:36:27 - *** *** 29,37 extern intoptind; int ! main(argc, argv) ! int argc; ! char**argv; { int tab_size = 8, min_spaces = 2, --- 29,35 extern intoptind; int ! main(int argc, char **argv) { int tab_size = 8, min_spaces = 2, Index: src/tools/entab/halt.c === RCS file: /opt/arc/cvs2/pgsql/src/tools/entab/halt.c,v retrieving revision 1.6 diff -u -c -r1.6 halt.c *** src/tools/entab/halt.c 15 Apr 2005 04:29:32 - 1.6 --- src/tools/entab/halt.c 5 May 2005 12:36:00 - *** *** 19,33 /*VARARGS*/ void ! halt(const char *path, ...) { va_list arg_ptr; ! char *format, ! *pstr; void(*sig_func) (); ! va_start(arg_ptr, path); ! format = va_arg(arg_ptr, char *); if (strncmp(format, PERROR, 6) != 0) vfprintf(stderr, format, arg_ptr); else --- 19,31 /*VARARGS*/ void ! halt(const char *format, ...) { va_list arg_ptr; ! const char *pstr; void(*sig_func) (); ! va_start(arg_ptr, format); if (strncmp(format, PERROR, 6) != 0) vfprintf(stderr, format, arg_ptr); else ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Cleaning up unreferenced table files
Bruce Momjian pgman@candle.pha.pa.us writes: Applied. Now that I've had a chance to look at it, this patch is thoroughly broken. Problems observed in a quick review: 1. It doesn't work at all for non-default tablespaces: it will claim that every file in such a tablespace is stale. The fact that it does that rather than failing entirely is accidental. It tries to read the database's pg_class in the target tablespace whether it's there or not. Because the system is still in recovery mode, the low-level routines allow the access to the nonexistent pg_class table to pass --- in fact they think they should create the file, so after it runs there's a bogus empty 1259 file in each such tablespace (which of course it complains about, too). The code then proceeds to think that pg_class is empty so of course everything draws a warning. 2. It's not robust against stale subdirectories of a tablespace (ie, subdirs corresponding to a nonexistent database) --- again, it'll try to read a nonexistent pg_class. Then it'll produce a bunch of off-target complaint messages. 3. It's assuming that relfilenode is unique database-wide, when no such assumption is safe. We only have a guarantee that it's unique tablespace-wide. 4. It fails to examine table segment files (such as nnn.1). These should be complained of when the nnn doesn't match any hash entry. 5. It will load every relfilenode value in pg_class into the hashtable whether it's meaningful or not. There should be a check on relkind. 6. I don't think relying on strtol to decide if a filename is entirely numeric is very safe. Note all the extra defenses in pg_atoi against various platform-specific misbehaviors of strtol. Personally I'd use a strspn test instead. 7. There are no checks for readdir failure (compare any other readdir loop in the backend). See also Simon Riggs' complaints that the circumstances under which it's done are pretty randomly selected. (One particular thing that I think is a bad idea is to do this in a standalone backend. Any sort of corruption in any db's pg_class would render it impossible to start up.) To fix the first three problems, and also avoid the performance problem of multiply rescanning a database's pg_class for each of its tablespaces, I would suggest that the hashtable entries be widened to RelFileNode structs (ie, db oid, tablespace oid, relfilenode oid). Then there should be one iteration over pg_database to learn the OIDs and default tablespaces of each database; with that you can read pg_class from its correct location for each database and load all the entries into the hashtable. Then you iterate through the tablespaces looking for stuff not present in the hashtable. You might also want to build a list or hashtable of known database OIDs, so that you can recognize a stale subdirectory immediately and issue a direct complaint about it without even recursing into it. regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] COPY CSV header line feature
Here is an updated version of this patch, with documentation changes. I have already updated the gram.y comment you suggested. --- Andrew Dunstan wrote: ammended patch attached. sorry for the oversight. I agree with Tom's remark - it's far too easy to miss this. cheers andrew Alvaro Herrera wrote: On Tue, Mar 15, 2005 at 08:55:36PM -0600, Andrew Dunstan wrote: Alvaro Herrera said: On Sun, Mar 13, 2005 at 06:32:20PM -0500, Andrew Dunstan wrote: The attached patch implements the previously discussed header line feature for CSV mode COPY. It is triggered by the keyword HEADER (blame Bruce - he chose it ;-) ). I think you should add the new reserved keyword to the unreserved_keywords list or some other. Please be more specific. I'll be happy to add in anything I've missed. The Postgres grammar classifies keywords in one of several lists, in order to make them available as names to users (column names, function names, etc). So each time you create a new keyword and add it to the keywords.c list, you have to add it to one of the lists on gram.y too. See gram.y line 7669 ff. I'd add a comment on this on gram.y: Index: gram.y === RCS file: /home/alvherre/cvs/pgsql/src/backend/parser/gram.y,v retrieving revision 2.484 diff -c -w -b -B -c -r2.484 gram.y *** gram.y 14 Mar 2005 00:19:36 - 2.484 --- gram.y 16 Mar 2005 03:12:48 - *** *** 327,333 /* * If you make any token changes, update the keyword table in * parser/keywords.c and add new keywords to the appropriate one of ! * the reserved-or-not-so-reserved keyword lists, below. */ /* ordinary key words in alphabetical order */ --- 327,334 /* * If you make any token changes, update the keyword table in * parser/keywords.c and add new keywords to the appropriate one of ! * the reserved-or-not-so-reserved keyword lists, below; search this ! * file for Name classification hierarchy. */ /* ordinary key words in alphabetical order */ ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/copy.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/copy.sgml,v retrieving revision 1.63 diff -c -c -r1.63 copy.sgml *** doc/src/sgml/ref/copy.sgml 4 Jan 2005 00:39:53 - 1.63 --- doc/src/sgml/ref/copy.sgml 6 May 2005 03:36:30 - *** *** 24,34 COPY replaceable class=parametertablename/replaceable [ ( replaceable class=parametercolumn/replaceable [, ...] ) ] FROM { 'replaceable class=parameterfilename/replaceable' | STDIN } [ [ WITH ] ! [ BINARY ] [ OIDS ] [ DELIMITER [ AS ] 'replaceable class=parameterdelimiter/replaceable' ] [ NULL [ AS ] 'replaceable class=parameternull string/replaceable' ] ! [ CSV [ QUOTE [ AS ] 'replaceable class=parameterquote/replaceable' ] [ ESCAPE [ AS ] 'replaceable class=parameterescape/replaceable' ] [ FORCE NOT NULL replaceable class=parametercolumn/replaceable [, ...] ] --- 24,35 COPY replaceable class=parametertablename/replaceable [ ( replaceable class=parametercolumn/replaceable [, ...] ) ] FROM { 'replaceable class=parameterfilename/replaceable' | STDIN } [ [ WITH ] ! [ BINARY ] [ OIDS ] [ DELIMITER [ AS ] 'replaceable class=parameterdelimiter/replaceable' ] [ NULL [ AS ] 'replaceable class=parameternull string/replaceable' ] ! [ CSV [ HEADER ] ! [ QUOTE [ AS ] 'replaceable class=parameterquote/replaceable' ] [ ESCAPE [ AS ] 'replaceable class=parameterescape/replaceable' ] [ FORCE NOT NULL replaceable class=parametercolumn/replaceable [, ...] ] *** *** 36,45 TO { 'replaceable class=parameterfilename/replaceable' | STDOUT } [ [ WITH ] [ BINARY ] [ OIDS ] [ DELIMITER [ AS ] 'replaceable class=parameterdelimiter/replaceable' ] [ NULL [ AS ] 'replaceable class=parameternull string/replaceable' ] ! [ CSV [ QUOTE [ AS ] 'replaceable class=parameterquote/replaceable' ] [ ESCAPE [ AS ] 'replaceable class=parameterescape/replaceable' ] [ FORCE QUOTE replaceable class=parametercolumn/replaceable [, ...] ]