Code review for LIKE INCLUDING patch --- clean up some cosmetic and not

so cosmetic stuff.

doc/src/sgml/ref/create_table.sgml

@@ -1,5 +1,5 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/ref/create_table.sgml,v 1.117 2009/10/12 19:49:24 adunstan Exp $
+$PostgreSQL: pgsql/doc/src/sgml/ref/create_table.sgml,v 1.118 2009/10/13 00:53:07 tgl Exp $
 PostgreSQL documentation
 -->
 
@@ -231,7 +231,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PAR
      </para>
 
      <para>
-      Column storage parameters are also copied from parent tables.
+      Column <literal>STORAGE</> settings are also copied from parent tables.
      </para>
 
 <!--
@@ -285,25 +285,27 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PAR
       specified.
      </para>
      <para>
-      Storage parameters for the copied column definitions will only be copied
-      if <literal>INCLUDING STORAGE</literal> is specified.  The default
-      behavior is to exclude storage parameters, resulting in the copied
-      columns in the new table having type-specific default parameters.  For
-      more on storage parameters, see <xref linkend="storage-toast">.
+      <literal>STORAGE</> settings for the copied column definitions will only
+      be copied if <literal>INCLUDING STORAGE</literal> is specified.  The
+      default behavior is to exclude <literal>STORAGE</> settings, resulting
+      in the copied columns in the new table having type-specific default
+      settings.  For more on <literal>STORAGE</> settings, see
+      <xref linkend="storage-toast">.
      </para>
      <para>
-      Comments for the copied column, constraint, index and columns of index
-	  definitions will only be copied if <literal>INCLUDING COMMENTS</literal>
-	  is specified. The default behavior is to exclude comments, resulting in
-	  the copied columns and constraints in the new table having no comments.
+      Comments for the copied columns, constraints, and indexes
+      will only be copied if <literal>INCLUDING COMMENTS</literal>
+      is specified. The default behavior is to exclude comments, resulting in
+      the copied columns and constraints in the new table having no comments.
      </para>
      <para>
       <literal>INCLUDING ALL</literal> is an abbreviated form of
       <literal>INCLUDING DEFAULTS INCLUDING CONSTRAINTS INCLUDING INDEXES INCLUDING STORAGE INCLUDING COMMENTS</literal>.
      </para>
      <para>
-      Note also that unlike <literal>INHERITS</literal>, copied columns and
-      constraints are not merged with similarly named columns and constraints.
+      Note also that unlike <literal>INHERITS</literal>, columns and
+      constraints copied by <literal>LIKE</> are not merged with similarly
+      named columns and constraints.
       If the same name is specified explicitly or in another
       <literal>LIKE</literal> clause, an error is signalled.
      </para>
@@ -752,14 +754,14 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PAR
      <para>
      Enables or disables the autovacuum daemon on a particular table.
      If true, the autovacuum daemon will initiate a <command>VACUUM</> operation
-     on a particular table when the number of updated or deleted tuples exceeds 
-     <literal>autovacuum_vacuum_threshold</> plus 
-     <literal>autovacuum_vacuum_scale_factor</> times the number of live tuples 
+     on a particular table when the number of updated or deleted tuples exceeds
+     <literal>autovacuum_vacuum_threshold</> plus
+     <literal>autovacuum_vacuum_scale_factor</> times the number of live tuples
      currently estimated to be in the relation.
      Similarly, it will initiate an <command>ANALYZE</> operation when the
      number of inserted, updated or deleted tuples exceeds
-     <literal>autovacuum_analyze_threshold</> plus 
-     <literal>autovacuum_analyze_scale_factor</> times the number of live tuples 
+     <literal>autovacuum_analyze_threshold</> plus
+     <literal>autovacuum_analyze_scale_factor</> times the number of live tuples
      currently estimated to be in the relation.
      If false, this table will not be autovacuumed, except to prevent
      transaction Id wraparound. See <xref linkend="vacuum-for-wraparound"> for
@@ -775,7 +777,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PAR
     <listitem>
      <para>
      Minimum number of updated or deleted tuples before initiate a
-     <command>VACUUM</> operation on a particular table. 
+     <command>VACUUM</> operation on a particular table.
      </para>
     </listitem>
    </varlistentry>
