[videodb] some tvshow linked tables (e.g. seasons) weren't cleaned #5006

Merged
merged 1 commit into from Jul 12, 2014

Conversation

Projects
None yet
5 participants
@jmarshallnz
Member

jmarshallnz commented Jul 11, 2014

Whenever the tvshow path was gone, we cleaned out the tvshowlinkpath, then removed the tvshow. But the cleaning of tables linked to a show was only performed if a show is removed due to having no episodes linked to it.

Moves the cleaning of the tables that link to shows out of that if clause.

Jonathan Marshall
[videodb] some tvshow linked tables (e.g. seasons) weren't cleaned in…
… some cases where a tvshow is removed from disk and clean library run
@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Jul 11, 2014

Member

@Montellese there's some argument for splitting the cleaning routine into stuff that relies on paths and stuff that can be run at anytime to cleanup mistakes (which could then be run on dbupdate).

Member

jmarshallnz commented Jul 11, 2014

@Montellese there's some argument for splitting the cleaning routine into stuff that relies on paths and stuff that can be run at anytime to cleanup mistakes (which could then be run on dbupdate).

@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Jul 11, 2014

Member

jenkins build this please

Member

jmarshallnz commented Jul 11, 2014

jenkins build this please

@MilhouseVH

This comment has been minimized.

Show comment
Hide comment
@MilhouseVH

MilhouseVH Jul 11, 2014

Contributor

Looks good here - it clobbered the seasons, which took out the artwork. NIce.

Contributor

MilhouseVH commented Jul 11, 2014

Looks good here - it clobbered the seasons, which took out the artwork. NIce.

@MilhouseVH

This comment has been minimized.

Show comment
Hide comment
@MilhouseVH

MilhouseVH Jul 11, 2014

Contributor

Ah, whoops - just noticed this in the debug:

22:13:21 240.063629 T:3058840096   DEBUG: CleanDatabase: Cleaning path table
22:13:21 240.064224 T:3058840096   DEBUG: Mysql execute: DELETE FROM path WHERE (strContent IS NULL OR strContent = '') AND (strSettings IS NULL OR strSettings = '') AND (strHash IS NULL OR strHash = '') AND (exclude IS NULL OR exclude != 1) AND (idParentPath IS NULL OR NOT EXISTS (SELECT 1 FROM path AS parentPath WHERE parentPath.idPath = path.idParentPath)) AND NOT EXISTS (SELECT 1 FROM files WHERE files.idPath = path.idPath) AND NOT EXISTS (SELECT 1 FROM tvshowlinkpath WHERE tvshowlinkpath.idPath = path.idPath) AND NOT EXISTS (SELECT 1 FROM movie WHERE movie.c23 = path.idPath) AND NOT EXISTS (SELECT 1 FROM episode WHERE episode.c19 = path.idPath) AND NOT EXISTS (SELECT 1 FROM musicvideo WHERE musicvideo.c14 = path.idPath)
22:13:21 240.065735 T:3058840096   ERROR: SQL: Undefined MySQL error: Code (1093)
                                            Query: DELETE FROM path WHERE (strContent IS NULL OR strContent = '') AND (strSettings IS NULL OR strSettings = '') AND (strHash IS NULL OR strHash = '') AND (exclude IS NULL OR exclude != 1) AND (idParentPath IS NULL OR NOT EXISTS (SELECT 1 FROM path AS parentPath WHERE parentPath.idPath = path.idParentPath)) AND NOT EXISTS (SELECT 1 FROM files WHERE files.idPath = path.idPath) AND NOT EXISTS (SELECT 1 FROM tvshowlinkpath WHERE tvshowlinkpath.idPath = path.idPath) AND NOT EXISTS (SELECT 1 FROM movie WHERE movie.c23 = path.idPath) AND NOT EXISTS (SELECT 1 FROM episode WHERE episode.c19 = path.idPath) AND NOT EXISTS (SELECT 1 FROM musicvideo WHERE musicvideo.c14 = path.idPath)
