[mythtv] Patch for mythfilldatabase --no-delete

Andrew M. Bishop amb at gedanken.demon.co.uk
Fri Mar 4 19:28:21 UTC 2005


Oscar Carlsson <webmaster at trekotor.se> writes:

> Andrew M. Bishop wrote, On 2005-03-02 19:52:
> >Oscar Carlsson <webmaster at trekotor.se> writes:
> >>But, anyway, if you look at the changes I made you can see (as far as I
> >>can remember) that the whole day _was_ indeed cleared in either case
> >>before entering that if-clause.

> > I can tell you from experience that the whole day is not cleared.  If
> > it was then I would not have had any SQL errors when performing the
> > insert.  If the whole day is cleared then there is no need to check if
> > a program already exists in the database, obviously it wouldn't.
> 
> > When I run mythfilldatabase --no-delete from version 0.17 I got
> > several thousand SQL insert errors due to duplicate data.  With my
> > one-line patch to reverse the 'if' statement there were no errors.
> > There can only be insert errors due to duplicate data if the whole day
> > was not cleared.
> 
> What I meant to say was that it _used_ to delete the whole day in 0.16
> (or at least CVS before my changes) before even considering deleting
> program by program.

I don't know about the CVS version that you edited, but version 0.16
and version 0.17 do not delete the whole delay before adding new
programs.  The code that prevents it happening is identical in both.

-------------------- filldata.cpp --------------------
void clearDBAtOffset(int offset, int chanid, QDate *qCurrentDate)
{
    if (no_delete)
        return;

...

}
-------------------- filldata.cpp --------------------

But wait...

When I look closer I can see that in version 0.17 this function is not
called at all!  It was called from the handlePrograms() function but
is now not used.  This means that for anybody using XMLTV and not
datadirect there is no deletion of existing programs at all.

The datadirect grabber calls the clearDataBySource() function which
doesn't take account of the --no-delete option at all.  This is a
different bug with the --no-delete option, it wouldn't work for
datadirect users.  (This code wasn't there when I added the original
--no-delete patch).

Which grabber do you use, I would guess that it is XMLTV (from *.se)
and not datadirect.  This is probably why you changed the 'if'
statement in question.  If the whole day was not being deleted then
the change that you made would have stopped you getting SQL errors.
The problem is that it cause me (using --no-delete) to get them
instead.  The real problem is the person that deleted the
clearDBAtOffset() function call from within the handlePrograms()
function.

When you fix the code not to delete the whole day (for all users, not
just those using the --no-delete option) you need to:

* Delete the clearDBAtOffset() function (not used now anyway).
* Delete the clearDataBySource() function (not needed if not deleting
  the whole day)
* Delete the clearDataByChannel() function (they both call this,
  nobody else does)
* Move the code in the 'if (no_delete)' clause (which you changed to
  'if(!no_delete)') so that it is run irrespective of the value of this
  option.

The only remaining use of the 'no_delete' variable is then in the
clearOldDBEntries() function where it is used correctly.

> But, as Hamish says, the help text for this switch clearly says that
> existing programs should _not_ be deleted when updating, wich obviously
> isn't what you intended from the beginning.

Yes.  The answer to Hamish, who has missed the first few messages in
this discussion, is that the help text is not totally accurate.  A
more accurate version is:

--no-delete
    Do not delete old programs from the database until 7 days old.
    Delete programs only if conflicting not the whole day when updating.

> I'll fix this in the next couple of days by making the "delete by
> program" also run when using --no-delete, thus making --no-delete only
> keep old programs in the db, nothing more. Might rename it as well, will
> see.

Is there a good reason to delete the data from the last seven days
rather than keeping it?  Surely people can spare the disk space and
the benefit of going back to see what you missed yesterday would say
that it should be kept.

Please don't rename it, doing that will break the scripts of people
that use the option.

-- 
Andrew.
----------------------------------------------------------------------
Andrew M. Bishop                             amb at gedanken.demon.co.uk
                                      http://www.gedanken.demon.co.uk/


More information about the mythtv-dev mailing list