@@ -834,7 +836,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } ] TABLE <replaceable class="PAR
      <para>
      Custom <xref linkend="guc-vacuum-freeze-min-age"> parameter. Note that
      autovacuum will ignore attempts to set a per-table
-     <literal>autovacuum_freeze_min_age</> larger than the half system-wide 
+     <literal>autovacuum_freeze_min_age</> larger than the half system-wide
      <xref linkend="guc-autovacuum-freeze-max-age"> setting.
      </para>
     </listitem>

src/backend/access/common/tupdesc.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/common/tupdesc.c,v 1.129 2009/10/12 19:49:24 adunstan Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/common/tupdesc.c,v 1.130 2009/10/13 00:53:07 tgl Exp $
  *
  * NOTES
  *	  some of the executor utility code such as "ExecTypeFromTL" should be
@@ -553,13 +553,15 @@ BuildDescForRelation(List *schema)
 		TupleDescInitEntry(desc, attnum, attname,
 						   atttypid, atttypmod, attdim);
 
+		/* Override TupleDescInitEntry's settings as requested */
+		if (entry->storage)
+			desc->attrs[attnum - 1]->attstorage = entry->storage;
+
 		/* Fill in additional stuff not handled by TupleDescInitEntry */
 		desc->attrs[attnum - 1]->attnotnull = entry->is_not_null;
 		has_not_null |= entry->is_not_null;
 		desc->attrs[attnum - 1]->attislocal = entry->is_local;
 		desc->attrs[attnum - 1]->attinhcount = entry->inhcount;
-		if (entry->storage)
-			desc->attrs[attnum - 1]->attstorage = entry->storage;
 	}
 
 	if (has_not_null)

src/backend/catalog/pg_constraint.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/pg_constraint.c,v 1.48 2009/10/12 19:49:24 adunstan Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/pg_constraint.c,v 1.49 2009/10/13 00:53:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -705,7 +705,8 @@ AlterConstraintNamespaces(Oid ownerId, Oid oldNspId,
 
 /*
  * GetConstraintByName
- *		Find a constraint with the specified name.
+ *		Find a constraint on the specified relation with the specified name.
+ *		Returns constraint's OID.
  */
 Oid
 GetConstraintByName(Oid relid, const char *conname)
@@ -725,7 +726,8 @@ GetConstraintByName(Oid relid, const char *conname)
 
 	ScanKeyInit(&skey[0],
 				Anum_pg_constraint_conrelid,
-				BTEqualStrategyNumber, F_OIDEQ, relid);
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(relid));
 
 	scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true,
 							  SnapshotNow, 1, skey);
@@ -737,28 +739,22 @@ GetConstraintByName(Oid relid, const char *conname)
 		if (strcmp(NameStr(con->conname), conname) == 0)
 		{
 			if (OidIsValid(conOid))
-			{
-				char *relname = get_rel_name(relid);
 				ereport(ERROR,
 						(errcode(ERRCODE_DUPLICATE_OBJECT),
-				 errmsg("table \"%s\" has multiple constraints named \"%s\"",
-					(relname ? relname : "(unknown)"), conname)));
-			}
+						 errmsg("table \"%s\" has multiple constraints named \"%s\"",
+								get_rel_name(relid), conname)));
 			conOid = HeapTupleGetOid(tuple);
 		}
 	}
 
 	systable_endscan(scan);
 
-	/* If no constraint exists for the relation specified, notify user */
+	/* If no such constraint exists, complain */
 	if (!OidIsValid(conOid))
-	{
-		char *relname = get_rel_name(relid);
 		ereport(ERROR,
 				(errcode(ERRCODE_UNDEFINED_OBJECT),
 				 errmsg("constraint \"%s\" for table \"%s\" does not exist",
-						conname, (relname ? relname : "(unknown)"))));
-	}
+						conname, get_rel_name(relid))));
 
 	heap_close(pg_constraint, AccessShareLock);
 

src/backend/commands/sequence.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.161 2009/07/16 06:33:42 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.162 2009/10/13 00:53:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -132,6 +132,7 @@ DefineSequence(CreateSeqStmt *seq)
 		coldef->inhcount = 0;
 		coldef->is_local = true;
 		coldef->is_not_null = true;
+		coldef->storage = 0;
 		coldef->raw_default = NULL;
 		coldef->cooked_default = NULL;
 		coldef->constraints = NIL;

