at1.1-default-composite.patch

text/plain

Filename: at1.1-default-composite.patch
Type: text/plain
Part: 0
Message: Re: ALTER TYPE 2: skip already-provable no-work rewrites

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: unified
File+
src/backend/commands/tablecmds.c 0 0
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 77ea9df..78262d8 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 3366,3384 **** ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
  	}
  
  	/*
- 	 * If we need to rewrite the table, the operation has to be propagated to
- 	 * tables that use this table's rowtype as a column type.
- 	 *
- 	 * (Eventually this will probably become true for scans as well, but at
- 	 * the moment a composite type does not enforce any constraints, so it's
- 	 * not necessary/appropriate to enforce them just during ALTER.)
- 	 */
- 	if (newrel)
- 		find_composite_type_dependencies(oldrel->rd_rel->reltype,
- 										 RelationGetRelationName(oldrel),
- 										 NULL);
- 
- 	/*
  	 * Generate the constraint and default execution states
  	 */
  
--- 3366,3371 ----
***************
*** 4300,4307 **** ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
--- 4287,4301 ----
  	 * table to fix that.
  	 */
  	if (isOid)
+ 	{
  		tab->new_changeoids = true;
  
+ 		/* See comments in ATExecDropColumn. */
+ 		find_composite_type_dependencies(rel->rd_rel->reltype,
+ 										 RelationGetRelationName(rel),
+ 										 NULL);
+ 	}
+ 
  	/*
  	 * Add needed dependency entries for the new column.
  	 */
***************
*** 4942,4948 **** ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
  
  	/*
  	 * If we dropped the OID column, must adjust pg_class.relhasoids and tell
! 	 * Phase 3 to physically get rid of the column.
  	 */
  	if (attnum == ObjectIdAttributeNumber)
  	{
--- 4936,4944 ----
  
  	/*
  	 * If we dropped the OID column, must adjust pg_class.relhasoids and tell
! 	 * Phase 3 to physically get rid of the column.  We formerly left the column
! 	 * in place physically, and this caused subtle problems:
! 	 * http://archives.postgresql.org/message-id/20594.1234109627@sss.pgh.pa.us
  	 */
  	if (attnum == ObjectIdAttributeNumber)
  	{
***************
*** 4972,4977 **** ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
--- 4968,4982 ----
  
  		/* Tell Phase 3 to physically remove the OID column */
  		tab->new_changeoids = true;
+ 
+ 		/*
+ 		 * We would also need to rewrite all tables using this table's rowtype
+ 		 * as a column type.  The OID column is not visible to or populated for
+ 		 * composite type users, but we do set HEAP_HASOID and reserve space.
+ 		 */
+ 		find_composite_type_dependencies(rel->rd_rel->reltype,
+ 										 RelationGetRelationName(rel),
+ 										 NULL);
  	}
  }
  
***************
*** 6364,6369 **** ATPrepAlterColumnType(List **wqueue,
--- 6369,6388 ----
  		newval->expr = (Expr *) transform;
  
  		tab->newvals = lappend(tab->newvals, newval);
+ 
+ 		/*
+ 		 * If we need to rewrite or scan this table, tables using its rowtype as
+ 		 * a column type would need the same treatment.
+ 		 */
+ 		find_composite_type_dependencies(rel->rd_rel->reltype,
+ 										 RelationGetRelationName(rel),
+ 										 NULL);
+ 	}
+ 	else if (tab->relkind == RELKIND_COMPOSITE_TYPE)
+ 	{
+ 		find_composite_type_dependencies(rel->rd_rel->reltype,
+ 										 NULL,
+ 										 RelationGetRelationName(rel));
  	}
  	else if (tab->relkind == RELKIND_FOREIGN_TABLE)
  	{
***************
*** 6373,6389 **** ATPrepAlterColumnType(List **wqueue,
  					 errmsg("ALTER TYPE USING is not supported on foreign tables")));
  	}
  
