[mythtv] [mythtv-commits] mythtv commit: r17160 - in trunk/mythtv/libs by danielk

Michael T. Dean mtdean at thirdcontact.com
Sat May 3 19:40:14 UTC 2008

On 04/29/2008 09:48 AM, Daniel Kristjansson wrote:
> On Mon, 2008-04-28 at 20:32 -0400, Michael T. Dean wrote:
>> On 04/28/2008 03:00 PM, mythtv at cvs.mythtv.org wrote:
>>> Refs #5267. Fixes two chicken and egg problems with DB initialization
>>> The second is not to do the database backup on an empty database, since it will fail to find settings and housekeeping tables.
>> Also, your change to not do the backup on an "empty" database (where 
>> dbver != "0") sounds good as long as dbVer doesn't get set to 0 if the 
>> settings table is missing or the DBSchemaVer setting, itself, happens to 
>> be gone from the DB (for the same reason Chris Pinkham reverted his 
>> change at http://svn.mythtv.org/trac/changeset/15933 , i.e. 
>> http://www.gossamer-threads.com/lists/mythtv/dev/315935#315935 ).
> I'll look into this.
>> And, finally, this adds a check for dbver != "0" before calling 
>> PromptForSchemaUpgrade(), but PromptForSchemaUpgrade() is also doing a 
>> check at libs/libmyth/mythcontext.cpp +3529 ( 
>> http://cvs.mythtv.org/trac/browser/trunk/mythtv/libs/libmyth/mythcontext.cpp#L3529 
>> ).  Since the "old" check--an isEmpty() check--is failing and (I'm 
>> assuming) yours is working, it seems that someone (I don't see where) is 
>> setting dbver to 0 before the call to PromptForSchemaUpgrade() (perhaps 
>> this changed since Nigel(?) first wrote it).  Does it make sense to 
>> remove the new check and fix the one in PromptForSchemaUpgrade()?
> It's probably best to check both. We could also modify the "empty
> database check" to check for the absence of tables. This is what I
> did in the datadirect portion of the patch.

So, I did some experimentation and figured out what's going on.  
Although the default value for MythContext::GetSetting() is "", the 
default value for MythContext::GetNumSetting() is 0.  And, when you 
factor in the settings cache, it turns out that whatever function is 
called first wins--so, if gContext.GetNumSetting("DBSchemaVer") is 
called first, all calls to gContext.GetSetting("DBSchemaVer") will return 0.

Therefore, since the addition of CompareTVDatabaseSchemaVersion() in 
[15902], the schema version checks for "" have all been broken.  (Yeah, 
that means I broke it. :( ).

So, how best to clean up the mess?  Change 
CompareTVDatabaseSchemaVersion() to use GetSetting() and toInt() (so 
we'll go back to defaulting to "")?  Change all the 'if (dbver = "")' 
checks to 'if (dbver.isEmpty() || dbver == "0")'?  Or, modify the 
settings cache to identify "missing"/not-stored settings (i.e. with 
QString::NULL and a check for it in GetSetting(), perhaps) so it doesn't 
save a specific call's requested default?  Thoughts?

Also, this means that any corrupted DB that's missing a DBSchemaVer 
setting (or the settings table itself) will no longer be backed up with 
the change to check for dbVer == "0" before doing the backup.  The 
similar check before calling MythContext::PromptForSchemaUpgrade() 
prevents asking for permission to upgrade for both empty databases /and/ 
for corrupt ones without a DBSchemaVer.  Since 
MythContext::PromptForSchemaUpgrade() is now doing an isEmpty()/=="0" 
check), the prompt wouldn't be displayed even without the new check.

So, if I were to modify DBUtil::BackupDB() to check for an empty 
database (i.e. the size of the QStringList returned by 
DBUtil::GetTables() is 0), can I also remove the new checks for 
undefined DBSchemaVer that prevent the backup?  And, since the new check 
that prevents the prompt is redundant, can I remove it so that the user 
will get the VB_GENERAL message, ""No current database version. Auto 

Note, also, that as it stands now, a missing DBSchemaVer in an database 
with tables actually allows Myth to run with the wrong schema version 
(i.e. the upgrade won't work, but the wrong checks are performed, so it 
doesn't error out).


More information about the mythtv-dev mailing list