src/backend/commands/tablecmds.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.302 2009/10/12 19:49:24 adunstan Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.303 2009/10/13 00:53:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -333,7 +333,7 @@ static void ATExecAddInherit(Relation rel, RangeVar *parent);
 static void ATExecDropInherit(Relation rel, RangeVar *parent);
 static void copy_relation_data(SMgrRelation rel, SMgrRelation dst,
 				   ForkNumber forkNum, bool istemp);
-static const char * storage_name(char c);
+static const char *storage_name(char c);
 
 
 /* ----------------------------------------------------------------
@@ -1102,22 +1102,25 @@ truncate_check_rel(Relation rel)
 	CheckTableNotInUse(rel, "TRUNCATE");
 }
 
-
-/*----------------
+/*
  * storage_name
- *    returns a name corresponding to a storage enum value
- * For use in error messages
+ *    returns the name corresponding to a typstorage/attstorage enum value
  */
 static const char *
 storage_name(char c)
 {
 	switch (c)
 	{
-		case 'p': return "PLAIN";
-		case 'm': return "MAIN";
-		case 'x': return "EXTENDED";
-		case 'e': return "EXTERNAL";
-		default: return "???";
+		case 'p':
+			return "PLAIN";
+		case 'm':
+			return "MAIN";
+		case 'x':
+			return "EXTENDED";
+		case 'e':
+			return "EXTERNAL";
+		default:
+			return "???";
 	}
 }
 
@@ -1189,7 +1192,6 @@ MergeAttributes(List *schema, List *supers, bool istemp,
 	List	   *constraints = NIL;
 	int			parentsWithOids = 0;
 	bool		have_bogus_defaults = false;
-	bool            have_bogus_comments = false;
 	int			child_attno;
 	static Node	bogus_marker = { 0 };		/* marks conflicting defaults */
 
@@ -1354,7 +1356,8 @@ MergeAttributes(List *schema, List *supers, bool istemp,
 							(errcode(ERRCODE_DATATYPE_MISMATCH),
 						errmsg("inherited column \"%s\" has a storage parameter conflict",
 							   attributeName),
-							   errdetail("%s versus %s", storage_name(def->storage),
+							   errdetail("%s versus %s",
+										 storage_name(def->storage),
 										 storage_name(attribute->attstorage))));
 
 				def->inhcount++;
@@ -1375,10 +1378,10 @@ MergeAttributes(List *schema, List *supers, bool istemp,
 				def->inhcount = 1;
 				def->is_local = false;
 				def->is_not_null = attribute->attnotnull;
+				def->storage = attribute->attstorage;
 				def->raw_default = NULL;
 				def->cooked_default = NULL;
 				def->constraints = NIL;
-				def->storage = attribute->attstorage;
 				inhSchema = lappend(inhSchema, def);
 				newattno[parent_attno - 1] = ++child_attno;
 			}
@@ -1525,7 +1528,8 @@ MergeAttributes(List *schema, List *supers, bool istemp,
 							(errcode(ERRCODE_DATATYPE_MISMATCH),
 						errmsg("column \"%s\" has a storage parameter conflict",
 							   attributeName),
-							   errdetail("%s versus %s", storage_name(def->storage),
+							   errdetail("%s versus %s",
+										 storage_name(def->storage),
 										 storage_name(newdef->storage))));
 
 				/* Mark the column as locally defined */
@@ -1580,20 +1584,6 @@ MergeAttributes(List *schema, List *supers, bool istemp,
 		}
 	}
 
-	/* Raise an error if we found conflicting comments. */
-	if (have_bogus_comments)
-	{
-		foreach(entry, schema)
-		{
-			ColumnDef  *def = lfirst(entry);
-
-			if (def->cooked_default == &bogus_marker)
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
-				  errmsg("column \"%s\" inherits conflicting comments", def->colname)));
-		}
-	}
-
 	*supOids = parentOids;
 	*supconstr = constraints;
 	*supOidCount = parentsWithOids;
@@ -3903,6 +3893,7 @@ ATPrepAddOids(List **wqueue, Relation rel, bool recurse, AlterTableCmd *cmd)
 		cdef->inhcount = 0;
 		cdef->is_local = true;
 		cdef->is_not_null = true;
