Re: [PATCHES] add WITH OIDS to CREATE TABLE AS

2004-01-07 Thread Neil Conway
Tom Lane [EMAIL PROTECTED] writes:
 One thing that is *not* fine is something that I see had snuck past
 me in the previous WITH OIDS patch.  It is not okay for gram.y or
 scan.l to be looking at GUC variables --- that creates
 synchronization issues.

Good point (I remember reading gram.y's warning about this, but it
must have slipped my mind...). Attached is a revised patch that
corrects this.

-Neil
Index: doc/src/sgml/runtime.sgml
===
RCS file: /var/lib/cvs/pgsql-server/doc/src/sgml/runtime.sgml,v
retrieving revision 1.227
diff -c -r1.227 runtime.sgml
*** doc/src/sgml/runtime.sgml	13 Dec 2003 23:59:06 -	1.227
--- doc/src/sgml/runtime.sgml	7 Jan 2004 02:29:15 -
***
*** 2437,2453 
termvarnamedefault_with_oids/varname (typeboolean/type)/term
listitem
 para
! This controls whether commandCREATE TABLE/command will
! include OIDs in newly-created tables, if neither literalWITH
! OIDS/literal or literalWITHOUT OIDS/literal have been
! specified. It also determines whether OIDs will be included in
! the table generated by commandSELECT INTO/command and
! commandCREATE TABLE AS/command. In
  productnamePostgreSQL/productname version;
! varnamedefault_with_oids/varname defaults to true. This is
! also the behavior of previous versions of
! productnamePostgreSQL/productname. However, assuming that
! tables will contain OIDs by default is not
  encouraged. Therefore, this option will default to false in a
  future release of productnamePostgreSQL/productname.
 /para
--- 2437,2453 
termvarnamedefault_with_oids/varname (typeboolean/type)/term
listitem
 para
! This controls whether commandCREATE TABLE/command
! and commandCREATE TABLE AS/command will include OIDs in
! newly-created tables, if neither literalWITH OIDS/literal
! or literalWITHOUT OIDS/literal have been specified. It
! also determines whether OIDs will be included in the table
! created by commandSELECT INTO/command. In
  productnamePostgreSQL/productname version;
! varnamedefault_with_oids/varname defaults to
! true. This is also the behavior of previous versions
! of productnamePostgreSQL/productname. However, assuming
! that tables will contain OIDs by default is not
  encouraged. Therefore, this option will default to false in a
  future release of productnamePostgreSQL/productname.
 /para
Index: doc/src/sgml/ref/create_table_as.sgml
===
RCS file: /var/lib/cvs/pgsql-server/doc/src/sgml/ref/create_table_as.sgml,v
retrieving revision 1.19
diff -c -r1.19 create_table_as.sgml
*** doc/src/sgml/ref/create_table_as.sgml	13 Dec 2003 23:59:07 -	1.19
--- doc/src/sgml/ref/create_table_as.sgml	7 Jan 2004 03:18:20 -
***
*** 20,26 
  
   refsynopsisdiv
  synopsis
! CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE replaceabletable_name/replaceable [ (replaceablecolumn_name/replaceable [, ...] ) ]
  AS replaceablequery/replaceable
  /synopsis
   /refsynopsisdiv
--- 20,26 
  
   refsynopsisdiv
  synopsis
! CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE replaceabletable_name/replaceable [ (replaceablecolumn_name/replaceable [, ...] ) ] [ [ WITH | WITHOUT ] OIDS ]
  AS replaceablequery/replaceable
  /synopsis
   /refsynopsisdiv
***
*** 99,104 
--- 99,118 
 /varlistentry
  
 varlistentry
+ termliteralWITH OIDS/literal/term
+ termliteralWITHOUT OIDS/literal/term
+  listitem
+   para
+This optional clause specifies whether the table created by
+commandCREATE TABLE AS/command should include OIDs. If
+neither form of this clause if specified, the value of the
+varnamedefault_with_oids/varname configuration parameter is
+used.
+   /para
+  /listitem
+/varlistentry
+ 
+varlistentry
  termreplaceablequery/replaceable/term
  listitem
   para
***
*** 121,143 
 This command is functionally similar to xref
 linkend=sql-selectinto endterm=sql-selectinto-title, but it is
 preferred since it is less likely to be confused with other uses of
!the commandSELECT INTO/command syntax.
/para
  
para
!Prior to productnamePostgreSQL/ 7.5, commandCREATE TABLE
!AS/command always included OIDs in the table it
 produced. Furthermore, these OIDs were newly generated: they were
 distinct from the OIDs of any of the rows in the source tables of
 the commandSELECT/command or commandEXECUTE/command
 statement. Therefore, if commandCREATE TABLE AS/command was
 frequently executed, the OID counter would be rapidly
!incremented. As of 

Re: [PATCHES] add WITH OIDS to CREATE TABLE AS

2004-01-07 Thread Bruce Momjian
Neil Conway wrote:
 Bruce Momjian [EMAIL PROTECTED] writes:
  Couldn't we use SET only when we need to change the existing value?
 
 I'm not sure what you mean. The pg_dump output will look like:
 
 -- at the top of the dump
 SET default_with_oids = false; -- or true, doesn't really matter
 
 -- for a table that doesn't have OIDs
 CREATE TABLE (...);
 
 -- for a table that does have OIDs
 SET default_with_oids = true;
 CREATE TABLE (...);
 SET default_with_oids = false;
 
 The point is that in this example if all the tables in the DB have
 OIDs, you'll emit two SETs for each CREATE TABLE, so what you'd really
 like is to have chosen a different default to begin with.
 
 Anyway, it's just an implementation detail: I'll definitely implement
 it one way or another in time for 7.5 (unless someone else would like
 to do it, in which case I'd gladly step aside).

I assume we would _remember_ the current with_oids value inside pg_dump.

For example, if I create two tables as user 'guest', I see in pg_dump
output:

SET SESSION AUTHORIZATION 'guest';

--
-- Name: g1; Type: TABLE; Schema: public; Owner: guest
--

CREATE TABLE g1 (
x integer
) WITH OIDS;


--
-- Name: g2; Type: TABLE; Schema: public; Owner: guest
--

CREATE TABLE g2 (
x integer
) WITH OIDS;


SET SESSION AUTHORIZATION 'postgres';

Notice that only one SESSION AUTHORIZATION is used for guest.  Can't we
do the same for WITH/WITHOUT OIDS?

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 6: Have you searched our list archives?

   http://archives.postgresql.org


[PATCHES] add WITH OIDS to CREATE TABLE AS

2004-01-06 Thread Neil Conway
This patch adds a WITH / WITHOUT OIDS clause to CREATE TABLE
AS. This allows the user to explicitly specify whether OIDs should be
included in the newly-created relation (if form of this clause is
specified, the default_with_oids configuration variable is used). This
is useful because it provides a way for application authors to ensure
their applications are compatible with future versions of PostgreSQL
(in which the relation created by CREATE TABLE AS won't include OIDs
by default).

No equivalent functionality has been added to SELECT INTO: there
isn't a convenient syntax for it that I could see, and in any case
CREATE TABLE AS has always offered a superset of the functionality of
SELECT INTO. Therefore, I don't view this as a problem.

The implementation is a tad messy (it would be nice if CREATE TABLE AS
were a distinct node, to avoid needing to clutter up SelectStmt
further). I also needed to add an additional production to avoid a
shift/reduce conflict in the parser (see the XXX note in the patch
itself).

The patch includes updates to the documentation and regression tests.

Unless anyone objects, I plan to apply this within 48 hours.

-Neil
Index: doc/src/sgml/runtime.sgml
===
RCS file: /var/lib/cvs/pgsql-server/doc/src/sgml/runtime.sgml,v
retrieving revision 1.227
diff -c -r1.227 runtime.sgml
*** doc/src/sgml/runtime.sgml	13 Dec 2003 23:59:06 -	1.227
--- doc/src/sgml/runtime.sgml	6 Jan 2004 19:04:09 -
***
*** 2437,2453 
termvarnamedefault_with_oids/varname (typeboolean/type)/term
listitem
 para
! This controls whether commandCREATE TABLE/command will
! include OIDs in newly-created tables, if neither literalWITH
! OIDS/literal or literalWITHOUT OIDS/literal have been
! specified. It also determines whether OIDs will be included in
! the table generated by commandSELECT INTO/command and
! commandCREATE TABLE AS/command. In
  productnamePostgreSQL/productname version;
! varnamedefault_with_oids/varname defaults to true. This is
! also the behavior of previous versions of
! productnamePostgreSQL/productname. However, assuming that
! tables will contain OIDs by default is not
  encouraged. Therefore, this option will default to false in a
  future release of productnamePostgreSQL/productname.
 /para
--- 2437,2453 
termvarnamedefault_with_oids/varname (typeboolean/type)/term
listitem
 para
! This controls whether commandCREATE TABLE/command
! and commandCREATE TABLE AS/command will include OIDs in
! newly-created tables, if neither literalWITH OIDS/literal
! or literalWITHOUT OIDS/literal have been specified. It
! also determines whether OIDs will be included in the table
! created by commandSELECT INTO/command. In
  productnamePostgreSQL/productname version;
! varnamedefault_with_oids/varname defaults to
! true. This is also the behavior of previous versions
! of productnamePostgreSQL/productname. However, assuming
! that tables will contain OIDs by default is not
  encouraged. Therefore, this option will default to false in a
  future release of productnamePostgreSQL/productname.
 /para
Index: doc/src/sgml/ref/create_table_as.sgml
===
RCS file: /var/lib/cvs/pgsql-server/doc/src/sgml/ref/create_table_as.sgml,v
retrieving revision 1.19
diff -c -r1.19 create_table_as.sgml
*** doc/src/sgml/ref/create_table_as.sgml	13 Dec 2003 23:59:07 -	1.19
--- doc/src/sgml/ref/create_table_as.sgml	6 Jan 2004 19:04:09 -
***
*** 20,26 
  
   refsynopsisdiv
  synopsis
! CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE replaceabletable_name/replaceable [ (replaceablecolumn_name/replaceable [, ...] ) ]
  AS replaceablequery/replaceable
  /synopsis
   /refsynopsisdiv
--- 20,26 
  
   refsynopsisdiv
  synopsis
! CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE replaceabletable_name/replaceable [ (replaceablecolumn_name/replaceable [, ...] ) ] [ [ WITH | WITHOUT ] OIDS ]
  AS replaceablequery/replaceable
  /synopsis
   /refsynopsisdiv
***
*** 99,104 
--- 99,118 
 /varlistentry
  
 varlistentry
+ termliteralWITH OIDS/literal/term
+ termliteralWITHOUT OIDS/literal/term
+  listitem
+   para
+This optional clauses specifies whether the table created
+by commandCREATE TABLE AS/command should include OIDs. If
+neither form of this clause if specified, the value of
+the varnamedefault_with_oids/varname configuration
+parameter is used.
+   /para
+  /listitem
+/varlistentry
+ 
+varlistentry
  termreplaceablequery/replaceable/term
  listitem
   para

Re: [PATCHES] add WITH OIDS to CREATE TABLE AS

2004-01-06 Thread Tom Lane
Neil Conway [EMAIL PROTECTED] writes:
 No equivalent functionality has been added to SELECT INTO: there
 isn't a convenient syntax for it that I could see, and in any case
 CREATE TABLE AS has always offered a superset of the functionality of
 SELECT INTO. Therefore, I don't view this as a problem.

I agree on that.

 The implementation is a tad messy (it would be nice if CREATE TABLE AS
 were a distinct node, to avoid needing to clutter up SelectStmt
 further).

Yeah, I have been wanting for awhile to redesign the parse
representation of CREATE TABLE AS/SELECT INTO.  It's not obvious exactly
what to do though.

 I also needed to add an additional production to avoid a
 shift/reduce conflict in the parser (see the XXX note in the patch
 itself).

This is a fairly standard way of avoiding conflicts --- looks fine to
me.

One thing that is *not* fine is something that I see had snuck past me
in the previous WITH OIDS patch.  It is not okay for gram.y or scan.l
to be looking at GUC variables --- that creates synchronization issues.
(Consider a single querystring containing a SET followed by something
that depends on the SET value.  The whole string will be parsed before
the SET is applied.)  The references to default_with_oids have to be
postponed to analyze.c's processing.  Compare the way that inhOpt has
an INH_DEFAULT setting --- you probably need the same kind of solution
for the WITH OIDS options.

regards, tom lane

---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
  joining column's datatypes do not match


Re: [PATCHES] add WITH OIDS to CREATE TABLE AS

2004-01-06 Thread Neil Conway
Bruce Momjian [EMAIL PROTECTED] writes:
 Does this deal with the fact we now emit WITH/WITHOUT OID in
 pg_dump?

No, that is an unrelated issue. I took a brief look at implementing
this over the break, but I couldn't see an easy way to do it properly:
if we pick a particular default for the GUC variable and then use SET
to change it when necessary for a particular CREATE TABLE, we could
potentially issue far more SET commands than are needed.

 FYI, that is a must-fix for 7.5 for portability reasons.

(Reasons which I still find pretty unconvincing, but ...)

-Neil


---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster


Re: [PATCHES] add WITH OIDS to CREATE TABLE AS

2004-01-06 Thread Bruce Momjian
Neil Conway wrote:
 Bruce Momjian [EMAIL PROTECTED] writes:
  Does this deal with the fact we now emit WITH/WITHOUT OID in
  pg_dump?
 
 No, that is an unrelated issue. I took a brief look at implementing
 this over the break, but I couldn't see an easy way to do it properly:
 if we pick a particular default for the GUC variable and then use SET
 to change it when necessary for a particular CREATE TABLE, we could
 potentially issue far more SET commands than are needed.

Couldn't we use SET only when we need to change the existing value? 
Isn't that what we do with SET AUTHORIZATION?

  FYI, that is a must-fix for 7.5 for portability reasons.
 
 (Reasons which I still find pretty unconvincing, but ...)

I understand, but there was general agreement not to add more
incompatibilities to pg_dump.  If it can't be fixed, WITH/WITHOUT OIDS
will need to be removed.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  [EMAIL PROTECTED]   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
  joining column's datatypes do not match


Re: [PATCHES] add WITH OIDS to CREATE TABLE AS

2004-01-06 Thread Neil Conway
Bruce Momjian [EMAIL PROTECTED] writes:
 Couldn't we use SET only when we need to change the existing value?

I'm not sure what you mean. The pg_dump output will look like:

-- at the top of the dump
SET default_with_oids = false; -- or true, doesn't really matter

-- for a table that doesn't have OIDs
CREATE TABLE (...);

-- for a table that does have OIDs
SET default_with_oids = true;
CREATE TABLE (...);
SET default_with_oids = false;

The point is that in this example if all the tables in the DB have
OIDs, you'll emit two SETs for each CREATE TABLE, so what you'd really
like is to have chosen a different default to begin with.

Anyway, it's just an implementation detail: I'll definitely implement
it one way or another in time for 7.5 (unless someone else would like
to do it, in which case I'd gladly step aside).

-Neil


---(end of broadcast)---
TIP 6: Have you searched our list archives?

   http://archives.postgresql.org