Re: patch for temporary view from TODO list
От | Neil Conway |
---|---|
Тема | Re: patch for temporary view from TODO list |
Дата | |
Msg-id | 1107306761.23033.21.camel@localhost.localdomain обсуждение исходный текст |
Ответ на | Re: patch for temporary view from TODO list (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: patch for temporary view from TODO list
|
Список | pgsql-patches |
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". I'm not sure there's a whole lot of value in documenting syntax we don't support, but if we're going to do it we may as well get it right. > "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. 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? > 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. Without a separate production, you get 8 shift/reduce conflicts because both opt_or_replace and OptTemp can be reduced on the empty string. The easiest workaround I can see is the one implemented in the patch, but I won't claim to be a bison expert. Suggestions for improvement are welcome. > 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. I've removed a few tests that didn't seem helpful. As for the tests taking "days to run", that would be fine with me -- if and when the tests actually _do_ take a long time to run, we can put more effort into defining a subset of them that can be run more quickly. In the mean time, though, I think we have far too few tests, not too many. > [ 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... ] Thanks for your comments. A revised version of the patch is attached. -Neil
Вложения
В списке pgsql-patches по дате отправления: