On Sat, Nov 13, 2010 at 7:59 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> Here is a series of three patches related to unlogged tables.
>> 1. The first one (relpersistence-v1) is a mostly mechanical patch that
>> replaces pg_class.relistemp (a Boolean) with pg_class.relpersistence
>> (a character), so that we can support more than two values. BE SURE
>> YOU INITDB, since the old catalog format will not work with this patch
>> applied.
>
> While I'm griping ... is there a really good reason to do it that way,
> rather than adding a new column? This will break clients that are
> looking at relistemp. Maybe there aren't any, but I wouldn't bet on
> that, and it doesn't seem like you're buying a lot by creating this
> incompatibility. I would also argue that temp-ness is a distinct
> concept from logged-ness.
I think that would be a recipe for bugs. Look at the three new macros
I introduced. If you keep relistemp around, then any code which
relies on it is likely testing for one of those three things, or maybe
even something subtly different from any of them, as in the cases
where I needed to add a switch statement. The way I see it, this is
ultimately a four-level hierarchy: permanent tables (write WAL, shared
buffers, ordinary namespace), unlogged tables (don't write WAL, shared
buffers, ordinary namespace), global temporary tables (don't write
WAL, local buffers, ordinary namespace), local temporary tables (don't
write WAL, local buffers, private namespace). Even if we don't end up
implementing global temporary tables in the way I'm envisioning (I
know you have an alternate proposal), it seem inevitable that a
boolean for relistemp isn't going to be sufficient.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company