Opened 12 years ago

Closed 11 years ago

#5286 closed patch (fixed)

Fix initial database creation

Reported by: sphery <mtdean@…> Owned by: danielk
Priority: minor Milestone: unknown
Component: mythtv Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

In current trunk, initial database creation is broken. This ticket will contain patches to fix the issues.

The root of the problem was the addition of CompareTVDatabaseSchemaVersion() in [15902]. Its addition caused the default value used for a nonexistent DBSchemaVer setting to change, rendering invalid many of the checks for a "new database."

The settings cache remembers the default value requested by the first function to call Get*Setting() for a particular setting if the value does not exist in the cache or in the database.

CompareTVDatabaseSchemaVersion() uses GetNumSetting?(), whereas previously the first function to request the DBSchemaVer used GetSetting?(). Both simply used the default default value ("" for GetSetting?() and 0 for GetNumSetting?()), so the default value for a non-existent DBSchemaVer has changed from "" to 0/"0".

Attachments (7)

mythtv-5286-no_backup_for_new_database.patch (1.8 KB) - added by sphery <mtdean@…> 12 years ago.
Adds DBUtil::IsNewDatabase?() and moves check for whether to do a backup to DBUtil::DoBackup?()
mythtv-5286-allow_PromptForSchemaUpgrade_to_check_for_empty_DB.patch (1.0 KB) - added by sphery <mtdean@…> 12 years ago.
Remove redundant "new database" check before call to PromptForSchemaUpgrade?()
mythtv-5286-check_for_empty_or_zero_to_call_InitializeDatabase.patch (381 bytes) - added by sphery <mtdean@…> 12 years ago.
Check for zero or empty to call InitializeDatabase?
mythtv-5286-use_IsNewDatabase_everywhere.patch (1.0 KB) - added by sphery <mtdean@…> 12 years ago.
replaces "new database" DBSchemaVer checks with DBUtil::IsNewDatabase?()
mythtv-5286-fail_upgrade_on_unrecognized_DBSchemaVer.patch (546 bytes) - added by sphery <mtdean@…> 12 years ago.
Fail upgrade on unrecognized DBSchemaVer
mythtv-5286-no_backup_for_new_database-20080615.patch (1.9 KB) - added by sphery <mtdean@…> 11 years ago.
Updates mythtv-5286-no_backup_for_new_database.patch for changes in [17479]
mythtv-5286-use_IsNewDatabase_everywhere-20080703.patch (1.0 KB) - added by sphery <mtdean@…> 11 years ago.
de-fuzzed patch after recent library changes

Download all attachments as: .zip

Change History (15)

Changed 12 years ago by sphery <mtdean@…>

Adds DBUtil::IsNewDatabase?() and moves check for whether to do a backup to DBUtil::DoBackup?()

comment:1 Changed 12 years ago by sphery <mtdean@…>

mythtv-5286-no_backup_for_new_database.patch adds a new function, DBUtil::IsNewDatabase?() and moves the check for whether to do a backup to DBUtil::DoBackup?() . The current implementation of IsNewBackup?() is not designed to be called frequently, but it would be possible to make it fast by performing a full test only on the first call and remembering the result, then resetting the result (to false) if InitializeDatabase?() succeeds. This approach may be more appropriate than the upcoming patch to change all 'if (dbver == "")' checks to 'if (dbver.isEmpty()
dbver == "0")' checks.

Changed 12 years ago by sphery <mtdean@…>

Remove redundant "new database" check before call to PromptForSchemaUpgrade?()

comment:2 Changed 12 years ago by sphery <mtdean@…>

mythtv-5286-allow_PromptForSchemaUpgrade_to_check_for_empty_DB.patch removes the check for 'if (dbver == "0") that prevents the call to PromptForSchemaUpgrade?(). Since PromptForSchemaUpgrade?() is doing a check, 'if (dbver.isEmpty()
dbver == "0")', the check before the call is unnecessary, and calling PromptForSchemaUpgrade?() causes the VB_GENERAL message, "No current database version. Auto upgrading", to be written.

Changed 12 years ago by sphery <mtdean@…>

Check for zero or empty to call InitializeDatabase?

comment:3 Changed 12 years ago by sphery <mtdean@…>

After removal of the "empty database" check before calling DoBackup?() and the one before calling PromptForSchemaUpgrade?(), the only remaining checks I could find were in PromptForSchemaUpgrade?() (which Nigel recently changed to check for empty or zero) and the one before the call to InitializeDatabase?(). mythtv-5286-check_for_empty_or_zero_to_call_InitializeDatabase.patch fixes the check before InitializeDatabase?().

Changed 12 years ago by sphery <mtdean@…>

replaces "new database" DBSchemaVer checks with DBUtil::IsNewDatabase?()

comment:4 Changed 12 years ago by sphery <mtdean@…>

mythtv-5286-use_IsNewDatabase_everywhere.patch can be used instead of mythtv-5286-check_for_empty_or_zero_to_call_InitializeDatabase.patch . It replaces the DBSchemaVer checks that are being used as a simple "new database" check in doUpgradeTVDatabaseSchema() and MythContext::PromptForSchemaUpgrade?() with calls to DBUtil::IsNewDatabase?(). This approach is more robust than simply checking DBSchemaVer since the setting could be missing in a corrupt database (i.e. user accidentally deleted the setting or the settings table or the settings table is crashed).

However, if used, the next patch (which prevents an unknown DBSchemaVer from successfully completing doUpgradeTVDatabaseSchema()) becomes critical.

Performance is not an issue (though I'm still happy to modify IsNewDatabase?()) since IsNewDatabase?() is only called 3 times and only if a schema upgrade is occurring.

Changed 12 years ago by sphery <mtdean@…>

Fail upgrade on unrecognized DBSchemaVer

comment:5 Changed 12 years ago by sphery <mtdean@…>

mythtv-5286-fail_upgrade_on_unrecognized_DBSchemaVer.patch checks for an unrecognized DBSchemaVer (i.e. after InitializeDatabase?, it's still empty or the DBSchemaVer is less than the oldest version from which we can do an upgrade--currently 1027). The check for empty is mainly useful if mythtv-5286-use_IsNewDatabase_everywhere.patch is applied rather than mythtv-5286-check_for_empty_or_zero_to_call_InitializeDatabase.patch .

Note that this patch changes the historic behavior of Myth. Previously, if we did not recognize the schema version, doUpgradeTVDatabaseSchema() would run to completion, but the 'dbver == "..."' checks would not match, so it would do nothing and think the upgrade had succeeded. This patch changes the behavior to prevent a user with an extremely old database version (or a corrupt DB with no DBSchemaVer) from running Myth since we can't guarantee compatibility.

comment:6 Changed 12 years ago by danielk

Owner: changed from Isaac Richards to danielk
Status: newaccepted

Changed 11 years ago by sphery <mtdean@…>

Updates mythtv-5286-no_backup_for_new_database.patch for changes in [17479]

Changed 11 years ago by sphery <mtdean@…>

de-fuzzed patch after recent library changes

comment:7 Changed 11 years ago by sphery <mtdean@…>

mythtv-5286-use_IsNewDatabase_everywhere-20080703.patch updates/replaces mythtv-5286-use_IsNewDatabase_everywhere.patch to remove fuzz introduced by library changes.

Quick overview for the committer: I recommend applying all current patches except mythtv-5286-check_for_empty_or_zero_to_call_InitializeDatabase.patch, specifically:

  • mythtv-5286-no_backup_for_new_database-20080615.patch
  • mythtv-5286-allow_PromptForSchemaUpgrade_to_check_for_empty_DB.patch
  • mythtv-5286-use_IsNewDatabase_everywhere-20080703.patch
  • mythtv-5286-fail_upgrade_on_unrecognized_DBSchemaVer.patch

descriptions/rationale in above comments.

Note that mythtv-5286-use_IsNewDatabase_everywhere-20080703.patch depends on mythtv-5286-no_backup_for_new_database-20080615.patch .

comment:8 Changed 11 years ago by danielk

Resolution: fixed
Status: acceptedclosed

(In [17820]) Fixes #5286. Fix initial DB creation in trunk by applying sphery's patches + adding a few QString->cstring conversions.

Note: See TracTickets for help on using tickets.