extragear/multimedia/amarok/src rev 570617

[Jeff Mitchell]

Hmm...

I didn't make this change myself because I'm not sure it's entirely correct.  
This function will run if isValid() is not true.  isValid checks for any url 
in tags, statistics, podcastchannels and podcastepisodes, and any id from 
devices...if no value exists for any of those, it assumes the tables don't 
exist.

However, could those tables exist, but for some reason be empty (nothing 
played yet so nothing in stats, no podcasts, no folders selected so tags 
empty)...I suppose there should always be a device, but is that true?

Anyways, what I'm getting at is that this could maybe make us skip the upgrade 
code when it is needed, if a person with an older revision has empty tables 
for whatever reason.  The safer thing in this sense would be to drop the 
tables and then recreate them.

Of course, that behavior led to the infamous "suddenly all my stats are gone 
and I have MySQL!" bug...except the fix for that really was in the MySQL 
initialization code, not here.  See the log message and diff for revision 
559197.  In it I remarked that it didn't make sense to drop tables that 
seemed to be empty and then recreate them, leaving them empty; however if I 
am right about this and we can end up getting into a state where the tables 
skip the upgrade because they're empty, and we then write the new rev 
numbers, could we end up with worse behavior?

I guess I'm suggesting that we do in fact drop the tables in this case (which 
shouldn't cause the stats bug again), and then recreate them (and then set 
the correct version numbers).  Or maybe do that for everything but stats, but 
always run the stats upgrade code?

--Jeff

On Monday 07 August 2006 08:47, Andrew Turner wrote:
> SVN commit 570617 by aturner:
>
> After creating tables, set the version numbers. This prevents the
> refactored DB code from wanting to update the tables. (This was due to the
> DB update code being moved into the checkDatabase function, whereas the old
> code was in the initialize function and would not run if the tables were
> being created for the first time.)
>
> Well done to jefferai for spotting this.
>
>
>  M  +16 -1     collectiondb.cpp
>
>
> --- trunk/extragear/multimedia/amarok/src/collectiondb.cpp #570616:570617
> @@ -4987,13 +4987,28 @@
>      }
>      else if ( !isValid() )
>      {
> -        warning() << "Tables seem to be empty; maybe they don't exist at
> all." << endl; +        //No tables seem to exist (as doing a count(url)
> didn't even return any number, even 0). +        warning() << "Tables seem
> to not exist." << endl;
>          warning() << "Attempting to create tables (this should be safe;
> ignore any errors)..." << endl; createTables(false);
>          createPersistentTables();
>          createPodcastTables();
>          createStatsTable();
>          warning() << "Tables should now definitely exist. (Stop ignoring
> errors)" << endl; +
> +        //Since we have created the tables, we need to make sure the
> version numbers are +        //set to the correct values. If this is not
> done now, the database update code may +        //run, which could corrupt
> things.
> +        amaroK::config( "Collection Browser" )->writeEntry( "Database
> Version", DATABASE_VERSION ); +        amaroK::config( "Collection Browser"
> )->writeEntry( "Database Stats Version", DATABASE_STATS_VERSION ); +       
> amaroK::config( "Collection Browser" )->writeEntry( "Database Persistent
> Tables Version", DATABASE_PERSISTENT_TABLES_VERSION ); +       
> amaroK::config( "Collection Browser" )->writeEntry( "Database Podcast
> Tables Version", DATABASE_PODCAST_TABLES_VERSION );    amaroK::config(
> "Collection Browser" )->writeEntry( "Database ATF Version",
> DATABASE_ATF_VERSION ); +
> +        setAdminValue( "Database Version", QString::number(
> DATABASE_VERSION ) ); +        setAdminValue( "Database Stats Version",
> QString::number( DATABASE_STATS_VERSION ) ); +        setAdminValue(
> "Database Persistent Tables Version", QString::number(
> DATABASE_PERSISTENT_TABLES_VERSION ) ); +        setAdminValue( "Database
> Podcast Tables Version", QString::number( DATABASE_PODCAST_TABLES_VERSION )
> ); +        setAdminValue( "Database ATF Version", QString::number(
> DATABASE_ATF_VERSION ) ); }
>      else {
>          //updates for the Devices table go here