Bug 2600 - Database schema incorrect concerning bitrate/seconds. Noticable only with mySQL strict DB checking
: Database schema incorrect concerning bitrate/seconds. Noticable only with my...
Status: RESOLVED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Database
: unspecified
: All All
: P2 normal (vote)
: ---
Assigned To: Dan Sully
http://forums.slimdevices.com/showthr...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-11-19 05:17 UTC by Jim
Modified: 2008-09-15 14:39 UTC (History)
0 users

See Also:
Category: ---


Attachments
Patch to dbcreate.sql (4.17 KB, patch)
2005-11-19 05:19 UTC, Jim
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jim 2005-11-19 05:17:57 UTC
e.g.

2005-11-04 13:29:31.3139 Couldn't create track for file:///D:/my.flac : Can't insert new Slim::DataStores::DBI::Track: DBD::mysql::st execute fail
ed: Data truncated for column 'bitrate' at row 1 at D:\Slim\CPAN/DBIx/Contextual
Fetch.pm line 51.

Also other users reporting similar problems related to over-long strings & MySQL.

See thread http://forums.slimdevices.com/showthread.php?t=17844 for more info.

Suggested patch:

Index: SQL/mysql/dbcreate.sql
===================================================================
--- SQL/mysql/dbcreate.sql	(revision 5254)
+++ SQL/mysql/dbcreate.sql	(working copy)
@@ -24,9 +24,9 @@
 CREATE TABLE tracks (
   id int(10) unsigned NOT NULL auto_increment,
   url text NOT NULL,
-  title varchar(255),
-  titlesort varchar(255),
-  titlesearch varchar(255),
+  title text,
+  titlesort text,
+  titlesearch text,
   album  int(10) unsigned,
   tracknum  int(10) unsigned,
   content_type varchar(255),
@@ -36,11 +36,11 @@
   audio_size  int(10) unsigned,
   audio_offset  int(10) unsigned,
   year  smallint(5) unsigned,
-  secs  int(10) unsigned,
+  secs  float unsigned,
   cover varchar(255),
   thumb varchar(255),
   vbr_scale varchar(255),
-  bitrate  int(10) unsigned,
+  bitrate  float unsigned,
   samplerate  int(10) unsigned,
   samplesize  int(10) unsigned,
   channels  tinyint(1) unsigned,
@@ -64,11 +64,11 @@

   replay_gain float,
   replay_peak float,
   multialbumsortkey  text,
-  INDEX trackTitleIndex (title),
+  INDEX trackTitleIndex (title(255)),
   INDEX trackAlbumIndex (album),
   INDEX ctSortIndex (content_type),
-  INDEX trackSortIndex (titlesort),
-  INDEX trackSearchIndex (titlesearch),
+  INDEX trackSortIndex (titlesort(255)),
+  INDEX trackSearchIndex (titlesearch(255)),
   INDEX trackRatingIndex (rating),
   INDEX trackPlayCountIndex (playCount),
   INDEX trackAudioIndex (audio),
@@ -110,10 +110,10 @@
 --
 CREATE TABLE albums (
   id int(10) unsigned NOT NULL auto_increment,
-  title varchar(255),
-  titlesort varchar(255),
-  titlesearch varchar(255),
-  contributor varchar(255),
+  title text,
+  titlesort text,
+  titlesearch text,
+  contributor text,
   compilation tinyint(1) unsigned,
   year  smallint(5) unsigned,
   artwork int(10) unsigned, -- pointer to a track id that contains artwork
@@ -123,9 +123,9 @@
   replay_peak float,
   musicbrainz_id varchar(40),	-- musicbrainz uuid (36 bytes of text)
   musicmagic_mixable tinyint(1) unsigned,
-  INDEX albumsTitleIndex (title),
-  INDEX albumsSortIndex (titlesort),
-  INDEX albumsSearchIndex (titlesearch),
+  INDEX albumsTitleIndex (title(255)),
+  INDEX albumsSortIndex (titlesort(255)),
+  INDEX albumsSearchIndex (titlesearch(255)),
   INDEX compilationSortIndex (compilation),
   PRIMARY KEY (id)
 ) TYPE=InnoDB;
@@ -137,16 +137,16 @@
 --
 CREATE TABLE contributors (
   id int(10) unsigned NOT NULL auto_increment,
-  name varchar(255),
-  namesort varchar(255),
-  namesearch varchar(255),
+  name text,
+  namesort text,
+  namesearch text,
   moodlogic_id  int(10) unsigned,
   moodlogic_mixable tinyint(1) unsigned,
   musicbrainz_id varchar(40),	-- musicbrainz uuid (36 bytes of text)
   musicmagic_mixable tinyint(1) unsigned,
-  INDEX contributorsNameIndex (name),
-  INDEX contributorsSortIndex (namesort),
-  INDEX contributorsSearchIndex (namesearch),
+  INDEX contributorsNameIndex (name(255)),
+  INDEX contributorsSortIndex (namesort(255)),
+  INDEX contributorsSearchIndex (namesearch(255)),
   PRIMARY KEY (id)
 ) TYPE=InnoDB;
 
@@ -158,11 +158,11 @@
   role  int(10) unsigned,
   contributor  int(10) unsigned,
   track  int(10) unsigned,
-  namesort varchar(255),
+  namesort text,
   INDEX contributor_trackContribIndex (contributor),
   INDEX contributor_trackTrackIndex (track),
   INDEX contributor_trackRoleIndex (role),
-  INDEX contributor_trackSortIndex (namesort),
+  INDEX contributor_trackSortIndex (namesort(255)),
   PRIMARY KEY (id),
   FOREIGN KEY (`track`) REFERENCES `tracks` (`id`) ON DELETE NO ACTION,
   FOREIGN KEY (`contributor`) REFERENCES `contributors` (`id`) ON DELETE NO ACTION
@@ -189,15 +189,15 @@
 --
 CREATE TABLE genres (
   id int(10) unsigned NOT NULL auto_increment,
-  name varchar(255),
-  namesort varchar(255),
-  namesearch varchar(255),
+  name text,
+  namesort text,
+  namesearch text,
   moodlogic_id  int(10) unsigned,
   moodlogic_mixable tinyint(1) unsigned,
   musicmagic_mixable tinyint(1) unsigned,
-  INDEX genreNameIndex (name),
-  INDEX genreSortIndex (namesort),
-  INDEX genreSearchIndex (namesearch),
+  INDEX genreNameIndex (name(255)),
+  INDEX genreSortIndex (namesort(255)),
+  INDEX genreSearchIndex (namesearch(255)),
   PRIMARY KEY (id)
 ) TYPE=InnoDB;
Comment 1 Jim 2005-11-19 05:19:33 UTC
Created attachment 1034 [details]
Patch to dbcreate.sql
Comment 2 Jim 2005-11-19 05:51:20 UTC
In addition I have since scanning some old MP3's noticed an additional problem with the YEAR.

Some MP3's have the year set as nothing, which Slim is reading back as the string "".  The DB then errors when it tries to write the string "" to the integer field Year.

Simple fix would be to change the Year to char(4), but then we are making alphanumeric what is essentially a number - plus in the future this date field might want to be changed to a real date, to allow for more accurate dating.

My suggestion would be to fix the various tag-reading modules to ensure if Year is "" (or possibly even "buggy" alpha chars) then it is converted to null.

Comments ?
Comment 3 Michael Wagner 2005-11-19 06:41:45 UTC
(In reply to comment #2)
> Some MP3's have the year set as nothing, which Slim is reading back as the
> string "".  The DB then errors when it tries to write the string "" to the
> integer field Year.

In the id3 definition, year is a 4 digit numerical string. But nothing architectural forces it to be either numeric or 4 digits, so you can expect defective or substandard taggers to potentially put anything in there. 

The way the tag reading code is written, it will make a copy of the tags (and this is probably as it should be).

There should probably be a layer between the tag reader(s) and the database, that acts like lint (the unix utility) and picks nits and maps to Slims needs for more strict data typing.

> Simple fix would be to change the Year to char(4), but then we are making
> alphanumeric what is essentially a number - plus in the future this date field
> might want to be changed to a real date, to allow for more accurate dating.

Yeah, this feels like a step backwards to me ... 

> My suggestion would be to fix the various tag-reading modules to ensure if Year
> is "" (or possibly even "buggy" alpha chars) then it is converted to null.

That would be my choice. 

It looks like the place to put this code would be in Music::Info.pm. Below that, the routines in CPAN seem to do "by the book" tag interpretation. The ones in Formats taje that and do some amount of tag warping and filling in or correcting specific to the individual tag type. 

Music::Info.pm seems to be where individual tags and tag types seem to be transformed into something Slim can treat uniformly. Also, missing or incomplete tag info is guessed at from file names, etc. But it's a big module. I'm not sure where would be the best place. I've never looked through it before so I don't know it really at all.
Comment 4 Dan Sully 2005-11-21 10:01:40 UTC
Thanks! Fixed in change 5269