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

Re: BUG #6041: Unlogged table was created bad in slave node

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Euler Taveira de Oliveira <euler(at)timbira(dot)com>, Emanuel <postgres(dot)arg(at)gmail(dot)com>, pgsql-bugs <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #6041: Unlogged table was created bad in slave node
Date: 2011-06-07 21:00:43
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-bugspgsql-hackers
On Tue, Jun 7, 2011 at 3:53 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> I found a few other holes in my previous patch as well.  I think this
>> plugs them all, but it's hard to be sure there aren't any other calls
>> to RelationGetNumberOfBlocks() that could bomb out.
> [ squint... ]  Do we need those additional tests in plancat.c?  I
> haven't paid attention to whether we support unlogged indexes on logged
> tables, but if we do, protecting the RelationGetNumberOfBlocks() call is
> the least of your worries.  You ought to be fixing things so the planner
> won't consider the index valid at all (cf. the indisvalid test at line
> 165).

Right now, RelationNeedsWAL() is always the same for a table and for
an index belonging to that table.  That is, indexes on temporary
tables are temporary; indees on unlogged tables are unlogged; indexes
on permanent tables are permanent.  But I agree that's something we'll
have to deal with if and when someone implements unlogged indexes on
logged tables.  (Though frankly I hope someone will come up with a
better name for that; else it's going to be worse than
constraint_exclusion vs. exclusion constraints.)

> Similarly, the change in estimate_rel_size seems to be at an
> awfully low level, akin to locking the barn door after the horses are
> out.  What code path are you thinking will reach there on an unlogged
> table?

Well, it gets there; I found this out empirically.
get_relation_info() calls it in two different places.  Actually, I see
now that the v3 patch has a few leftovers: the test in
estimate_relation_size() makes the first of the two checks in
get_relaton_info() redundant -- but the second hunk in
get_relation_info() is needed, because there it calls
RelationGetNumberOfBlocks() directly.  This is why I thought it might
be better to provide a version of RelationGetNumberOfBlocks() that
doesn't fail if the file is missing, instead of trying to plug these
holes one by one.

> It might be that it'd be best just to have both the planner and executor
> throwing errors on unlogged tables, rather than rejiggering pieces of
> the planner to sort-of not fail on an unlogged table.

Mmm, that's not a bad thought either.  Although I think if we can be
certain that the planner will error out, the executor checks aren't
necessary.  It would disallow preparing a statement and then executing
it after promotion, but that doesn't seem terribly important.  Any
idea where to put the check?

Robert Haas
The Enterprise PostgreSQL Company

In response to


pgsql-hackers by date

Next:From: Tom LaneDate: 2011-06-07 21:01:05
Subject: Re: [Pgbuildfarm-members] CREATE FUNCTION hang on test machine polecat on HEAD
Previous:From: Robert HaasDate: 2011-06-07 20:52:43
Subject: Re: reducing the overhead of frequent table locks - now, with WIP patch

pgsql-bugs by date

Next:From: Tom LaneDate: 2011-06-07 21:05:06
Subject: Re: BUG #6041: Unlogged table was created bad in slave node
Previous:From: Tom LaneDate: 2011-06-07 20:07:51
Subject: Re: Re: BUG #6050: Dump and restore of view after a schema change: can't restore the view

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