Re: Removal of temp tables

Here is a patch that allows people to delete the pg_temp* tables used as
temp tables. They are left around after a backend crash and the only
way previously to remove them was to start postgres with the -O override
option.

I am wondering if pg_temp tables should even be seen as system tables by
IsSystemRelationName(). We have to call them pg_ so user applications
don't display them, but other than that they aren't like system tables.
Comments?

There are no tests to see if the table is actually in use. I can add
them if people want it.

---------------------------------------------------------------------------

test=> CREATE TEMP TABLE test (x int);
CREATE
test=> \dS
List of relations
Name | Type | Owner
----------------+---------+----------
...
pg_temp.6682.0 | table | postgres
...
test=> DROP TABLE "pg_temp.6682.0";
DROP

--
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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> I am wondering if pg_temp tables should even be seen as system tables by
> IsSystemRelationName(). We have to call them pg_ so user applications
> don't display them, but other than that they aren't like system tables.
> Comments?

This oughta be discussed on pghackers, not just -patches. But my
thought is that we need a three-way distinction; at least some of the
IsSystemRelation checks presumably *should* accept temp relnames, else
we'd not have decided to do it that way in the first place.

Another point is that when we implement schemas (= Real Soon Now, I
trust), the whole business of temprels having different physical and
logical relnames will go away anyhow. Temp rels will become plain rels
that live in a temp schema. So it may not be worth adding further
complexity to support the present approach. We'll just have to rip
it out again ... better to expend the work on making schemas.

regards, tom lane

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > I am wondering if pg_temp tables should even be seen as system tables by
> > IsSystemRelationName(). We have to call them pg_ so user applications
> > don't display them, but other than that they aren't like system tables.
> > Comments?
>
> This oughta be discussed on pghackers, not just -patches. But my
> thought is that we need a three-way distinction; at least some of the
> IsSystemRelation checks presumably *should* accept temp relnames, else
> we'd not have decided to do it that way in the first place.
>
> Another point is that when we implement schemas (= Real Soon Now, I
> trust), the whole business of temprels having different physical and
> logical relnames will go away anyhow. Temp rels will become plain rels
> that live in a temp schema. So it may not be worth adding further
> complexity to support the present approach. We'll just have to rip
> it out again ... better to expend the work on making schemas.

OK, the patch allows temp tables to be used in the places that need to
drop them. Not sure what else is needed. We don't want people creating
them tables with names like pg_temp because it could conflict with an
existing backend so it seems we will just need to open it up in special
places.

Other comments?

--
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

Tom Lane writes:

> Another point is that when we implement schemas (= Real Soon Now, I
> trust), the whole business of temprels having different physical and
> logical relnames will go away anyhow. Temp rels will become plain rels
> that live in a temp schema.

I don't think this is the right solution. You should be able to create
temporary tables in any schema you choose (and have permission to). After
all, temporary tables are supposed to act like real tables expect for the
"temporary" aspect.

--
Peter Eisentraut peter_e(at)gmx(dot)net http://funkturm.homeip.net/~peter

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> Tom Lane writes:
>> ... the whole business of temprels having different physical and
>> logical relnames will go away anyhow. Temp rels will become plain rels
>> that live in a temp schema.

> I don't think this is the right solution. You should be able to create
> temporary tables in any schema you choose (and have permission to). After
> all, temporary tables are supposed to act like real tables expect for the
> "temporary" aspect.

If we did that, temp table names would conflict with real table names.
The lack of conflicts is one of the (few ;-)) features of our current
temp table implementation that I really like. Furthermore, I read in
SQL92 sec 4.9

... because global temporary ta-
ble contents are distinct within SQL-sessions, and created local
temporary tables are distinct within <module>s within SQL-sessions,
the effective <schema name> of the schema in which the global tem-
porary table or the created local temporary table is instantiated
is an implementation-dependent <schema name> that may be thought
of as having been effectively derived from the <schema name> of
the schema in which the global temporary table or created local
temporary table is defined and the implementation-dependent SQL-
session identifier associated with the SQL-session. In addition,
the effective <schema name> of the schema in which the created
local temporary table is instantiated may be thought of as being
further qualified by a unique implementation-dependent name associ-
ated with the <module> in which the created local temporary table
is referenced.

This appears to me to require that it be done in the way I suggest:
temp tables live in their own per-backend schema, or schemas.

regards, tom lane

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > I am wondering if pg_temp tables should even be seen as system tables by
> > IsSystemRelationName(). We have to call them pg_ so user applications
> > don't display them, but other than that they aren't like system tables.
> > Comments?
>
> This oughta be discussed on pghackers, not just -patches. But my
> thought is that we need a three-way distinction; at least some of the
> IsSystemRelation checks presumably *should* accept temp relnames, else
> we'd not have decided to do it that way in the first place.
>
> Another point is that when we implement schemas (= Real Soon Now, I
> trust), the whole business of temprels having different physical and
> logical relnames will go away anyhow. Temp rels will become plain rels
> that live in a temp schema. So it may not be worth adding further
> complexity to support the present approach. We'll just have to rip
> it out again ... better to expend the work on making schemas.

Here is an updated patch that uses underscores in temp table names so
the DROP doesn't have to quote the table name:

pg_temp_7199_0

I will apply this in two days if there are no other comments.

--
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

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.

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.

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.)

regards, tom lane

> 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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> 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'm not wedded to those names; if you have a better idea, let's hear it.
I changed it because I didn't like the use of the word "sort"; temp
files are not used only for sorts, but for several other things, and so
I wanted to see them called temp files not sorttemp files. But if you
want to change them so that they look even less like the logical names
of temp relations, that's OK with me.