+		cdef->storage = 0;
 		cmd->def = (Node *) cdef;
 	}
 	ATPrepAddColumn(wqueue, rel, recurse, cmd);

src/backend/commands/view.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.117 2009/07/16 06:33:42 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.118 2009/10/13 00:53:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -124,6 +124,7 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
 			def->inhcount = 0;
 			def->is_local = true;
 			def->is_not_null = false;
+			def->storage = 0;
 			def->raw_default = NULL;
 			def->cooked_default = NULL;
 			def->constraints = NIL;

src/backend/nodes/copyfuncs.c

@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.446 2009/10/12 20:39:39 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.447 2009/10/13 00:53:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2115,6 +2115,7 @@ _copyColumnDef(ColumnDef *from)
 	COPY_SCALAR_FIELD(inhcount);
 	COPY_SCALAR_FIELD(is_local);
 	COPY_SCALAR_FIELD(is_not_null);
+	COPY_SCALAR_FIELD(storage);
 	COPY_NODE_FIELD(raw_default);
 	COPY_NODE_FIELD(cooked_default);
 	COPY_NODE_FIELD(constraints);

src/backend/nodes/equalfuncs.c

@@ -22,7 +22,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.368 2009/10/12 20:39:40 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.369 2009/10/13 00:53:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2084,6 +2084,7 @@ _equalColumnDef(ColumnDef *a, ColumnDef *b)
 	COMPARE_SCALAR_FIELD(inhcount);
 	COMPARE_SCALAR_FIELD(is_local);
 	COMPARE_SCALAR_FIELD(is_not_null);
+	COMPARE_SCALAR_FIELD(storage);
 	COMPARE_NODE_FIELD(raw_default);
 	COMPARE_NODE_FIELD(cooked_default);
 	COMPARE_NODE_FIELD(constraints);

src/backend/nodes/outfuncs.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.368 2009/10/12 18:10:45 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.369 2009/10/13 00:53:08 tgl Exp $
  *
  * NOTES
  *	  Every node type that can appear in stored rules' parsetrees *must*
@@ -1885,6 +1885,7 @@ _outColumnDef(StringInfo str, ColumnDef *node)
 	WRITE_INT_FIELD(inhcount);
 	WRITE_BOOL_FIELD(is_local);
 	WRITE_BOOL_FIELD(is_not_null);
+	WRITE_INT_FIELD(storage);
 	WRITE_NODE_FIELD(raw_default);
 	WRITE_NODE_FIELD(cooked_default);
 	WRITE_NODE_FIELD(constraints);

src/backend/parser/parse_utilcmd.c

@@ -19,7 +19,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.27 2009/10/12 19:49:24 adunstan Exp $
+ *	$PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.28 2009/10/13 00:53:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -642,6 +642,8 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
 		/* Likewise, copy storage if requested */
 		if (inhRelation->options & CREATE_TABLE_LIKE_STORAGE)
 			def->storage = attribute->attstorage;
+		else
+			def->storage = 0;
 
 		/* Likewise, copy comment if requested */
 		if ((inhRelation->options & CREATE_TABLE_LIKE_COMMENTS) &&

src/include/nodes/parsenodes.h

@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.408 2009/10/12 20:39:42 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.409 2009/10/13 00:53:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -460,20 +460,20 @@ typedef struct ColumnDef
 	int			inhcount;		/* number of times column is inherited */
 	bool		is_local;		/* column has local (non-inherited) def'n */
 	bool		is_not_null;	/* NOT NULL constraint specified? */
-	char		storage;		/* storage parameter of column */
+	char		storage;		/* attstorage setting, or 0 for default */
 	Node	   *raw_default;	/* default value (untransformed parse tree) */
 	Node	   *cooked_default; /* default value (transformed expr tree) */
 	List	   *constraints;	/* other constraints on column */
 } ColumnDef;
 
 /*
- * inhRelation - Relations a CREATE TABLE is to inherit attributes of
+ * inhRelation - Relation a CREATE TABLE is to inherit attributes of
  */
 typedef struct InhRelation
 {
 	NodeTag		type;
 	RangeVar   *relation;
-	bits32		options;		/* bitmap of CreateStmtLikeOption */
+	bits32		options;		/* OR of CreateStmtLikeOption flags */
 } InhRelation;
 
 typedef enum CreateStmtLikeOption