Re: [PATCHES] Patch for ALTER TABLE / TYPE

[Tom Lane]

NAKANO Yoshihisa <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> After looking at the test case a bit, I have an alternate approach:
>> constraints on the column will have DEPENDENCY_AUTO type, while
>> constraints using the column will have DEPENDENCY_NORMAL type.

> O.K.  It seems nicer than my solution.

> Please find the patch attached.  I fixed the patch to decide the order
> of the list by deptype of the pg_depend record.

Patch applied with some minor comment improvements.  Thanks!

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [PATCHES] Patch for ALTER TABLE / TYPE

[NAKANO Yoshihisa]

Tom Lane wrote:
 > After looking at the test case a bit, I have an alternate approach:
> constraints on the column will have DEPENDENCY_AUTO type, while
> constraints using the column will have DEPENDENCY_NORMAL type.
> Therefore, if we drop the NORMAL ones before the AUTO ones, that
> should be enough to fix it.  This doesn't require much extra code,
> or any extra catalog searches, since the pg_depend record is already
> available in ATExecAlterColumnType where we need to decide whether
> to stick the item on the front or back of the list.

O.K.  It seems nicer than my solution.

Please find the patch attached.  I fixed the patch to decide the order
of the list by deptype of the pg_depend record.

Regards,
Nakano



*** postgresql-8.1.2-org/src/backend/commands/tablecmds.c   2005-11-23 
03:23:07.0 +0900
--- postgresql-8.1.2/src/backend/commands/tablecmds.c   2006-01-30 
09:43:13.0 +0900
***
*** 4970,4979 
Assert(foundObject.objectSubId == 0);
if 
(!list_member_oid(tab->changedConstraintOids, foundObject.objectId))
{
!   tab->changedConstraintOids = 
lappend_oid(tab->changedConstraintOids,
!   
   foundObject.objectId);
!   tab->changedConstraintDefs = 
lappend(tab->changedConstraintDefs,
! 
pg_get_constraintdef_string(foundObject.objectId));
}
break;
  
--- 4970,4996 
Assert(foundObject.objectSubId == 0);
if 
(!list_member_oid(tab->changedConstraintOids, foundObject.objectId))
{
!   char *defstring = 
pg_get_constraintdef_string(foundObject.objectId);
! 
!   /* FOREIGN KEY constraints depend on 
the indexes which depend on
!* PRIMARY KEY constraints, so we need 
to append FOREIGN KEY
!* constraints ahead of PRIMARY KEY 
constraints.  We decide this
!* by the dependency type.
!*/
!   if (foundDep->deptype == 
DEPENDENCY_NORMAL)
!   {
!   tab->changedConstraintOids = 
lcons_oid(foundObject.objectId,
!   
   tab->changedConstraintOids);
!   tab->changedConstraintDefs = 
lcons(defstring,
!   
   tab->changedConstraintDefs);
!   }
!   else
!   {
!   tab->changedConstraintOids = 
lappend_oid(tab->changedConstraintOids,
!   
 foundObject.objectId);
!   tab->changedConstraintDefs = 
lappend(tab->changedConstraintDefs,
!   
 defstring);
!   }
}
break;
  
***
*** 5141,5149 
  
/*
 * Now we can drop the existing constraints and indexes --- constraints
!* first, since some of them might depend on the indexes. It should be
!* okay to use DROP_RESTRICT here, since nothing else should be 
depending
!* on these objects.
 */
foreach(l, tab->changedConstraintOids)
{
--- 5158,5168 
  
/*
 * Now we can drop the existing constraints and indexes --- constraints
!* first, since some of them might depend on the indexes.  (FOREIGN KEY
!* constraints depend on the indexes of PRIMARY KEY constraints, but
!* these will have DEPENDENCY_NORMAL type and will be deleted before
!* PRIMARY KEY constraints.) It should be okay to use DROP_RESTRICT 
here,
!* since nothing else should be depending on these objects.
 */
foreach(l, tab->changedConstraintOids)
{

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

   http://archives.postgresql.org

Re: [PATCHES] Patch for ALTER TABLE / TYPE

[Tom Lane]

NAKANO Yoshihisa <[EMAIL PROTECTED]> writes:
> Please find the patch attached.  This is for the bug which is posted to
> hackers before.
> http://archives.postgresql.org/pgsql-hackers/2005-06/msg01442.php

> We can see a problem by this bug in following way.

> CREATE TABLE pktable (a int primary key);
> CREATE TABLE fktable (b int references pktable);
> ALTER TABLE pktable ALTER COLUMN a TYPE bigint;  -- succeed
> REINDEX TABLE pg_depend;
> ALTER TABLE pktable ALTER COLUMN a TYPE int; -- fail
> NOTICE:  constraint fktable_b_fkey on table fktable depends on index
> pktable_pkey
> ERROR:  cannot drop constraint pktable_pkey on table pktable because
> other objects depend on it
> HINT:  Use DROP ... CASCADE to drop the dependent objects too.

Ah, thanks for providing the simple test case.  Much nicer than
Neil's way...

I don't much like the patch though :-(.  It seems like a brute force
solution, and it's lacking error checking.

After looking at the test case a bit, I have an alternate approach:
constraints on the column will have DEPENDENCY_AUTO type, while
constraints using the column will have DEPENDENCY_NORMAL type.
Therefore, if we drop the NORMAL ones before the AUTO ones, that
should be enough to fix it.  This doesn't require much extra code,
or any extra catalog searches, since the pg_depend record is already
available in ATExecAlterColumnType where we need to decide whether
to stick the item on the front or back of the list.

regards, tom lane

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