Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JSON]Fix filtering in GetSongs and GetAlbums #14201

Merged
merged 1 commit into from
Jul 20, 2018

Conversation

DaveTBlake
Copy link
Member

@DaveTBlake DaveTBlake commented Jul 19, 2018

Fix path filtering in AudioLibrary::GetSongs and filtering by playcount, lastplayed or dateadded in AudioLibrary::GetAlbums after changes of #14160.

JSON API version bump patch as only internal processing change.

Can be tested with JSON commands:

{"jsonrpc": "2.0", "id": 1, "method": "AudioLibrary.GetAlbums", 
"params": { "filter": {"operator": "is", "field": "playcount", "value": "1"},
"properties": [ "lastplayed"] }}
{"jsonrpc":"2.0","id":1,"method":"audioLibrary.GetSongs",
"params":{"limits": {"start": 0, "end": 600},
"filter": {"operator": "contains", "field": "path", "value": "bruckner"},
"properties":["displayartist", "file"]}}

@DaveTBlake DaveTBlake added this to the Leia 18.0-alpha3 milestone Jul 19, 2018
// Count number of songs that satisfy selection criteria
// (includes xsp limits from filter, but not sort limits)
// Use songview as filter rules in where clause may use album and path JOIN fields
total = (int)strtol(GetSingleValue("SELECT COUNT(1) FROM songview " + strSQLExtra, m_pDS).c_str(), NULL, 10);

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@DaveTBlake
Copy link
Member Author

Win10 test build available here @zag2me or @rmrector if you would care to test it
http://mirrors.kodi.tv/test-builds/windows/win64/KodiSetup-20180719-5293ee88-PR14201-merge-x64.exe

@zag2me
Copy link
Contributor

zag2me commented Jul 19, 2018

Tested on windows and confirmed this fixes the SQL error I found while using the Artwork beef Add-on.

Thanks!

@MilhouseVH
Copy link
Contributor

MilhouseVH commented Jul 19, 2018

@DaveTBlake there's an issue with the first GetAlbums query and MySQL (btw it should be playcount not playedcount):

curl -s --data-binary '{"jsonrpc": "2.0", "id": 1, "method": "AudioLibrary.GetAlbums", "params": { "filter": {"operator": "is", "field": "playcount", "value": "1"}, "properties": [ "lastplayed"] }}' -H 'content-type: application/json;' http://localhost:8080/jsonrpc

results in

{"error":{"code":-32603,"message":"Internal error."},"id":1,"jsonrpc":"2.0"}

and in kodi.log:

09:57:51.509 T:1718379376   DEBUG: CWebServer[8080]: request received for /jsonrpc
09:57:51.580 T:1718379376   DEBUG: GetAlbumsByWhereJSON query: SELECT album.idAlbum, strAlbum, (SELECT ROUND(AVG(song.iTimesPlayed)) FROM song WHERE song.idAlbum = album.idAlbum) AS iTimesPlayed, (SELECT MAX(song.lastplayed) FROM song WHERE song.idAlbum = album.idAlbum) AS lastplayed FROM album  WHERE (((CAST(iTimesPlayed as DECIMAL(5,1)) = 1))) AND (album.strReleaseType = 'album') ORDER BY album.idAlbum
09:57:51.588 T:1718379376   ERROR: SQL: [MyMusic72] Undefined MySQL error: Code (1054)
                                            Query: SELECT album.idAlbum, strAlbum, (SELECT ROUND(AVG(song.iTimesPlayed)) FROM song WHERE song.idAlbum = album.idAlbum) AS iTimesPlayed, (SELECT MAX(song.lastplayed) FROM song WHERE song.idAlbum = album.idAlbum) AS lastplayed FROM album  WHERE (((CAST(iTimesPlayed as DECIMAL(5,1)) = 1))) AND (album.strReleaseType = 'album') ORDER BY album.idAlbum
09:57:51.589 T:1718379376   ERROR: GetAlbumsByWhereJSON failed

The query is:

SELECT album.idAlbum, strAlbum, 
       (SELECT ROUND(AVG(song.iTimesPlayed))
        FROM song
        WHERE song.idAlbum = album.idAlbum) AS iTimesPlayed, 
       (SELECT MAX(song.lastplayed)
        FROM song
        WHERE song.idAlbum = album.idAlbum) AS lastplayed
FROM album
WHERE (((CAST(iTimesPlayed as DECIMAL(5,1)) = 1))) AND (album.strReleaseType = 'album')
ORDER BY album.idAlbum

and when running this query in MySQL Workbench, the error is:

Error Code: 1054. Unknown column 'iTimesPlayed' in 'where clause'

Which is nasty.

The following query works, but I'm not suggesting it's a good solution...

SELECT * FROM
(
  SELECT album.idAlbum, strAlbum,
         (SELECT ROUND(AVG(song.iTimesPlayed))
          FROM song
          WHERE song.idAlbum = album.idAlbum) AS iTimesPlayed, 
         (SELECT MAX(song.lastplayed)
          FROM song
          WHERE song.idAlbum = album.idAlbum) AS lastplayed
  FROM album
  WHERE album.strReleaseType = 'album'
) as album
WHERE (((CAST(iTimesPlayed as DECIMAL(5,1)) = 1)))
ORDER BY album.idAlbum

@DaveTBlake
Copy link
Member Author

Thanks for testing @MilhouseVH, darn substandard MySQL strikes again!
Anyway I have adjusted GetAlbums to handle it, only "albums" has scalar subquery fields as possible filter criteria. Could you test GetAlbums again please, in case I have slipped again.

@MilhouseVH
Copy link
Contributor

I've tested the two queries in this PR again, and the MySQL issue is resolved. I've also repeated the testing from the previous PR, and those queries continue to return the expected results. Many thanks @DaveTBlake, this looks good to me.

@DaveTBlake DaveTBlake merged commit cd04cf2 into xbmc:master Jul 20, 2018
@DaveTBlake DaveTBlake deleted the JSONGetSongsPathFilter branch July 20, 2018 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants