mtdean at thirdcontact
May 3, 2008, 12:40 PM
Post #7 of 8
(1363 views)
Permalink
|
|
Re: [mythtv-commits] mythtv commit: r17160 - in trunk/mythtv/libs by danielk
[In reply to]
|
|
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 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
upgrading"?
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).
Mike
_______________________________________________
mythtv-dev mailing list
mythtv-dev [at] mythtv
http://mythtv.org/cgi-bin/mailman/listinfo/mythtv-dev
|
