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
Index: doc/src/sgml/ref/create_table.sgml
===
RCS file: /var/lib/cvs/pgsql/doc/src/sgml/ref/create_table.sgml,v
retrieving revision 1.91
diff -c -r1.91 create_table.sgml
*** doc/src/sgml/ref/create_table.sgml 22 Jan 2005 23:22:17 - 1.91
--- doc/src/sgml/ref/create_table.sgml 1 Feb 2005 23:32:45 -
***
*** 66,77
If a schema name is given (for example, CREATE TABLE
!myschema.mytable ...) then the table is created in the
!specified schema. Otherwise it is created in the current schema.
!Temporary tables exist in a special schema, so a schema name may not be
!given when creating a temporary table.
!The table name must be distinct from the name of any other table,
!sequence, index, or view in the same schema.
--- 66,77
If a schema name is given (for example, CREATE TABLE
!myschema.mytable ...) then the table is created in the specified
!schema. Otherwise it is created in the current schema. Temporary
!tables exist in a special schema, so a schema name may not be given
!when creating a temporary table. The name of the table must be
!distinct from the name of any other table, sequence, index, or view
!in the same schema.
Index: doc/src/sgml/ref/create_view.sgml
===
RCS file: /var/lib/cvs/pgsql/doc/src/sgml/ref/create_view.sgml,v
retrieving revision 1.29
diff -c -r1.29 create_view.sgml
*** doc/src/sgml/ref/create_view.sgml 4 Jan 2005 00:39:53 - 1.29
--- doc/src/sgml/ref/create_view.sgml 1 Feb 2005 23:32:45 -
***
*** 20,26
! CREATE [ OR REPLACE ] VIEW name [ ( column_name [, ...] ) ] AS query
--- 20,26
! CREATE [ OR REPLACE ] [ TEMP | TEMPORARY ] VIEW name [ ( column_name [, ...] ) ] AS query
***
*** 43,52
If a schema name is given (for example, CREATE VIEW
!myschema.myview ...) then the view is created in the
!specified schema. Otherwise it is created in the current schema.
!The view name must be distinct from the name of any other view, table,
!sequence, or index in the same schema.
--- 43,54
If a schema name is given (for example, CREATE VIEW
!myschema.myview ...) then the view is created in the specified
!schema. Otherwise it is created in the current schema. Temporary
!views exist in a special schema, so a schema name may not be given
!when creating a temporary view. The name of the view must be
!distinct from the name of any other view, table, sequence, or index
!in the same schema.
***
*** 55,60
--- 57,84
+ TEMPORARY or TEMP
+
