From: Ash Berlin Date: Tue, 1 Apr 2008 12:01:16 +0000 (+0000) Subject: Constraint/index name fix from rdj X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=0da8b7da7a9df7b84e9a3228cd1f5feb1d65bb07;p=dbsrgits%2FDBIx-Class-Historic.git Constraint/index name fix from rdj --- diff --git a/Changes b/Changes index 3d391bb..3516d74 100644 --- a/Changes +++ b/Changes @@ -7,6 +7,8 @@ Revision history for DBIx::Class - sqltargs respected correctly in deploy et al. - Added support for savepoints, and using them automatically in nested transactions if auto_savepoint is set in connect_info. + - Changed naming scheme for constraints and keys in the sqlt parser; + names should now be consistent and collision-free. 0.08010 2008-03-01 10:30 - Fix t/94versioning.t so it passes with latest SQL::Translator diff --git a/Makefile.PL b/Makefile.PL index f8ccc29..f45d51e 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -18,6 +18,7 @@ requires 'Class::Inspector' => 0; requires 'Class::Accessor::Grouped' => 0.05002; requires 'JSON::Any' => 1.00; requires 'Scope::Guard' => 0.03; +requires 'Digest::SHA1' => 2.00; # Perl 5.8.0 doesn't have utf8::is_utf8() requires 'Encode' => 0 if ($] <= 5.008000); diff --git a/lib/DBIx/Class.pm b/lib/DBIx/Class.pm index c8dbd36..9147f46 100644 --- a/lib/DBIx/Class.pm +++ b/lib/DBIx/Class.pm @@ -268,6 +268,8 @@ phaylon: Robert Sedlacek quicksilver: Jules Bean +rdj: Ryan D Johnson + sc_: Just Another Perl Hacker scotty: Scotty Allen diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index bfdaece..2c45eee 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -1538,6 +1538,7 @@ sub create_ddl_dir unless $dest_schema->name; } + $DB::single = 1; my $diff = SQL::Translator::Diff::schema_diff($source_schema, $db, $dest_schema, $db, $sqltargs diff --git a/lib/SQL/Translator/Parser/DBIx/Class.pm b/lib/SQL/Translator/Parser/DBIx/Class.pm index dc18bbb..1b6266f 100644 --- a/lib/SQL/Translator/Parser/DBIx/Class.pm +++ b/lib/SQL/Translator/Parser/DBIx/Class.pm @@ -14,6 +14,7 @@ $DEBUG = 0 unless defined $DEBUG; use Exporter; use Data::Dumper; +use Digest::SHA1 qw( sha1_hex ); use SQL::Translator::Utils qw(debug normalize_name); use base qw(Exporter); @@ -107,13 +108,12 @@ sub parse { if (!$source->compare_relationship_keys($unique_constraints{$uniq}, \@primary)) { $table->add_constraint( type => 'unique', - name => "$uniq", + name => _create_unique_symbol($uniq), fields => $unique_constraints{$uniq} ); my $index = $table->add_index( - # TODO: Pick a better than that wont conflict - name => $unique_constraints{$uniq}->[0], + name => _create_unique_symbol(join('_', @{$unique_constraints{$uniq}})), fields => $unique_constraints{$uniq}, type => 'NORMAL', ); @@ -179,7 +179,9 @@ sub parse { if (scalar(@keys)) { $table->add_constraint( type => 'foreign_key', - name => $table->name . "_fk_$keys[0]", + name => _create_unique_symbol($table->name + . '_fk_' + . join('_', @keys)), fields => \@keys, reference_fields => \@refkeys, reference_table => $rel_table, @@ -189,7 +191,7 @@ sub parse { ); my $index = $table->add_index( - name => join('_', @keys), + name => _create_unique_symbol(join('_', @keys)), fields => \@keys, type => 'NORMAL', ); @@ -209,5 +211,31 @@ sub parse { return 1; } -1; +# TODO - is there a reasonable way to pass configuration? +# Default of 64 comes from mysql's limit. +our $MAX_SYMBOL_LENGTH ||= 64; +our $COLLISION_TAG_LENGTH ||= 8; + +# ------------------------------------------------------------------- +# $resolved_name = _create_unique_symbol($desired_name) +# +# If desired_name is really long, it will be truncated in a way that +# has a high probability of leaving it unique. +# ------------------------------------------------------------------- +sub _create_unique_symbol { + my $desired_name = shift; + return $desired_name if length $desired_name <= $MAX_SYMBOL_LENGTH; + my $truncated_name = substr $desired_name, 0, $MAX_SYMBOL_LENGTH - $COLLISION_TAG_LENGTH - 1; + + # Hex isn't the most space-efficient, but it skirts around allowed + # charset issues + my $digest = sha1_hex($desired_name); + my $collision_tag = substr $digest, 0, $COLLISION_TAG_LENGTH; + + return $truncated_name + . '_' + . $collision_tag; +} + +1; diff --git a/t/86sqlt.t b/t/86sqlt.t index 987ee39..513a9b6 100644 --- a/t/86sqlt.t +++ b/t/86sqlt.t @@ -10,7 +10,7 @@ plan skip_all => 'SQL::Translator required' if $@; my $schema = DBICTest->init_schema; -plan tests => 77; +plan tests => 160; my $translator = SQL::Translator->new( parser_args => { @@ -40,12 +40,14 @@ my %fk_constraints = ( twokeys => [ { 'display' => 'twokeys->cd', + 'name' => 'twokeys_fk_cd', 'index_name' => 'cd', 'selftable' => 'twokeys', 'foreigntable' => 'cd', 'selfcols' => ['cd'], 'foreigncols' => ['cdid'], on_delete => '', on_update => '', deferrable => 0, }, { 'display' => 'twokeys->artist', + 'name' => 'twokeys_fk_artist', 'index_name' => 'artist', 'selftable' => 'twokeys', 'foreigntable' => 'artist', 'selfcols' => ['artist'], 'foreigncols' => ['artistid'], on_delete => 'CASCADE', on_update => 'CASCADE', deferrable => 1, @@ -56,12 +58,14 @@ my %fk_constraints = ( fourkeys_to_twokeys => [ { 'display' => 'fourkeys_to_twokeys->twokeys', + 'name' => 'fourkeys_to_twokeys_fk_t_cd_t_artist', 'index_name' => 't_cd_t_artist', 'selftable' => 'fourkeys_to_twokeys', 'foreigntable' => 'twokeys', 'selfcols' => ['t_artist', 't_cd'], 'foreigncols' => ['artist', 'cd'], on_delete => 'CASCADE', on_update => 'CASCADE', deferrable => 1, }, { - 'display' => 'fourkeys_to_twokeys->fourkeys', + 'display' => 'fourkeys_to_twokeys->fourkeys', 'index_name' => 'f_foo_f_goodbye_f_hello_f_bar', + 'name' => 'fourkeys_to_twokeys_fk_f_foo_f_goodbye_f_hello_f_bar', 'selftable' => 'fourkeys_to_twokeys', 'foreigntable' => 'fourkeys', 'selfcols' => [qw(f_foo f_bar f_hello f_goodbye)], 'foreigncols' => [qw(foo bar hello goodbye)], @@ -73,12 +77,14 @@ my %fk_constraints = ( cd_to_producer => [ { 'display' => 'cd_to_producer->cd', + 'name' => 'cd_to_producer_fk_cd', 'index_name' => 'cd', 'selftable' => 'cd_to_producer', 'foreigntable' => 'cd', 'selfcols' => ['cd'], 'foreigncols' => ['cdid'], on_delete => 'CASCADE', on_update => 'CASCADE', deferrable => 1, }, { 'display' => 'cd_to_producer->producer', + 'name' => 'cd_to_producer_fk_producer', 'index_name' => 'producer', 'selftable' => 'cd_to_producer', 'foreigntable' => 'producer', 'selfcols' => ['producer'], 'foreigncols' => ['producerid'], on_delete => '', on_update => '', deferrable => 1, @@ -89,12 +95,14 @@ my %fk_constraints = ( self_ref_alias => [ { 'display' => 'self_ref_alias->self_ref for self_ref', + 'name' => 'self_ref_alias_fk_self_ref', 'index_name' => 'self_ref', 'selftable' => 'self_ref_alias', 'foreigntable' => 'self_ref', 'selfcols' => ['self_ref'], 'foreigncols' => ['id'], on_delete => 'CASCADE', on_update => 'CASCADE', deferrable => 1, }, { 'display' => 'self_ref_alias->self_ref for alias', + 'name' => 'self_ref_alias_fk_alias', 'index_name' => 'alias', 'selftable' => 'self_ref_alias', 'foreigntable' => 'self_ref', 'selfcols' => ['alias'], 'foreigncols' => ['id'], on_delete => '', on_update => '', deferrable => 1, @@ -105,6 +113,7 @@ my %fk_constraints = ( cd => [ { 'display' => 'cd->artist', + 'name' => 'cd_fk_artist', 'index_name' => 'artist', 'selftable' => 'cd', 'foreigntable' => 'artist', 'selfcols' => ['artist'], 'foreigncols' => ['artistid'], on_delete => 'CASCADE', on_update => 'CASCADE', deferrable => 1, @@ -115,12 +124,14 @@ my %fk_constraints = ( artist_undirected_map => [ { 'display' => 'artist_undirected_map->artist for id1', + 'name' => 'artist_undirected_map_fk_id1', 'index_name' => 'id1', 'selftable' => 'artist_undirected_map', 'foreigntable' => 'artist', 'selfcols' => ['id1'], 'foreigncols' => ['artistid'], on_delete => 'CASCADE', on_update => '', deferrable => 1, }, { 'display' => 'artist_undirected_map->artist for id2', + 'name' => 'artist_undirected_map_fk_id2', 'index_name' => 'id2', 'selftable' => 'artist_undirected_map', 'foreigntable' => 'artist', 'selfcols' => ['id2'], 'foreigncols' => ['artistid'], on_delete => 'CASCADE', on_update => '', deferrable => 1, @@ -131,6 +142,7 @@ my %fk_constraints = ( track => [ { 'display' => 'track->cd', + 'name' => 'track_fk_cd', 'index_name' => 'cd', 'selftable' => 'track', 'foreigntable' => 'cd', 'selfcols' => ['cd'], 'foreigncols' => ['cdid'], on_delete => 'CASCADE', on_update => 'CASCADE', deferrable => 1, @@ -141,6 +153,7 @@ my %fk_constraints = ( treelike => [ { 'display' => 'treelike->treelike for parent', + 'name' => 'treelike_fk_parent', 'index_name' => 'parent', 'selftable' => 'treelike', 'foreigntable' => 'treelike', 'selfcols' => ['parent'], 'foreigncols' => ['id'], on_delete => 'CASCADE', on_update => 'CASCADE', deferrable => 1, @@ -151,6 +164,7 @@ my %fk_constraints = ( twokeytreelike => [ { 'display' => 'twokeytreelike->twokeytreelike for parent1,parent2', + 'name' => 'twokeytreelike_fk_parent1_parent2', 'index_name' => 'parent1_parent2', 'selftable' => 'twokeytreelike', 'foreigntable' => 'twokeytreelike', 'selfcols' => ['parent1', 'parent2'], 'foreigncols' => ['id1','id2'], on_delete => '', on_update => '', deferrable => 1, @@ -161,6 +175,7 @@ my %fk_constraints = ( tags => [ { 'display' => 'tags->cd', + 'name' => 'tags_fk_cd', 'index_name' => 'cd', 'selftable' => 'tags', 'foreigntable' => 'cd', 'selfcols' => ['cd'], 'foreigncols' => ['cdid'], on_delete => 'CASCADE', on_update => 'CASCADE', deferrable => 1, @@ -171,6 +186,7 @@ my %fk_constraints = ( bookmark => [ { 'display' => 'bookmark->link', + 'name' => 'bookmark_fk_link', 'index_name' => 'link', 'selftable' => 'bookmark', 'foreigntable' => 'link', 'selfcols' => ['link'], 'foreigncols' => ['id'], on_delete => '', on_update => '', deferrable => 1, @@ -180,12 +196,42 @@ my %fk_constraints = ( forceforeign => [ { 'display' => 'forceforeign->artist', + 'name' => 'forceforeign_fk_artist', 'index_name' => 'artist', 'selftable' => 'forceforeign', 'foreigntable' => 'artist', 'selfcols' => ['artist'], 'foreigncols' => ['artist_id'], on_delete => '', on_update => '', deferrable => 1, }, ], + # LongColumns + long_columns => [ + { + 'display' => 'long_columns->owner', + 'name' => 'long_columns_fk_64_character_column_aaaaaaaaaaaaaaaaaaa_1ca973e2', + 'index_name' => '64_character_column_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + 'selftable' => 'long_columns', 'foreigntable' => 'long_columns', + 'selfcols' => ['64_character_column_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'], + 'foreigncols' => ['lcid'], + on_delete => '', on_update => '', deferrable => 1, + }, + { + 'display' => 'long_columns->owner2', + 'name' => 'long_columns_fk_32_character_column_aaaaaaaaaaaa_32_cha_6060a8f3', + 'index_name' => '32_character_column_aaaaaaaaaaaa_32_character_column_bb_30f7a7fe', + 'selftable' => 'long_columns', 'foreigntable' => 'long_columns', + 'selfcols' => ['32_character_column_bbbbbbbbbbbb', '32_character_column_aaaaaaaaaaaa'], + 'foreigncols' => ['32_character_column_aaaaaaaaaaaa', '32_character_column_bbbbbbbbbbbb'], + on_delete => '', on_update => '', deferrable => 1, + }, + { + 'display' => 'long_columns->owner3', + 'name' => 'long_columns_fk_16_character_col', + 'index_name' => '16_character_col', + 'selftable' => 'long_columns', 'foreigntable' => 'long_columns', + 'selfcols' => ['16_character_col'], 'foreigncols' => ['8_char_c'], + on_delete => '', on_update => '', deferrable => 1, + }, + ], ); my %unique_constraints = ( @@ -193,6 +239,7 @@ my %unique_constraints = ( cd => [ { 'display' => 'cd artist and title unique', + 'name' => 'cd_artist_title', 'table' => 'cd', 'cols' => ['artist', 'title'], }, ], @@ -201,14 +248,39 @@ my %unique_constraints = ( producer => [ { 'display' => 'producer name unique', + 'name' => 'prod_name', # explicit name 'table' => 'producer', 'cols' => ['name'], }, ], + long_columns => [ + { + 'display' => 'long but not quite truncated unique', + 'name' => 'long_columns_16_character_col_32_character_column_aaaaaaaaaaaa', + 'table' => 'long_columns', 'cols' => [qw( 32_character_column_aaaaaaaaaaaa 16_character_col )], + }, + { + 'display' => 'multi column truncated unique', + 'name' => 'long_columns_8_char_c_16_character_col_32_character_col_ee4a438c', + 'table' => 'long_columns', 'cols' => [qw( 32_character_column_aaaaaaaaaaaa 16_character_col 8_char_c )], + }, + { + 'display' => 'different multi column truncated unique with same base', + 'name' => 'long_columns_8_char_c_16_character_col_32_character_col_c5dbc7a7', + 'table' => 'long_columns', 'cols' => [qw( 32_character_column_bbbbbbbbbbbb 16_character_col 8_char_c )], + }, + { + 'display' => 'single column truncated unique', + 'name' => 'long_columns_64_character_column_aaaaaaaaaaaaaaaaaaaaaa_095dc664', + 'table' => 'long_columns', 'cols' => ['64_character_column_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'], + }, + ], + # TwoKeyTreeLike twokeytreelike => [ { 'display' => 'twokeytreelike name unique', + 'name' => 'tktlnameunique', # explicit name 'table' => 'twokeytreelike', 'cols' => ['name'], }, ], @@ -218,6 +290,7 @@ my %unique_constraints = ( # employee => [ # { # 'display' => 'employee position and group_id unique', +# 'name' => 'position_group', # 'table' => 'employee', cols => ['position', 'group_id'], # }, # ], @@ -264,6 +337,7 @@ for my $expected_constraints (keys %unique_constraints) { 'UNIQUE', $expected_constraint->{table}, $expected_constraint->{cols}, ); ok( defined($constraint), "UNIQUE constraint matching `$desc' found" ); + test_unique($expected_constraint, $constraint); } } @@ -355,10 +429,23 @@ sub get_index { sub test_fk { my ($expected, $got) = @_; my $desc = $expected->{display}; + is( $got->name, $expected->{name}, + "name parameter correct for `$desc'" ); is( $got->on_delete, $expected->{on_delete}, "on_delete parameter correct for `$desc'" ); is( $got->on_update, $expected->{on_update}, "on_update parameter correct for `$desc'" ); is( $got->deferrable, $expected->{deferrable}, "is_deferrable parameter correct for `$desc'" ); + + my $index = get_index( $got->table, { fields => $expected->{selfcols} } ); + ok( defined $index, "index exists for `$desc'" ); + is( $index->name, $expected->{index_name}, "index has correct name for `$desc'" ); +} + +sub test_unique { + my ($expected, $got) = @_; + my $desc = $expected->{display}; + is( $got->name, $expected->{name}, + "name parameter correct for `$desc'" ); } diff --git a/t/lib/DBICTest/Schema.pm b/t/lib/DBICTest/Schema.pm index e3329a7..a07db60 100644 --- a/t/lib/DBICTest/Schema.pm +++ b/t/lib/DBICTest/Schema.pm @@ -38,7 +38,8 @@ __PACKAGE__->load_classes(qw/ qw/SelfRefAlias TreeLike TwoKeyTreeLike Event EventTZ NoPrimaryKey/, qw/Collection CollectionObject TypedObject/, qw/Owners BooksInLibrary/, - qw/ForceForeign/ + qw/ForceForeign/, + qw/LongColumns/, ); sub sqlt_deploy_hook {