22:13:21 240.088943 T:3058840096   ERROR: CleanDatabase failed
22:13:21 240.089935 T:3058840096   DEBUG: Mysql rollback transaction
Contributor

MilhouseVH commented Jul 11, 2014

Ah, whoops - just noticed this in the debug:

22:13:21 240.063629 T:3058840096   DEBUG: CleanDatabase: Cleaning path table
22:13:21 240.064224 T:3058840096   DEBUG: Mysql execute: DELETE FROM path WHERE (strContent IS NULL OR strContent = '') AND (strSettings IS NULL OR strSettings = '') AND (strHash IS NULL OR strHash = '') AND (exclude IS NULL OR exclude != 1) AND (idParentPath IS NULL OR NOT EXISTS (SELECT 1 FROM path AS parentPath WHERE parentPath.idPath = path.idParentPath)) AND NOT EXISTS (SELECT 1 FROM files WHERE files.idPath = path.idPath) AND NOT EXISTS (SELECT 1 FROM tvshowlinkpath WHERE tvshowlinkpath.idPath = path.idPath) AND NOT EXISTS (SELECT 1 FROM movie WHERE movie.c23 = path.idPath) AND NOT EXISTS (SELECT 1 FROM episode WHERE episode.c19 = path.idPath) AND NOT EXISTS (SELECT 1 FROM musicvideo WHERE musicvideo.c14 = path.idPath)
22:13:21 240.065735 T:3058840096   ERROR: SQL: Undefined MySQL error: Code (1093)
                                            Query: DELETE FROM path WHERE (strContent IS NULL OR strContent = '') AND (strSettings IS NULL OR strSettings = '') AND (strHash IS NULL OR strHash = '') AND (exclude IS NULL OR exclude != 1) AND (idParentPath IS NULL OR NOT EXISTS (SELECT 1 FROM path AS parentPath WHERE parentPath.idPath = path.idParentPath)) AND NOT EXISTS (SELECT 1 FROM files WHERE files.idPath = path.idPath) AND NOT EXISTS (SELECT 1 FROM tvshowlinkpath WHERE tvshowlinkpath.idPath = path.idPath) AND NOT EXISTS (SELECT 1 FROM movie WHERE movie.c23 = path.idPath) AND NOT EXISTS (SELECT 1 FROM episode WHERE episode.c19 = path.idPath) AND NOT EXISTS (SELECT 1 FROM musicvideo WHERE musicvideo.c14 = path.idPath)
22:13:21 240.088943 T:3058840096   ERROR: CleanDatabase failed
22:13:21 240.089935 T:3058840096   DEBUG: Mysql rollback transaction
@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Jul 11, 2014

Member

mysql is truly the suckiest SQL implementation in existence. FFS it's rubbish.

Member

jmarshallnz commented Jul 11, 2014

mysql is truly the suckiest SQL implementation in existence. FFS it's rubbish.

@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Jul 11, 2014

Member

@Montellese ETA on your UPnP stuff?

Member

jmarshallnz commented Jul 11, 2014

@Montellese ETA on your UPnP stuff?

@MilhouseVH

This comment has been minimized.

Show comment
Hide comment
@MilhouseVH

MilhouseVH Jul 11, 2014

Contributor

What's rather odd is that, despite rolling back the transaction due to the failure of the path deletion, it has deleted the rows from the seasons and art tables... maybe another MySQL oddity? If you want me to test an updated query before committing it, just paste the query as a comment and I'll confirm if it works or not!

Contributor

MilhouseVH commented Jul 11, 2014

What's rather odd is that, despite rolling back the transaction due to the failure of the path deletion, it has deleted the rows from the seasons and art tables... maybe another MySQL oddity? If you want me to test an updated query before committing it, just paste the query as a comment and I'll confirm if it works or not!

@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Jul 11, 2014

Member

My guess is MySQL completely ignores the transaction stuff.

Member

jmarshallnz commented Jul 11, 2014

My guess is MySQL completely ignores the transaction stuff.

@MilhouseVH

This comment has been minimized.

Show comment
Hide comment
@MilhouseVH

MilhouseVH Jul 11, 2014

