Re: patch for temporary view from TODO list

Neil Conway <neilc(at)samurai(dot)com> writes:
> On Mon, 2004-09-27 at 09:39 +1000, Koju Iijima wrote:
>> I have implemented temporary view related items as I proposed few days ago.
>> http://archives.postgresql.org/pgsql-hackers/2004-09/msg00682.php

> Barring any objections, I'll apply this patch tomorrow.

Some minor quibbles:

The references to "CASCADED" (sic) in the patch are surely bogus.

"tempViewWalker" in view.c is bereft of either comments or a usefully
descriptive name. Yeah, you find out what it is supposed to do when you
reach the routine below, but the whole thing is poorly presented. Waste
a static declaration forward reference so you can put the documented
routine first, and rename the walker to something based on the calling
routine's name.

"explicitely" and "implicitely" are not wel speled.

+ ereport(NOTICE,
+ (errmsg("view \"%s\" will be created in a temporary schema",
+ view->relname)));

This seems to me to be exposing an implementation issue without need,
ie, the average user does not care that we use temp schemas to implement
temp tables. Why not

+ (errmsg("view \"%s\" will be a temporary view",

(Possibly the documentation patches should be adjusted similarly.)

Does the gram.y change really require breaking out OR REPLACE as a
separate production, and if so why? That strikes me as something
that should not be necessary ... if it is then there is some deeper
problem to investigate.

The regression tests seem a tad excessive --- I'll grant this is a
matter of taste, but if every tiny little feature we have were tested
like that, the regression tests would take days to run.

[ If I were applying this I'd just editorialize on these things without
comment, but if you want to do it then you get to do the cleanup... ]

regards, tom lane