[PATCHES] entab vs. gcc4 again

2005-05-05 Thread Marko Kreen
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

2005-05-05 Thread Tom Lane
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

2005-05-05 Thread Bruce Momjian

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 [, ...] ]