regards, tom lane

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > 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'm not wedded to those names; if you have a better idea, let's hear it.
> I changed it because I didn't like the use of the word "sort"; temp
> files are not used only for sorts, but for several other things, and so
> I wanted to see them called temp files not sorttemp files. But if you
> want to change them so that they look even less like the logical names
> of temp relations, that's OK with me.

OK, I will do that. What else do we do with them except sorts? Seems
pid_ was a good file name because they are always based on pid in
storage/file/fd.c. The directory could be called simply 'tempfile' with
no pg_. How is that?

--
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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> What else do we do with them except sorts?

Hash joins. Materialize nodes. Not to mention that sorting is used for
things that aren't obviously sorts (SELECT count(distinct foo), for
example).

> Seems
> pid_ was a good file name because they are always based on pid in
> storage/file/fd.c. The directory could be called simply 'tempfile' with
> no pg_. How is that?

You had that to begin with, and I changed it because I thought it was a
bad idea. The directory name and file name should both make perfectly
clear that the files are temp files belonging to Postgres. For example,
it would be unsafe to make pg_tempfiles be a symlink pointing to a temp
directory shared with other apps if there was any risk of temp file name
collisions. (Not sure you'd do that anyway, because of security issues,
but let's not foreclose it with a poor choice of file names.) A purely
numeric file name for temp files is a particularly bad idea because it
looks too much like our numeric names for table data files. Don't
eliminate a hypothetical confusion factor between relnames and filenames
(which are never seen in the same context anyway) by introducing one
between filenames and other filenames.

If you don't like pg_temp here, maybe post_temp? pgsql_temp?

regards, tom lane

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > What else do we do with them except sorts?
>
> Hash joins. Materialize nodes. Not to mention that sorting is used for
> things that aren't obviously sorts (SELECT count(distinct foo), for
> example).

Oh, I didn't know that. OK.

> > Seems
> > pid_ was a good file name because they are always based on pid in
> > storage/file/fd.c. The directory could be called simply 'tempfile' with
> > no pg_. How is that?
>
> You had that to begin with, and I changed it because I thought it was a
> bad idea. The directory name and file name should both make perfectly
> clear that the files are temp files belonging to Postgres. For example,
> it would be unsafe to make pg_tempfiles be a symlink pointing to a temp
> directory shared with other apps if there was any risk of temp file name
> collisions. (Not sure you'd do that anyway, because of security issues,
> but let's not foreclose it with a poor choice of file names.) A purely
> numeric file name for temp files is a particularly bad idea because it
> looks too much like our numeric names for table data files. Don't
> eliminate a hypothetical confusion factor between relnames and filenames
> (which are never seen in the same context anyway) by introducing one
> between filenames and other filenames.

OK, I see, you think it could share a directory with another app. Now I
see why you used pg_temp.

>
> If you don't like pg_temp here, maybe post_temp? pgsql_temp?

OK, pgsql_temp works for me. Everytime I see pg_ I think system table.
I will use pgsql_temp for directory and file names.

--
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

> You had that to begin with, and I changed it because I thought it was a
> bad idea. The directory name and file name should both make perfectly
> clear that the files are temp files belonging to Postgres. For example,
> it would be unsafe to make pg_tempfiles be a symlink pointing to a temp
> directory shared with other apps if there was any risk of temp file name
> collisions. (Not sure you'd do that anyway, because of security issues,
> but let's not foreclose it with a poor choice of file names.) A purely
> numeric file name for temp files is a particularly bad idea because it
> looks too much like our numeric names for table data files. Don't
> eliminate a hypothetical confusion factor between relnames and filenames
> (which are never seen in the same context anyway) by introducing one
> between filenames and other filenames.
>
> If you don't like pg_temp here, maybe post_temp? pgsql_temp?

What if I call the directory tmp or pgsql_tmp and the files
pgsql_pid_#.#?

--
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

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>> If you don't like pg_temp here, maybe post_temp? pgsql_temp?

> What if I call the directory tmp or pgsql_tmp and the files
> pgsql_pid_#.#?

Why are you so eager not to call temp files "temp"? If you want to
spell it "pgsql_tmp" or "pg_temporary" or something different from
"pg_temp", that's fine, but I really think the files ought to clearly
label themselves as temporary. "pid" is not a clear label. With a
name like that, people might even think they are lock files.

regards, tom lane

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> >> If you don't like pg_temp here, maybe post_temp? pgsql_temp?
>
> > What if I call the directory tmp or pgsql_tmp and the files
> > pgsql_pid_#.#?
>
> Why are you so eager not to call temp files "temp"? If you want to
> spell it "pgsql_tmp" or "pg_temporary" or something different from
> "pg_temp", that's fine, but I really think the files ought to clearly
> label themselves as temporary. "pid" is not a clear label. With a
> name like that, people might even think they are lock files.

I wanted to call them pid so people knew what the file number meant.

--
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

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> >> If you don't like pg_temp here, maybe post_temp? pgsql_temp?
>
> > What if I call the directory tmp or pgsql_tmp and the files
> > pgsql_pid_#.#?
>
> Why are you so eager not to call temp files "temp"? If you want to
> spell it "pgsql_tmp" or "pg_temporary" or something different from
> "pg_temp", that's fine, but I really think the files ought to clearly
> label themselves as temporary. "pid" is not a clear label. With a
> name like that, people might even think they are lock files.

OK, here is the new version with directories and files called pgsql_tmp.
That doesn't look like it holds temp tables.

--
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