Table.tometadata() foreign key copying behavior with different schemas

[Dimitris Theodorou]

I think the behavior of Table.tometadata()'s copying of foreign keys is unexpected when it comes to copying foreign keys that reference tables in a schema different than the schema of the table being copied. This occurred to me in the discussion at https://groups.google.com/forum/#!topic/sqlalchemy-alembic/FA8wpmPJc7U

For instance, when we have two tables in different schemas

m = Metadata()
t = Table('t', m,
    Column('x', Integer, ForeignKey('a.x')),
    schema="shared")

t1 = Table('t1', m,
    Column('t_x', Integer, ForeignKey('shared.t.x')),
    schema="schema1")

m2 = Metadata()
t1_copy = t1.tometadata(m2)

t1_copy_schema2 = t1.tometadata(m2, schema="schema2")

After this, t1_copy.t_x now references a column schema1.t.x, while t1_copy_schema2.t_x references a schema2.t.x, both of which are wrong.

I think the correct behavior would be to only modify the schema of the foreign key's reference only if that schema is the same as the table's original schema. To be more precise:

1) If referenced_schema == previous_schema, update as is the current behavior 2) If referenced_schema != previous_schema, then need to decide how/if to update the referenced column's schema. I would leave the schema unchanged, or pass a dictionary that maps previous schemas to the new ones.

In code, something like this:

def tometadata(self, metadata, schema=RETAIN_SCHEMA, fk_schema_replace={}):
    # ....
    #section on constraints copy
        previous_schema = self.schema
        #by default, modify FK's schema if the schema is the same as the table's
        if previous_schema not in fk_schema_replace:
            fk_schema_replace.update({previous_schema: schema})
        for c in self.constraints:
            constraint_schema = schema
            if isinstance(c, ForeignKeyConstraint):
                fk_schema = c.elements[0](0).column.table.schema
                constraint_schema = fk_schema_replace.get(fk_schema)
                #if a replacement is found it is used, otherwise schema is unchanged
            table.append_constraint(c.copy(schema=constraint_schema, target_table=table))

With this change, I can do

t1_copy = t1.tometadata(m2)

t1_copy_schema2 = t1.tometadata(m2, schema="schema2", fk_schema_replace={'shared':'schema2'})

Then t1_copy.t_x will reference shared.t.x, while t1_copy_schema2.t_x will reference schema2.t.x

Thoughts?

[Dimitris Theodorou]

Ticket contains some mistakes, but I can't find a way to edit it:

The original code should be

m = Metadata(schema="shared")
t = Table('t', m,
    Column('x', Integer))

t1 = Table('t1', m,
    Column('t_x', Integer, ForeignKey('shared.t.x')),
    schema="schema1")

m2 = Metadata()
t1_copy = t1.tometadata(m2)

t1_copy_schema2 = t1.tometadata(m2, schema="schema2")

Now t1_copy.t_x now references a column schema1.t.x, while t1_copy_schema2.t_x references a schema2.t.x, both of which are wrong.

The proposed changed changes these results to shared.t.x and schema2.t.x respectively Replying to dtheodor:

I think the behavior of Table.tometadata()'s copying of foreign keys is unexpected when it comes to copying foreign keys that reference tables in a schema different than the schema of the table being copied. This occurred to me in the discussion at https://groups.google.com/forum/#!topic/sqlalchemy-alembic/FA8wpmPJc7U

For instance, when we have two tables in different schemas {{{#!python m = Metadata() t = Table('t', m, Column('x', Integer, ForeignKey('a.x')), schema="shared")

t1 = Table('t1', m, Column('t_x', Integer, ForeignKey('shared.t.x')), schema="schema1")

m2 = Metadata() t1_copy = t1.tometadata(m2)

t1_copy_schema2 = t1.tometadata(m2, schema="schema2") }}}

After this, t1_copy.t_x now references a column schema1.t.x, while t1_copy_schema2.t_x references a schema2.t.x, both of which are wrong.

I think the correct behavior would be to only modify the schema of the foreign key's reference only if that schema is the same as the table's original schema. To be more precise:

1) If referenced_schema == previous_schema, update as is the current behavior 2) If referenced_schema != previous_schema, then need to decide how/if to update the referenced column's schema. I would leave the schema unchanged, or pass a dictionary that maps previous schemas to the new ones.

In code, something like this:

{{{#!python def tometadata(self, metadata, schema=RETAIN_SCHEMA, fk_schema_replace={}): # .... #section on constraints copy previous_schema = self.schema #by default, modify FK's schema if the schema is the same as the table's if previous_schema not in fk_schema_replace: fk_schema_replace.update({previous_schema: schema}) for c in self.constraints: constraint_schema = schema if isinstance(c, ForeignKeyConstraint): fk_schema = c.elements0.column.table.schema constraint_schema = fk_schema_replace.get(fk_schema) #if a replacement is found it is used, otherwise schema is unchanged table.append_constraint(c.copy(schema=constraint_schema, target_table=table)) }}}

With this change, I can do {{{#!python t1_copy = t1.tometadata(m2)

t1_copy_schema2 = t1.tometadata(m2, schema="schema2", fk_schema_replace={'shared':'schema2'}) }}}

Then t1_copy.t_x will reference shared.t.x, while t1_copy_schema2.t_x will reference schema2.t.x

Thoughts?

[Mike Bayer]

yeah I had thought it already worked that way in that it wouldn't change the schema of a referenced table with an already different schema, however I think what i was likely remembering was logic that's in Postgresql's reflection system specifically, to deal with the fact that PG won't actually tell us the schema of an FK if that schema is in the current search path.

the second thought is that the change here, while I think that's how it should be, is not backwards compatible. So I'd want to check two things - one is that it has always worked this way, e.g. the current system isn't there for any particular reason, and two is that there's no tests for this behavior right now, which would kind of imply we've just never considered/supported this use. At some point we received a significant patch that added the "schema" argument to MetaData in the first place, and the patch went through a lot of review, so I'm surprised we never touched this aspect of it at all.

[Mike Bayer]

both of these changes have been made in 2d4b457924fa722ef2cf. this includes the FK schema change based on the referenced schema being the same as the parent schema, as well as a new optional callable referred_schema_fn which can be provided to supply any kind of logic to how the referenced schemas should be assigned.

[Mike Bayer]

Removing milestone: 0.9.2 (automated comment)