Neil Conway <neilc@samurai.com> writes:
> On Tue, 2005-02-01 at 01:28 -0500, Tom Lane wrote:
>> The references to "CASCADED" (sic) in the patch are surely bogus.
> Per SQL:2003 section 11.22, it is spelt "CASCADED".
Maybe I'm reading the wrong thing, but the only uses of CASCADED-with-
a-D that I see in the spec are in the context of WITH CHECK OPTION,
which this patch does not implement. I am not sure why we are
documenting stuff we don't implement.
>> "tempViewWalker" in view.c is bereft of either comments or a usefully
>> descriptive name.
> I've added the forward declaration, but I couldn't think of a better
> name for the walker routine. isViewOnTempTableWalker() seemed too
> clumsy; any suggestions?
isViewOnTempTable_walker. If that isn't euphonious to you, then change
the name of isViewOnTempTable. Everywhere else that we have walker
subroutines, foo() invokes foo_walker(). You need a seriously good reason
to deviate from that convention.
>> Does the gram.y change really require breaking out OR REPLACE as a
>> separate production, and if so why?
> Without a separate production, you get 8 shift/reduce conflicts because
> both opt_or_replace and OptTemp can be reduced on the empty string.
[ scratches head... ] Seems like it ought to work anyway, since there
are no lookahead keywords for which the parse is ambiguous. I still
think there's something odd here.
If we do have to do it this way, a minor readability improvement would
be to write the CREATE case first and CREATE OR REPLACE second.
regards, tom lane