Commit 5bf3f7be authored by Matěj Laitl's avatar Matěj Laitl
Browse files

DB: change lyrics table: text url -> integer url pointing to the urls table

I believe that the old lyrics table structure was more or less a mistake:
TABLE lyrics (
    id INTEGER PRIMARY KEY, -- actually never used in code
    url VARBINARY(324), -- actually a rpath from urls table
    lyrics TEXT
)

Apart from data duplication and non-conformity to the "update anomaly"
requirement of the database design, there was additional problem that rpath
itself is not unique. The (deviceId, rpath) is.

This change makes the lyrics table look more sane:
TABLE lyrics (
    url INTEGER PRIMARY KEY, -- points to url.id column
    lyrics TEXT
)

with an effort to transition existing entries. The transition of 5000
lyrics entries takes 16s on my 2.5 GHz Core i5 (one core used), so it
should be acceptable.

This is the first time I'm changing db structure, I'd be glad to
receive thorough review, namely of the update13to14() method and
especially the duplicate-removing query. This is critical because
database-corrupting fault would leave many Amarok users in a state
where they would need to drop their database to make Amarok working
again.

ChangeLog of the unrelated iPod fix is updated because DB_VERSION bump
triggers full collection rescan as far as it is documented.

BUG: 242350
FIXED-IN: 2.6
REVIEW: 104966
parent ac9b9bd6
......@@ -49,6 +49,8 @@ VERSION 2.6-Beta 1
vice versa. (BR 142579)
CHANGES:
* Database structure (lyrics table) was updated. Starting Amarok for the first
time after the upgrade may take up to one minute as the data is migrated.
* Amazon store: try to show a sensible default in the country selection.
* Only offer delete action when Shift key is pressed in Collection context menu.
* Only offer move action when Shift key is pressed in Collection context menu.
......@@ -92,6 +94,7 @@ VERSION 2.6-Beta 1
"1.2 GB free" is shown instead of "85% used"; thicker capacity bar.
BUGFIXES:
* Saved lyrics are preserved when the track is moved. (BR 242350)
* Lyrics, labels and album actions are correctly displayed for tracks
from main and saved playlists on Amarok startup. (BR 299150)
* Lyrics applet scrolls more intelligently. Patch by Alexander Potashev.
......@@ -114,8 +117,7 @@ VERSION 2.6-Beta 1
correctly; users are notified about these when iPod is plugged in.
* iPod playlists now work correctly. (BR 289304)
* Show correct error when transferring unsupported files to iPod. (BR 234876)
* Fix m4a tracks transferred as mp4 videos to iPods. Affected users need to
fully rescan their collection. (BR 268238)
* Fix m4a tracks transferred as mp4 videos to iPods. (BR 268238)
* Combination of double click and context menu actions caused multiple
delete of downloaded podcast actions, etc. (BR 297092)
* Prevent merging tracks with same title but different track (disc) numbers;
......
......@@ -221,11 +221,9 @@ DROP TABLE IF EXISTS `lyrics`;
/*!40101 SET @saved_cs_client = @@character_set_client */;
/*!40101 SET character_set_client = utf8 */;
CREATE TABLE `lyrics` (
`id` int(11) NOT NULL AUTO_INCREMENT,
`url` varchar(324) COLLATE utf8_bin DEFAULT NULL,
`url` int(11) NOT NULL,
`lyrics` text COLLATE utf8_bin,
PRIMARY KEY (`id`),
UNIQUE KEY `lyrics_url` (`url`)
PRIMARY KEY (`url`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8 COLLATE=utf8_bin;
/*!40101 SET character_set_client = @saved_cs_client */;
......
......@@ -30,7 +30,7 @@
#include <KGlobal>
#include <KMessageBox>
static const int DB_VERSION = 13;
static const int DB_VERSION = 14;
int
DatabaseUpdater::expectedDatabaseVersion()
......@@ -141,6 +141,11 @@ DatabaseUpdater::update()
upgradeVersion12to13();
dbVersion = 13;
}
if( dbVersion == 13 && dbVersion < DB_VERSION )
{
upgradeVersion13to14();
dbVersion = 14;
}
/*
if( dbVersion == X && dbVersion < DB_VERSION )
{
......@@ -628,6 +633,37 @@ DatabaseUpdater::upgradeVersion12to13()
m_collection->sqlStorage()->query( "UPDATE urls SET uniqueid = REPLACE(uniqueid, 'MB_', 'mb-');" );
}
void
DatabaseUpdater::upgradeVersion13to14()
{
DEBUG_BLOCK
SqlStorage *storage = m_collection->sqlStorage();
/* Following commands transition lyrics table from text-based urls (in fact just rpath
* parts) to references to urls table. */
// first, rename column
storage->query( "ALTER TABLE lyrics CHANGE url rpath VARCHAR(324) CHARACTER SET utf8 COLLATE utf8_bin NULL DEFAULT NULL" );
// add integer column for url id
storage->query( "ALTER TABLE lyrics ADD COLUMN url INT NULL DEFAULT NULL FIRST" );
// try to extract url id from urls table using rpath
storage->query( "UPDATE lyrics l SET l.url = (SELECT u.id FROM urls u WHERE u.rpath = l.rpath LIMIT 1)" );
// delete entries with no matches in urls table; these should be just stale ones
storage->query( "DELETE FROM lyrics WHERE url IS NULL" );
// make the url columnt non-null
storage->query( "ALTER TABLE lyrics MODIFY url INT NOT NULL" );
// select duplicate ids into temporary table
storage->query( "CREATE TEMPORARY TABLE duplicate_lyrics_ids ( id INT NOT NULL ) "
"ENGINE=MEMORY SELECT dupl.id FROM lyrics orig "
"LEFT JOIN lyrics dupl ON dupl.url = orig.url AND dupl.id > orig.id" );
// delete duplicate lyrics entries
storage->query( "DELETE FROM lyrics WHERE id IN (SELECT id FROM duplicate_lyrics_ids)" );
// drop unwanted columns along with indexes defined on them
storage->query( "ALTER TABLE lyrics DROP id, DROP rpath" );
// add primary key; should definitely not fail as we have removed duplicate entries
storage->query( "ALTER TABLE lyrics ADD PRIMARY KEY(url)" );
}
void
DatabaseUpdater::cleanupDatabase()
......@@ -831,12 +867,10 @@ DatabaseUpdater::createTables() const
}
{
QString q = "CREATE TABLE lyrics ("
"id " + storage->idType() +
",url " + storage->exactIndexableTextColumnType() +
"url INTEGER PRIMARY KEY"
",lyrics " + storage->longTextColumnType() +
") COLLATE = utf8_bin ENGINE = MyISAM;";
storage->query( q );
storage->query( "CREATE UNIQUE INDEX lyrics_url ON lyrics(url);" );
}
storage->query( "INSERT INTO admin(component,version) "
"VALUES('AMAROK_TRACK'," + QString::number( DB_VERSION ) + ");" );
......
......@@ -67,6 +67,7 @@ public:
void upgradeVersion10to11();
void upgradeVersion11to12();
void upgradeVersion12to13();
void upgradeVersion13to14();
/** Checks the given table for redundant entries.
* Table can be artist,album,genre,composer,urls or year
......
......@@ -1155,8 +1155,8 @@ SqlTrack::collection() const
QString
SqlTrack::cachedLyrics() const
{
QString query = QString( "SELECT lyrics FROM lyrics WHERE url = '%1'" )
.arg( m_collection->sqlStorage()->escape( m_rpath ) );
/* We don't cache the string as it may be potentially very long */
QString query = QString( "SELECT lyrics FROM lyrics WHERE url = %1" ).arg( m_urlId );
QStringList result = m_collection->sqlStorage()->query( query );
if( result.isEmpty() )
return QString();
......@@ -1166,26 +1166,23 @@ SqlTrack::cachedLyrics() const
void
SqlTrack::setCachedLyrics( const QString &lyrics )
{
QString query = QString( "SELECT count(*) FROM lyrics WHERE url = '%1'")
.arg( m_collection->sqlStorage()->escape(m_rpath) );
QString query = QString( "SELECT count(*) FROM lyrics WHERE url = %1").arg( m_urlId );
const QStringList queryResult = m_collection->sqlStorage()->query( query );
if( queryResult.isEmpty() )
return;
return; // error in the query?
if( queryResult.first().toInt() == 0 )
{
QString insert = QString( "INSERT INTO lyrics( url, lyrics ) VALUES ( '%1', '%2' );" )
.arg( m_collection->sqlStorage()->escape( m_rpath ),
QString insert = QString( "INSERT INTO lyrics( url, lyrics ) VALUES ( %1, '%2' )" )
.arg( QString::number( m_urlId ),
m_collection->sqlStorage()->escape( lyrics ) );
m_collection->sqlStorage()->insert( insert, "lyrics" );
}
else
{
QString update = QString( "UPDATE lyrics SET lyrics = '%1' WHERE url = '%2';" )
QString update = QString( "UPDATE lyrics SET lyrics = '%1' WHERE url = %2" )
.arg( m_collection->sqlStorage()->escape( lyrics ),
m_collection->sqlStorage()->escape( m_rpath ) );
QString::number( m_urlId ) );
m_collection->sqlStorage()->query( update );
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment