Composite foreign keys and ConventionManager

[Douglas Wilson]

I want foreign key relationships to just have the lowercased-class
name whenever possible. I have the below code as part of my
Convention Manager. The problem is with tables that have multi-column
foreign keys. First this sub is first called with only one column of
the foreign key, the f_class gets added to the $used_names, then the
sub is called with both columns, but the name I'd like to use is
"used", so I end up with "f_class_obj" as the foreign key method name,
when I'd like just "f_class". The plain "f_class" method with one
column is never added as an actual foreign key though, so it seems
like at least it's filtered out later.

sub auto_foreign_key_name {
my($self, $f_class, $current_name, $key_columns, $used_names) = @_;
(my $name = lc($f_class)) =~ s/.*:://;

# Avoid method name conflicts
if($self->method_name_conflicts($name) || $used_names->{$name}) {
for my $s ('_obj', '_object') {
# Try the name with a suffix appended
unless($self->method_name_conflicts($name . $s) ||
$used_names->{$name . $s})
{
return $name . $s;
}
}

my $i = 1;

# Give up and go with numbers...
$i++ while($self->method_name_conflicts($name . $i) ||
$used_names->{$name . $i});

return $name . $i;
}
return $name;
}

[Douglas Wilson]

On Oct 20, 10:41 am, Douglas Wilson <douglasg.wil...@gmail.com> wrote:
> I want foreign key relationships to just have the lowercased-class
> name whenever possible.  I have the below code as part of my
> Convention Manager. The problem is with tables that have multi-column
> foreign keys. First this sub is first called with only one column of
> the foreign key, the f_class gets added to the $used_names, then the
> sub is called with both columns, but the name I'd like to use is
> "used", so I end up with "f_class_obj" as the foreign key method name,
> when I'd like just "f_class". The plain "f_class" method with one
> column is never added as an actual foreign key though, so it seems
> like at least it's filtered out later.

I've tried to come up with a simple test case of this, but since I was
trying at home, I used SQLite, and SQLite doesn't call
Rose::DB::Object::Metadata::Auto::auto_generate_foreign_keys where the
problem occurs (but after examining the function, it's now obvious why
it occurs). I think the columns in FK's should just be grouped by
FK_NAME, not sent incrementally to
ConventionManager::auto_foreign_key_name, but I assume by this line
that FK_NAME is not defined in all databases:
my $fk_id = $fk_info->{'RDBO_FK_ID'} = $fk_info->{'FK_NAME'} ||
$fk_info->{'UK_NAME'};

Using UK_NAME seems like a mistake, because you can have multiple
foreign key columns in a table all pointing to the same primary key
table, though I suppose if FK_NAME is undefined, what else is there to
do? But if FK_NAME is defined, then it ought to work as I've said
before.

-Doug

[Douglas Wilson]

I've uploaded a patch for Rose::DB::Object::Metadata::Auto.pm
auto_generate_foreign_keys() that fixes the problem.

Basically it pre-populates the %fk hash as the info is being
fetched from $dbh->foreign_key_info(), so that it's
all populated by the time the call to the ConventionManager
auto_foreign_key_name method is made.

[John Siracusa]

What version is that a patch against? Did you run the RDBO test suite
with your patched version to see that it doesn't break anything?

-John

[Douglas Wilson]

On Nov 19, 4:57 pm, John Siracusa <sirac...@gmail.com> wrote:
>
> What version is that a patch against?  Did you run the RDBO test suite
> with your patched version to see that it doesn't break anything?

I've just uploaded a new patch against version 784. I ran the tests
with the RDBO_ORACLE_* env vars set, and all passed, with 9430 skipped
(10285 skipped before I set any variables).

-Doug

[Douglas Wilson]

On Nov 20, 9:33 am, Douglas Wilson <douglasg.wil...@gmail.com> wrote:
>
> I've just uploaded a new patch against version 784. I ran the tests
> with the RDBO_ORACLE_* env vars set, and all passed, with 9430 skipped
> (10285 skipped before I set any variables).

Uploaded new version. "New" Auto.pm in the diff file has timestamp:
+++ Auto.pm Fri Nov 20 10:01:29 2009

[John Siracusa]

Got it. I'd still like to have some tests for this, though. Can you
describe a scenario that was broken before but fixed with this patch?

-John

[Douglas Wilson]

On Nov 20, 10:14 am, John Siracusa <sirac...@gmail.com> wrote:
>
> Got it.  I'd still like to have some tests for this, though.  Can you
> describe a scenario that was broken before but fixed with this patch?

Say you have a table/class order_items (a detail to an orders table),
with a composite primary key (order_id, item_id). Another table,
order_item_comments, has a foreign key to order_items (with maybe
primary key (order_id, item_id, comment_num)).

One would like the foreign key relationship from order_item_comments
to order_items to be named "order_item", but since the
ConventionManager auto_foreign_key_name() gets called once with just
the first column "order_id", the foreign key name "order_item" gets
marked as used, and when auto_foreign_key_name() gets called again
later with both columns, the foreign key name ends up beign
"order_item_obj" instead.

That's what this fixes :-)

HTH,
Doug

[Douglas Wilson]

On Nov 20, 11:40 am, Douglas Wilson <douglasg.wil...@gmail.com> wrote:
>
> That's what this fixes :-)

And as I previously mention, SQLite has it's own Auto.pm that
overrides this method and doesn't appear to have the problem. I see
Informix and MySQL also override the method, but I haven't looked
closely to see if they have the same problem.

HTH,
Doug

[John Siracusa]

On Fri, Nov 20, 2009 at 3:06 PM, Douglas Wilson
<douglas...@gmail.com> wrote:
> And as I previously mention, SQLite has it's own Auto.pm that
> overrides this method and doesn't appear to have the problem. I see
> Informix and MySQL also override the method, but I haven't looked
> closely to see if they have the same problem.

Okay, I looked at those and I don't think they suffer from the same
problem. All tests pass for me against Pg, SQLite, Oracle, and MySQL,
so I've submitted your patch. Thanks for the contribution.

-John