Обсуждение: relpersistence and temp table
In 9.1, if a table is created using an explicit pg_temp qualification,
the pg_class.relpersistence is marked 'p', not 't'.
postgres=# CREATE TABLE pg_temp.temptable (i int4);
CREATE TABLE
postgres=# select relpersistence from pg_class where relname = 'temptable';relpersistence
----------------p
(1 row)
BUt the table does go away if I exit the session:
postgres=# \q
edb-pg ~/git-pg1 $ psql
psql (9.0.1, server 9.2devel)
WARNING: psql version 9.0, server version 9.2. Some psql features might not work.
Type "help" for help.
postgres=# select relpersistence from pg_class where relname = 'temptable';relpersistence
----------------
(0 rows)
So in that sense, it does work as a temp table, but the relpersistence
is not 't'. So, is the "temp"ness no longer identified by
pg_class.relpersistence now?
In RelationBuildLocalRelation(), previously, namespace was used to
determine if the table should be marked temporary:
/* it is temporary if and only if it is in my temp-table namespace */rel->rd_istemp =
isTempOrToastNamespace(relnamespace);
But in Master branch, now if I look at RelationBuildLocalRelation(),
there is no such logic to mark relpersistence.
Was this intentional or is it a bug?
Regards
-Amit Khandekar
On Fri, Jul 1, 2011 at 8:06 AM, Amit Khandekar <amit.khandekar@enterprisedb.com> wrote: > In 9.1, if a table is created using an explicit pg_temp qualification, > the pg_class.relpersistence is marked 'p', not 't'. > > postgres=# CREATE TABLE pg_temp.temptable (i int4); > CREATE TABLE > > postgres=# select relpersistence from pg_class where relname = 'temptable'; > relpersistence > ---------------- > p > (1 row) > > > BUt the table does go away if I exit the session: > > postgres=# \q > edb-pg ~/git-pg1 $ psql > psql (9.0.1, server 9.2devel) > WARNING: psql version 9.0, server version 9.2. > Some psql features might not work. > Type "help" for help. > > postgres=# select relpersistence from pg_class where relname = 'temptable'; > relpersistence > ---------------- > (0 rows) > > So in that sense, it does work as a temp table, but the relpersistence > is not 't'. So, is the "temp"ness no longer identified by > pg_class.relpersistence now? > > > In RelationBuildLocalRelation(), previously, namespace was used to > determine if the table should be marked temporary: > > /* it is temporary if and only if it is in my temp-table namespace */ > rel->rd_istemp = isTempOrToastNamespace(relnamespace); > > But in Master branch, now if I look at RelationBuildLocalRelation(), > there is no such logic to mark relpersistence. > > Was this intentional or is it a bug? That's a bug. Thanks for the report. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Jul 1, 2011 at 10:32 AM, Robert Haas <robertmhaas@gmail.com> wrote: > On Fri, Jul 1, 2011 at 8:06 AM, Amit Khandekar > <amit.khandekar@enterprisedb.com> wrote: >> In 9.1, if a table is created using an explicit pg_temp qualification, >> the pg_class.relpersistence is marked 'p', not 't'. > > That's a bug. Thanks for the report. OK, so I think the problem here is that, in 9.0, it was possible to figure out what value relistemp should take at a very late date, because it was entirely a function of the schema name. A temporary schema implies relistemp = true, while a non-temporary schema implies relistemp = false. However, in 9.1, that clearly won't do, since unlogged and permanent tables can share the same schema. Moreover, by the time we get as far as RelationBuildLocalRelation(), we've already made lots of other decisions based on relpersistence, so it seems that we need to make this correct as early as possible. It's not feasible to do that in the parser, because the creation namespace could also come from search_path: SET search_path = pg_temp; CREATE TABLE foo (a int); So it seems we can't fix this any earlier than RangeVarGetCreationNamespace(). In the attached patch, I took basically that approach, but created a new function RangeVarAdjustRelationPersistence() that does the actual adjusting (since de-constifying RangeVarGetCreationNamespace() didn't seem smart), plus adds a bunch of additional sanity-checking that I previously overlooked. Namely, it forbids: - creating unlogged tables in temporary schemas - creating relations in temporary schemas of other sessions On the other hand, it does allow CREATE TEMP TABLE pg_temp.foo(a int), which was somewhat pointlessly forbidden by previous releases. In short, the code now checks directly what it used to check by inference: that you're not creating a temporary table in a permanent schema, or the other way around. I also rearranged a few other bits of code to make sure that the appropriate fixups happen BEFORE we enforce the condition that temporary tables mustn't be created in security-restricted contexts. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Вложения
On Fri, Jul 1, 2011 at 1:59 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Fri, Jul 1, 2011 at 10:32 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Fri, Jul 1, 2011 at 8:06 AM, Amit Khandekar >> <amit.khandekar@enterprisedb.com> wrote: >>> In 9.1, if a table is created using an explicit pg_temp qualification, >>> the pg_class.relpersistence is marked 'p', not 't'. >> >> That's a bug. Thanks for the report. > > OK, so I think the problem here is that, in 9.0, it was possible to > figure out what value relistemp should take at a very late date, > because it was entirely a function of the schema name. A temporary > schema implies relistemp = true, while a non-temporary schema implies > relistemp = false. However, in 9.1, that clearly won't do, since > unlogged and permanent tables can share the same schema. Moreover, by > the time we get as far as RelationBuildLocalRelation(), we've already > made lots of other decisions based on relpersistence, so it seems that > we need to make this correct as early as possible. It's not feasible > to do that in the parser, because the creation namespace could also > come from search_path: > > SET search_path = pg_temp; > CREATE TABLE foo (a int); > > So it seems we can't fix this any earlier than > RangeVarGetCreationNamespace(). In the attached patch, I took > basically that approach, but created a new function > RangeVarAdjustRelationPersistence() that does the actual adjusting > (since de-constifying RangeVarGetCreationNamespace() didn't seem > smart), plus adds a bunch of additional sanity-checking that I > previously overlooked. Namely, it forbids: > > - creating unlogged tables in temporary schemas > - creating relations in temporary schemas of other sessions > > On the other hand, it does allow CREATE TEMP TABLE pg_temp.foo(a int), > which was somewhat pointlessly forbidden by previous releases. In > short, the code now checks directly what it used to check by > inference: that you're not creating a temporary table in a permanent > schema, or the other way around. > > I also rearranged a few other bits of code to make sure that the > appropriate fixups happen BEFORE we enforce the condition that > temporary tables mustn't be created in security-restricted contexts. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Fri, Jul 1, 2011 at 10:32 AM, Robert Haas <robertmhaas@gmail.com> wrote: > > On Fri, Jul 1, 2011 at 8:06 AM, Amit Khandekar > > <amit.khandekar@enterprisedb.com> wrote: > >> In 9.1, if a table is created using an explicit pg_temp qualification, > >> the pg_class.relpersistence is marked 'p', not 't'. > > > > That's a bug. ?Thanks for the report. > > OK, so I think the problem here is that, in 9.0, it was possible to > figure out what value relistemp should take at a very late date, > because it was entirely a function of the schema name. A temporary > schema implies relistemp = true, while a non-temporary schema implies > relistemp = false. However, in 9.1, that clearly won't do, since > unlogged and permanent tables can share the same schema. Moreover, by > the time we get as far as RelationBuildLocalRelation(), we've already > made lots of other decisions based on relpersistence, so it seems that > we need to make this correct as early as possible. It's not feasible > to do that in the parser, because the creation namespace could also > come from search_path: > > SET search_path = pg_temp; > CREATE TABLE foo (a int); > > So it seems we can't fix this any earlier than > RangeVarGetCreationNamespace(). In the attached patch, I took > basically that approach, but created a new function > RangeVarAdjustRelationPersistence() that does the actual adjusting > (since de-constifying RangeVarGetCreationNamespace() didn't seem > smart), plus adds a bunch of additional sanity-checking that I > previously overlooked. Namely, it forbids: > > - creating unlogged tables in temporary schemas > - creating relations in temporary schemas of other sessions > > On the other hand, it does allow CREATE TEMP TABLE pg_temp.foo(a int), > which was somewhat pointlessly forbidden by previous releases. In > short, the code now checks directly what it used to check by > inference: that you're not creating a temporary table in a permanent > schema, or the other way around. > > I also rearranged a few other bits of code to make sure that the > appropriate fixups happen BEFORE we enforce the condition that > temporary tables mustn't be created in security-restricted contexts. Does this affect tables created during 9.1 beta? I assume a server restart fixes all this, but I am just checking. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Jul 11, 2011, at 8:55 PM, Bruce Momjian <bruce@momjian.us> wrote: > Does this affect tables created during 9.1 beta? I assume a server > restart fixes all this, but I am just checking. Yes, I think a server restart will fix it, though there might be corner cases I'm not thinking of. ...Robert