On Sat, Jun 9, 2012 at 10:49 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@gmail.com> writes: >> Whee, testing is fun. Second try. > > I'm concerned by the fact that neither the original nor the new code > bother to test whether the relation is WAL-loggable. It may be that > ginbuildempty cannot be invoked for temp tables, but it still seems > like an oversight waiting to bite you. I'd be happier if there were > a RelationNeedsWAL test here. Come to think of it, the other > foobuildempty functions aren't checking this either.
That's a good thing, because if they were, it would be wrong. RelationNeedsWAL() tells you whether the main fork is WAL-logged, but the buildempty routines exist for the purpose of constructing the init fork, which should always be WAL-logged. > A related point is that most of the other existing calls to log_newpage > are covered by "if (XLogIsNeeded())" tests. It's not immediately > obvious why these two shouldn't be. After some reflection I think > that's correct, but probably the comments for log_newpage and > log_newpage_buffer need to explain the different WAL-is-needed tests > that apply to the two usages. > > (I'm also thinking that the XLogIsNeeded macro is very poorly named, > as the situations it should be used in are far more narrow than the > macro name suggests. Haven't consumed enough caffeine to think of > a better name, though.) XLogIsNeeded() is, indeed, not a very good name: it's telling us whether wal_level > minimal, and thus whether there might be a standby. When log_newpage() is used to rebuild a relation, we use WAL bypass whenever possible, since the whole relation will be removed if the transaction rolls back, but we must still log it if a standby exists. In contrast, the init forks need to make it to the standby regardless. I'm not sure that I agree that it's the job of log_newpage() to explain to people calling it on what things they must conditionalize WAL generation: after all, none of this is unique to new-page records. If we emit some other record while creating an index on the primary, we can skip that when !XLogIsNeeded(), too. Any operation we perform on any relation fork other than the init fork can be conditionalized on RelationNeedsWAL(), not just new-page records. The charter of the function is just to avoid duplicating the mumbo-jumbo that's required to emit the record. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