Contributor

That certainly would explain it!

Contributor

MilhouseVH commented Jul 11, 2014

That certainly would explain it!

@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Jul 11, 2014

Member

And no, I won't be giving you an updated query - I don't have the time or energy to donate any further to fixing up mysql - it's a waste of the little time I have for XBMC. I'm happy to review changes ofc.

In case you have time to look at it, the problem is path being used in the subquery (for parentPath). There's various spots you can look at for inspiration, such as the changes to the seasons query at ln 4605.

Member

jmarshallnz commented Jul 11, 2014

And no, I won't be giving you an updated query - I don't have the time or energy to donate any further to fixing up mysql - it's a waste of the little time I have for XBMC. I'm happy to review changes ofc.

In case you have time to look at it, the problem is path being used in the subquery (for parentPath). There's various spots you can look at for inspiration, such as the changes to the seasons query at ln 4605.

@MilhouseVH

This comment has been minimized.

Show comment
Hide comment
@MilhouseVH

MilhouseVH Jul 11, 2014

Contributor

How about replacing the subquery with the following:

AND (idParentPath IS NULL OR NOT EXISTS (SELECT 1 FROM (SELECT idPath FROM path) as parentPath WHERE parentPath.idPath = path.idParentPath))

MySQL seems happy with it, and I think it does what the existing code does (though perhaps slightly less efficiently).

Contributor

MilhouseVH commented Jul 11, 2014

How about replacing the subquery with the following:

AND (idParentPath IS NULL OR NOT EXISTS (SELECT 1 FROM (SELECT idPath FROM path) as parentPath WHERE parentPath.idPath = path.idParentPath))

MySQL seems happy with it, and I think it does what the existing code does (though perhaps slightly less efficiently).

@jmarshallnz

This comment has been minimized.

Show comment
Hide comment
@jmarshallnz

jmarshallnz Jul 11, 2014

Member

Looks reasonable to me - let's see what @Montellese says :)

Member

jmarshallnz commented Jul 11, 2014

Looks reasonable to me - let's see what @Montellese says :)

@mkortstiege

This comment has been minimized.

Show comment
Hide comment
@mkortstiege

mkortstiege Jul 11, 2014

Member

@Montellese UPnP shared lib please. Time to let mysql and all these cumbersome hack workarounds (like path substitutions) rot in hell ;) So yea, I am interested in the current status as well.

Member

mkortstiege commented Jul 11, 2014

@Montellese UPnP shared lib please. Time to let mysql and all these cumbersome hack workarounds (like path substitutions) rot in hell ;) So yea, I am interested in the current status as well.

@MilhouseVH

This comment has been minimized.

Show comment
Hide comment
@MilhouseVH

MilhouseVH Jul 12, 2014

Contributor

I've created #5007 for the path issue.

Contributor

MilhouseVH commented Jul 12, 2014

I've created #5007 for the path issue.

@Montellese

This comment has been minimized.

Show comment
Hide comment
@Montellese

Montellese Jul 12, 2014

Member

This change here looks sane.

I don't have any ETA on the media import stuff since I haven't had the time to work on it for a few months now :-( Furthermore there are quite a few fundamental issues that need solving before I can continue working on it.

Member

Montellese commented Jul 12, 2014

This change here looks sane.

I don't have any ETA on the media import stuff since I haven't had the time to work on it for a few months now :-( Furthermore there are quite a few fundamental issues that need solving before I can continue working on it.

jmarshallnz added a commit that referenced this pull request Jul 12, 2014

Merge pull request #5006 from jmarshallnz/season_clean
[videodb] some tvshow linked tables (e.g. seasons) weren't cleaned

@jmarshallnz jmarshallnz merged commit f2d482c into xbmc:master Jul 12, 2014

1 check passed

default Merged build #1012 succeeded in 58 min
Details

@jmarshallnz jmarshallnz deleted the jmarshallnz:season_clean branch Jul 12, 2014

@MartijnKaijser MartijnKaijser added this to the Helix 14.0-alpha1 milestone Jul 13, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment