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

[Daniel Kristjansson]

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.
> >   
> 
> Daniel,
> 
> I had a couple of questions about the changes in here.  The first one 
> was regarding your change to DBUtil::GetTables() to remove the check for 
> whether SHOW FULL TABLES is supported.  Are we going to also modify the 
> minimum MySQL version to 5.0.2 (or, a more appropriate later 
> version--like the first stable release of MySQL 5; I think Cardoe told 
> me that was 5.0.15)?  I'll do a patch, but I was just wondering if it 
> would be desired (and what minimum MySQL version seems most appropriate).

Hmm, I though we were already requiring 5.0.2, if not I'll revert that
part of the fix. I was just trying to avoid this needing to be an
instance method rather that a static function.

> 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.

-- Daniel