- 	if (tab->relkind == RELKIND_COMPOSITE_TYPE)
- 	{
- 		/*
- 		 * For composite types, do this check now.  Tables will check
- 		 * it later when the table is being rewritten.
- 		 */
- 		find_composite_type_dependencies(rel->rd_rel->reltype,
- 										 NULL,
- 										 RelationGetRelationName(rel));
- 	}
- 
  	ReleaseSysCache(tuple);
  
  	/*
--- 6392,6397 ----
diff --git a/src/test/regress/expected/index 6fc9db8..f2cfc07 100644
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
***************
*** 1483,1490 **** alter table tab1 alter column b type varchar; -- fails
  ERROR:  cannot alter table "tab1" because column "tab2"."y" uses its rowtype
  alter table tab1 add check (b <> 'foo');
  alter table tab1 add c int not null;
! alter table tab1 add d int not null default 1; -- fails
! ERROR:  cannot alter table "tab1" because column "tab2"."y" uses its rowtype
  alter table tab1 drop a;
  alter table tab1 set with oids;	-- fails
  ERROR:  cannot alter table "tab1" because column "tab2"."y" uses its rowtype
--- 1483,1489 ----
  ERROR:  cannot alter table "tab1" because column "tab2"."y" uses its rowtype
  alter table tab1 add check (b <> 'foo');
  alter table tab1 add c int not null;
! alter table tab1 add d int not null default 1;
  alter table tab1 drop a;
  alter table tab1 set with oids;	-- fails
  ERROR:  cannot alter table "tab1" because column "tab2"."y" uses its rowtype
diff --git a/src/test/regress/expected/rowtypes.index e5cd714..707ca38 100644
*** a/src/test/regress/expected/rowtypes.out
--- b/src/test/regress/expected/rowtypes.out
***************
*** 82,91 **** select * from people;
   (Joe,Blow) | 01-10-1984
  (1 row)
  
! -- at the moment this will not work due to ALTER TABLE inadequacy:
  alter table fullname add column suffix text default '';
! ERROR:  cannot alter table "fullname" because column "people"."fn" uses its rowtype
! -- but this should work:
  alter table fullname add column suffix text default null;
  select * from people;
       fn      |     bd     
--- 82,91 ----
   (Joe,Blow) | 01-10-1984
  (1 row)
  
! -- accepted, but the default does not propagate
  alter table fullname add column suffix text default '';
! alter table fullname drop column suffix;
! -- same effect
  alter table fullname add column suffix text default null;
  select * from people;
       fn      |     bd     
diff --git a/src/test/regress/sql/alter_tableindex ed857cf..1818010 100644
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
***************
*** 1098,1104 **** create table tab2 (x int, y tab1);
  alter table tab1 alter column b type varchar; -- fails
  alter table tab1 add check (b <> 'foo');
  alter table tab1 add c int not null;
! alter table tab1 add d int not null default 1; -- fails
  alter table tab1 drop a;
  alter table tab1 set with oids;	-- fails
  
--- 1098,1104 ----
  alter table tab1 alter column b type varchar; -- fails
  alter table tab1 add check (b <> 'foo');
  alter table tab1 add c int not null;
! alter table tab1 add d int not null default 1;
  alter table tab1 drop a;
  alter table tab1 set with oids;	-- fails
  
diff --git a/src/test/regress/sql/rowtypes.index 9041df1..839cd7c 100644
*** a/src/test/regress/sql/rowtypes.sql
--- b/src/test/regress/sql/rowtypes.sql
***************
*** 45,54 **** insert into people values ('(Joe,Blow)', '1984-01-10');
  
  select * from people;
  
! -- at the moment this will not work due to ALTER TABLE inadequacy:
  alter table fullname add column suffix text default '';
  
! -- but this should work:
  alter table fullname add column suffix text default null;
  
  select * from people;
--- 45,55 ----
  
  select * from people;
  
! -- accepted, but the default does not propagate
  alter table fullname add column suffix text default '';
+ alter table fullname drop column suffix;
  
! -- same effect
  alter table fullname add column suffix text default null;
  
  select * from people;