Skip site navigation (1) Skip section navigation (2)

Re: Removal of temp tables

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Removal of temp tables
Date: 2001-06-14 17:25:37
Message-ID: 200106141725.f5EHPbb11824@candle.pha.pa.us (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Here is an updated patch that uses underscores in temp table names so
> > the DROP doesn't have to quote the table name:
> 
> That seems like a reasonable idea, but don't do it to temp file
> names, ie, drop this part of the diff:
> 
> > *** src/backend/storage/file/fd.c	2001/06/11 04:12:29	1.81
> > --- src/backend/storage/file/fd.c	2001/06/14 16:34:02
> > ***************
> > *** 756,762 ****
> >   	 * transaction and database instance.
> >   	 */
> >   	snprintf(tempfilepath, sizeof(tempfilepath),
> > ! 			 "%s/%s%d.%ld", PG_TEMP_FILES_DIR, PG_TEMP_FILE_PREFIX,
> >   			 MyProcPid, tempFileCounter++);
>   
> >   	/*
> > --- 756,762 ----
> >   	 * transaction and database instance.
> >   	 */
> >   	snprintf(tempfilepath, sizeof(tempfilepath),
> > ! 			 "%s/%s%d_%ld", PG_TEMP_FILES_DIR, PG_TEMP_FILE_PREFIX,
> >   			 MyProcPid, tempFileCounter++);
>   
> >   	/*
> 
> There's no reason to spell temp file names as if they were rel names,
> and probably it's best not to make them look the same.

I was wondering that.  The old vacuum file detection patch had the sort
files going into /pg_sorttemp and files called pid_.  Your changes made
it pg_tempfile directory and pg_temp file names.  I like the older names
that made them clear they were _not_ temp tables.  Seemed you wanted
them to have similar names for reasons I couldn't figure.

I don't care if the sort files have dots so I will remove that part of
the patch, but I think we should consider making the sort files more
different than they are now --- dots vs. underscores.


> Also, an item I've ranted about before:
> 
> > + #define is_temp_relname(relname) \
> > + 		(!strncmp(relname, PG_TEMP_REL_PREFIX, strlen(PG_TEMP_REL_PREFIX)))
> 
> It's bad style to treat the result of strcmp or strncmp as though it
> were a boolean, cf
> http://fts.postgresql.org/db/mw/msg.html?mid=68294
> Write (strncmp(...) == 0) instead.

OK, changed.

> Otherwise the patch seems reasonable, although I wonder what your
> motivation was for choosing these particular IsSystemRelationName calls
> to tweak.  It looks like you did more than the minimum needed to allow
> a DROP TABLE; why these extra ones and not others?  (Not that I'm
> encouraging you to go around and hit every IsSystemRelationName call.
> If you did, that'll just be more changes that I suspect we'll have to
> remove again in the long run.  I'm just curious why you touched, for
> example, VACUUM.)

I removed the vacuum part.  I added it because it looked particularly
bad to do REINDEX on temp tables but I have no reason to know that for
sure.

Patch attached.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman(at)candle(dot)pha(dot)pa(dot)us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

In response to

Responses

pgsql-hackers by date

Next:From: Peter EisentrautDate: 2001-06-14 17:39:18
Subject: What (not) to do in signal handlers
Previous:From: Tom LaneDate: 2001-06-14 17:13:04
Subject: Re: Removal of temp tables

pgsql-patches by date

Next:From: Stephen RamseyDate: 2001-06-14 17:29:02
Subject: possible patch to increase number of hash overflow pages?
Previous:From: Tom LaneDate: 2001-06-14 17:13:04
Subject: Re: Removal of temp tables

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group