DBIx::DataModel::Schema::Generator creating incorrect output

[Diab Jerius]

I'm creating schema with SQL::Translator so that I can generate both
DDL and DBIx::DataModel schema from a single source.

I've attached the code which generates the DDL and the schema code, as
well as its output.

I'm running into a problem where DBIx::DataModel::Schema::Generator is
emitting code which DBIx::DataModel is rejecting.

The problem seems to be associated with a table containing foreign
keys to two other tables, as well as a primary key composed of the
foreign keys. Here's the generated SQLite DDL:

BEGIN TRANSACTION;

CREATE TABLE object (
id INTEGER PRIMARY KEY NOT NULL
);

CREATE TABLE profiles (
id INTEGER PRIMARY KEY NOT NULL
);

CREATE TABLE observed_profiles (
object_id integer NOT NULL,
profile_id integer NOT NULL,
PRIMARY KEY (object_id, profile_id),
FOREIGN KEY (object_id) REFERENCES object(id) ON DELETE cascade,
FOREIGN KEY (profile_id) REFERENCES profiles(id) ON DELETE cascade
);

COMMIT;


When I run the output schema code, I get the following error message:

Association: numbers of columns do not match at Schema.pm line 5.

And there's something odd about the Association declarations in the
generated code:

->Association(
[qw/Object object 1 object_id profile_id/],
[qw/ObservedProfiles observed_profiles * object_id /])

->Association(
[qw/Profiles profile 1 object_id profile_id/],
[qw/ObservedProfiles observed_profiles * profile_id /])


Shouldn't the code look like this?

->Association(
[qw/Object object 1 id /],
[qw/ObservedProfiles observed_profiles * object_id /])

->Association(
[qw/Profiles profile 1 id /],
[qw/ObservedProfiles observed_profiles * profile_id /])


Thanks,

Diab

[Diab Jerius]

On Wed, Aug 12, 2015 at 12:22 PM, Diab Jerius <dje...@cfa.harvard.edu> wrote:
> I'm creating schema with SQL::Translator so that I can generate both
> DDL and DBIx::DataModel schema from a single source.
>

[...]

> I'm running into a problem where DBIx::DataModel::Schema::Generator is
> emitting code which DBIx::DataModel is rejecting.
>
> The problem seems to be associated with a table containing foreign
> keys to two other tables, as well as a primary key composed of the
> foreign keys.

I believe I've found the problem; a possible fix is here:

https://github.com/damil/DBIx-DataModel/pull/14

[Dami Laurent (PJ)]

> -----Message d'origine-----
> De : dbix-da...@googlegroups.com [mailto:dbix-
> data...@googlegroups.com] De la part de Diab Jerius

>
> I believe I've found the problem; a possible fix is here:
>
> https://github.com/damil/DBIx-DataModel/pull/14
>

Thanks for your work.
I'll look at your pull request next weekend.

Laurent D.

[Laurent Dami]

Le 12.08.2015 18:22, Diab Jerius a écrit :
> I'm creating schema with SQL::Translator so that I can generate both
> DDL and DBIx::DataModel schema from a single source.
>
> I've attached the code which generates the DDL and the schema code, as
> well as its output.
>
> I'm running into a problem where DBIx::DataModel::Schema::Generator is
> emitting code which DBIx::DataModel is rejecting.
>
>

Hi Diab,

I had a look at your post and at your pull request.
I was able to reproduce the original problem. Indeed there seems to be a
bug when generating a DBIDM Schema from SQL::Translator; I still don't
know if the bug is in Schema::Generator.pm (most probably), or in
SQL::Translator. Anyway, this should probably be solved by changing the
"parse_SQL_Translator" function.

However, when loading the SQL in SQLite, and then generating the DBIDM
Schema through parse_DBI, I see no problem. So the path you took in the
pull request doesn't seem appropriate to me. Moreover, if there are
several "ON DELETE CASCADE"for several foreign keys to the same primary
key, it's most probably a sign of some defect within the database
structure, so I wouldn't change the Generator.pm for that; I think it's
better to generate an error, and if people have a very good reason for
overriding it, and they know what they are doing, then they can always
hand-edit the generated schema.

I hope you feel not frustrated with this answer, but I won't incorporate
the pull request.
I'll try to see what is wrong with the SQL::Translator generated code.

Best regards, Laurent Dami

[Diab Jerius]

On Mon, Aug 17, 2015 at 4:08 PM, Laurent Dami <lauren...@free.fr> wrote:
> Le 12.08.2015 18:22, Diab Jerius a écrit :
>>
>> I'm creating schema with SQL::Translator so that I can generate both
>> DDL and DBIx::DataModel schema from a single source.
>>
>> I've attached the code which generates the DDL and the schema code, as
>> well as its output.
>>
>> I'm running into a problem where DBIx::DataModel::Schema::Generator is
>> emitting code which DBIx::DataModel is rejecting.
>>
>>
> Hi Diab,
>
> I had a look at your post and at your pull request.
> I was able to reproduce the original problem. Indeed there seems to be a bug
> when generating a DBIDM Schema from SQL::Translator; I still don't know if
> the bug is in Schema::Generator.pm (most probably), or in SQL::Translator.
> Anyway, this should probably be solved by changing the
> "parse_SQL_Translator" function.

Which is what PR #14 does.

>
> However, when loading the SQL in SQLite, and then generating the DBIDM
> Schema through parse_DBI, I see no problem. So the path you took in the pull
> request doesn't seem appropriate to me. Moreover, if there are several "ON
> DELETE CASCADE"for several foreign keys to the same primary key, it's most
> probably a sign of some defect within the database structure, so I wouldn't
> change the Generator.pm for that; I think it's better to generate an error,
> and if people have a very good reason for overriding it, and they know what
> they are doing, then they can always hand-edit the generated schema.

Now you've confused me. The problem which PR#14 is attempting to
solve isn't that there are multiple foreign keys to a single primary
key in another table, it's that parse_SQL_Translator is not using the
correct name for the primary key in the referred table, which is what
PR #14 corrects.

Are you referring to PR# 14 or PR #15 ?

If #15, I'm still confused, as what you've described isn't one of the
cases which I'm trying to accommodate. Would you please note which of
the four cases I list in the pull request comment you believe are bad
database designs?

[Diab Jerius]

I apologize; PR#14 does have a foreign key cascade fix which was
supposed to go into PR #15.
That should not have been in PR #14 at all. Again I apologize for
confusing the issues.

I believe that PR#14 without the foreign key cascade change is still
valid. I'll rework the code.


Thanks,
Diab