Merge pull request #10974 from DaveTBlake/ReworkGetFilter

Fix GetFilter SQL for music artists, albums and songs nodes

3 files changed, 299 insertions, 153 deletions

diff --git a/xbmc/dbwrappers/Database.cpp b/xbmc/dbwrappers/Database.cpp
index 7d9abc0356..bd7974aeba 100644
--- a/xbmc/dbwrappers/Database.cpp
+++ b/xbmc/dbwrappers/Database.cpp
@@ -101,6 +101,54 @@ void CDatabase::Filter::AppendGroup(const std::string &strGroup)
     group += ", " + strGroup;
 }
 
+void CDatabase::ExistsSubQuery::AppendJoin(const std::string &strJoin)
+{
+  if (strJoin.empty())
+    return;
+
+  if (join.empty())
+    join = strJoin;
+  else
+    join += " " + strJoin;
+}
+
+void CDatabase::ExistsSubQuery::AppendWhere(const std::string &strWhere, bool combineWithAnd /* = true */)
+{
+  if (strWhere.empty())
+    return;
+
+  if (where.empty())
+    where = strWhere;
+  else
+  {
+    where += combineWithAnd ? " AND " : " OR ";
+    where += strWhere;
+  }
+}
+
+bool CDatabase::ExistsSubQuery::BuildSQL(std::string & strSQL)
+{
+  if (tablename.empty())
+    return false;
+  strSQL = "EXISTS (SELECT 1 FROM " + tablename;
+  if (!join.empty())
+    strSQL += " " + join;
+  std::string strWhere;
+  if (!param.empty())
+    strWhere = param;
+  if (!where.empty())
+  {
+    if (!strWhere.empty())
+      strWhere += " AND ";
+    strWhere += where;
+  }
+  if (!strWhere.empty())      
+    strSQL += " WHERE " + strWhere;
+
+  strSQL += ")";
+  return true;
+}
+
 CDatabase::CDatabase(void)
 {
   m_openCount = 0;
diff --git a/xbmc/dbwrappers/Database.h b/xbmc/dbwrappers/Database.h
index aa63111e8a..593a8eac47 100644
--- a/xbmc/dbwrappers/Database.h
+++ b/xbmc/dbwrappers/Database.h
@@ -57,6 +57,22 @@ public:
     std::string limit;
   };
 
+  class ExistsSubQuery
+  {
+  public:
+    ExistsSubQuery(const std::string &table) : tablename(table) {};
+    ExistsSubQuery(const std::string &table, const std::string &parameter) : tablename(table), param(parameter) {};
+    void AppendJoin(const std::string &strJoin);
+    void AppendWhere(const std::string &strWhere, bool combineWithAnd = true);
+    bool BuildSQL(std::string &strSQL);
+    
+    std::string tablename;
+    std::string param;
+    std::string join;
+    std::string where;
+  };
+
+
   CDatabase(void);
   virtual ~CDatabase(void);
   bool IsOpen();
diff --git a/xbmc/music/MusicDatabase.cpp b/xbmc/music/MusicDatabase.cpp
index e85ae40ebe..9f3239515c 100644
--- a/xbmc/music/MusicDatabase.cpp
+++ b/xbmc/music/MusicDatabase.cpp
@@ -6320,13 +6320,13 @@ bool CMusicDatabase::GetFilter(CDbUrl &musicUrl, Filter &filter, SortDescription
   int idRole = 1; // Default restrict song_artist to "artists" only, no other roles.
   option = options.find("roleid");
   if (option != options.end())
-    idRole = (int)option->second.asInteger();
+    idRole = static_cast<int>(option->second.asInteger());
   else
   {
     option = options.find("role");
     if (option != options.end())
     {
-      if (option->second.asString() == "all")
+      if (option->second.asString() == "all" || option->second.asString() == "%")
         idRole = -1000; //All roles
       else
         idRole = GetRoleByName(option->second.asString());
@@ -6342,104 +6342,130 @@ bool CMusicDatabase::GetFilter(CDbUrl &musicUrl, Filter &filter, SortDescription
   std::string strRoleSQL; //Role < 0 means all roles, otherwise filter by role
   if(idRole > 0) strRoleSQL = PrepareSQL(" AND song_artist.idRole = %i ", idRole);  
 
-  if (type == "artists")
-  {
-    int idArtist = -1, idGenre = -1, idAlbum = -1, idSong = -1;
-    bool albumArtistsOnly = false;
+  int idArtist = -1, idGenre = -1, idAlbum = -1, idSong = -1;
+  bool albumArtistsOnly = false;
+  std::string artistname;
+
+  // Process albumartistsonly option
+  option = options.find("albumartistsonly");
+  if (option != options.end())
+    albumArtistsOnly = option->second.asBoolean();
 
-    option = options.find("artistid");
+  // Process genre option
+  option = options.find("genreid");
+  if (option != options.end())
+    idGenre = static_cast<int>(option->second.asInteger());
+  else
+  {
+    option = options.find("genre");
     if (option != options.end())
-      idArtist = (int)option->second.asInteger();
+      idGenre = GetGenreByName(option->second.asString());
+  }
 
-    option = options.find("genreid");
+  // Process album option
+  option = options.find("albumid");
+  if (option != options.end())
+    idAlbum = static_cast<int>(option->second.asInteger());
+  else
+  {
+    option = options.find("album");
     if (option != options.end())
-      idGenre = (int)option->second.asInteger();
-    else
-    {
-      option = options.find("genre");
-      if (option != options.end())
-        idGenre = GetGenreByName(option->second.asString());
-    }
+      idAlbum = GetAlbumByName(option->second.asString());
+  }
 
-    option = options.find("albumid");
+  // Process artist option
+  option = options.find("artistid");
+  if (option != options.end())
+    idArtist = static_cast<int>(option->second.asInteger());
+  else
+  {
+    option = options.find("artist");
     if (option != options.end())
-      idAlbum = (int)option->second.asInteger();
-    else
     {
-      option = options.find("album");
-      if (option != options.end())
-        idAlbum = GetAlbumByName(option->second.asString());
+      idArtist = GetArtistByName(option->second.asString());
+      if (idArtist == -1)
+      {// not found with that name, or more than one found as artist name is not unique
+        artistname = option->second.asString();
+      }
     }
+  }
 
-    option = options.find("songid");
-    if (option != options.end())
-      idSong = (int)option->second.asInteger();
-
-    option = options.find("albumartistsonly");
-    if (option != options.end())
-      albumArtistsOnly = option->second.asBoolean();
+  //  Process song option
+  option = options.find("songid");
+  if (option != options.end())
+    idSong = static_cast<int>(option->second.asInteger());
 
-    std::string strSQL = "artistview.idArtist IN ";
-    if (idArtist > 0)
-      strSQL += PrepareSQL("(%d)", idArtist);
-    else if (idAlbum > 0)
-      strSQL += PrepareSQL("(SELECT album_artist.idArtist FROM album_artist" 
-                           " WHERE album_artist.idAlbum = %i)", idAlbum);
-    else if (idSong > 0)
-    {
-      strSQL += PrepareSQL("(SELECT song_artist.idArtist FROM song_artist"
-                           " WHERE song_artist.idSong = %i %s)", idSong, strRoleSQL.c_str());
-    }
-    else if (idGenre > 0)
-    { // same statements as below, but limit to the specified genre
-      if (albumArtistsOnly || idRole == 1)  //Album artists only or no specific role, check album_artist table
-        strSQL += PrepareSQL("(SELECT album_artist.idArtist FROM album_artist" // All album artists linked to extra genres
-        " JOIN album_genre ON album_artist.idAlbum = album_genre.idAlbum"
-        " WHERE album_genre.idGenre = %i)", idGenre);
-      if (albumArtistsOnly && idRole > 1)  //Album artists only with role, check AND in song_artist
-        strSQL += " AND artistview.idArtist IN ";
-      if (!albumArtistsOnly && idRole == 1) //Song and album artists no specific role, check OR in song_artist
-        strSQL += " OR artistview.idArtist IN ";
-      if (!albumArtistsOnly || idRole > 1)  //Song and album artists or album artists with specific role, check song_artist
-        strSQL += PrepareSQL("(SELECT song_artist.idArtist FROM song_artist" // All artists linked to extra genres
-        " JOIN song_genre ON song_artist.idSong = song_genre.idSong"
-        " WHERE song_genre.idGenre = %i AND song_artist.idRole = %i )", idGenre, idRole);
-
-      if (idRole < -1) //All artists contributing to songs, all roles.
-        strSQL = PrepareSQL("artistview.idArtist IN (SELECT song_artist.idArtist FROM song_artist"
-        " JOIN song_genre ON song_artist.idSong = song_genre.idSong"
-        " WHERE song_genre.idGenre = %i)"
-        " OR artistview.idArtist IN"
-        " (SELECT album_artist.idArtist FROM album_artist" // All album artists linked to extra genres
-        " JOIN album_genre ON album_artist.idAlbum = album_genre.idAlbum"
-        " WHERE album_genre.idGenre = %i)",
-        idGenre, idGenre);
-    }
-    else
-    { // Artists can be only album artists, so for all artists (with linked albums or songs) 
-      // we need to check both album_artist and song_artist tables.
-      // Role is determined from song_artist table, so even if looking for album artists only
-      // we can check those that have a specific role e.g. which album artist is a composer
-      // (but not neccessarily of songs in that album), from entries in the song_artist table.
-      // Role < -1 is used to indicate that all roles are wanted.
-      // When not album artists only and a specific role wanted then only the song_artist table is checked.
-      // When album artists only and role = 1 (an "artist") then only the album_artist table is checked.
-      if (albumArtistsOnly || idRole == 1)  //Album artists only or no specific role, check album_artist table
-        strSQL += "(SELECT album_artist.idArtist FROM album_artist )"; // Includes compliation albums hence "Various artists"
-      if (albumArtistsOnly && idRole > 1)  //Album artists only with role, check AND in song_artist
-        strSQL += " AND artistview.idArtist IN ";
-      if (!albumArtistsOnly && idRole == 1) //Song and album artists no specific role, check OR in song_artist
-        strSQL += " OR artistview.idArtist IN ";
-      if (!albumArtistsOnly || idRole > 1)  //Song and album artists or album artists with specific role, check song_artist
-        strSQL += PrepareSQL("(SELECT song_artist.idArtist FROM song_artist WHERE song_artist.idRole = %i)", idRole);
-
-      if (idRole < 0) //All artists contributing to songs, all roles.
-        strSQL = "artistview.idArtist IN (SELECT song_artist.idArtist FROM song_artist) OR "
-                 "artistview.idArtist IN (SELECT album_artist.idArtist FROM album_artist)";
-    }
+  if (type == "artists")
+  {
     if (!hasRoleRules)
-      filter.AppendWhere(strSQL);
-
+    { // Not an "artists" smart playlist with roles rules, so get filter from options
+      if (idArtist > 0)
+        filter.AppendWhere(PrepareSQL("artistview.idArtist = %d", idArtist));
+      else if (idAlbum > 0)
+        filter.AppendWhere(PrepareSQL("artistview.idArtist IN (SELECT album_artist.idArtist FROM album_artist "
+          "WHERE album_artist.idAlbum = %i)", idAlbum));
+      else if (idSong > 0)
+      {
+        filter.AppendWhere(PrepareSQL("artistview.idArtist IN (SELECT song_artist.idArtist FROM song_artist "
+          "WHERE song_artist.idSong = %i %s)", idSong, strRoleSQL.c_str()));
+      }
+      else
+      { // Artists can be only album artists, so for all artists (with linked albums or songs) 
+        // we need to check both album_artist and song_artist tables.
+        // Role is determined from song_artist table, so even if looking for album artists only
+        // we can check those that have a specific role e.g. which album artist is a composer
+        // of songs in that album, from entries in the song_artist table.
+        // Role < -1 is used to indicate that all roles are wanted.
+        // When not album artists only and a specific role wanted then only the song_artist table is checked.
+        // When album artists only and role = 1 (an "artist") then only the album_artist table is checked.
+        std::string albumArtistSQL, songArtistSQL;
+        ExistsSubQuery albumArtistSub("album_artist", "album_artist.idArtist = artistview.idArtist");
+        ExistsSubQuery songArtistSub("song_artist", "song_artist.idArtist = artistview.idArtist");
+        if (idRole > 0)
+          songArtistSub.AppendWhere(PrepareSQL("song_artist.idRole = %i", idRole));
+        if (idGenre > 0)
+        {
+          songArtistSub.AppendJoin("JOIN song_genre ON song_genre.idSong = song_artist.idSong");
+          songArtistSub.AppendWhere(PrepareSQL("song_genre.idGenre = %i", idGenre));
+        }
+        if (idRole <= 1 && idGenre > 0)
+        {// Check genre of songs of album using nested subquery
+          std::string strGenre = PrepareSQL("EXISTS(SELECT 1 FROM song JOIN song_genre ON song_genre.idSong = song.idSong "
+            "WHERE song.idAlbum = album_artist.idAlbum AND song_genre.idGenre = %i)", idGenre);
+          albumArtistSub.AppendWhere(strGenre);
+        }
+        if (idRole > 1 && albumArtistsOnly)
+        { // Album artists only with role, check AND in album_artist for album of song
+          // using nested subquery correlated with album_artist
+          songArtistSub.AppendJoin("JOIN song ON song.idSong = song_artist.idSong");
+          songArtistSub.param = "song_artist.idArtist = album_artist.idArtist";
+          songArtistSub.AppendWhere("song.idAlbum = album_artist.idAlbum");
+          songArtistSub.BuildSQL(songArtistSQL);
+          albumArtistSub.AppendWhere(songArtistSQL);
+          albumArtistSub.BuildSQL(albumArtistSQL);
+          filter.AppendWhere(albumArtistSQL);
+        }
+        else
+        {
+          songArtistSub.BuildSQL(songArtistSQL);
+          albumArtistSub.BuildSQL(albumArtistSQL);
+          if (idRole < 0 || (idRole == 1 && !albumArtistsOnly))
+          { // Artist contributing to songs, any role, check OR album artist too
+            // as artists can be just album artists but not song artists
+            filter.AppendWhere(songArtistSQL + " OR " + albumArtistSQL);
+          }
+          else if (idRole > 1)
+          {
+            // Artist contributes that role (not albmartistsonly as already handled)
+            filter.AppendWhere(songArtistSQL);
+          }
+          else // idRole = 1 and albumArtistsOnly
+          { // Only look at album artists, not albums where artist features on songs
+            filter.AppendWhere(albumArtistSQL);
+          }
+        }
+      }
+    }
     // remove the null string
     filter.AppendWhere("artistview.strArtist != ''");
 
@@ -6454,51 +6480,94 @@ bool CMusicDatabase::GetFilter(CDbUrl &musicUrl, Filter &filter, SortDescription
   {
     option = options.find("year");
     if (option != options.end())
-      filter.AppendWhere(PrepareSQL("albumview.iYear = %i", (int)option->second.asInteger()));
+      filter.AppendWhere(PrepareSQL("albumview.iYear = %i", static_cast<int>(option->second.asInteger())));
     
     option = options.find("compilation");
     if (option != options.end())
       filter.AppendWhere(PrepareSQL("albumview.bCompilation = %i", option->second.asBoolean() ? 1 : 0));
 
-    option = options.find("genreid");
-    if (option != options.end())
-      filter.AppendWhere(PrepareSQL("albumview.idAlbum IN (SELECT song.idAlbum FROM song JOIN song_genre ON song.idSong = song_genre.idSong WHERE song_genre.idGenre = %i)", (int)option->second.asInteger()));
-
-    option = options.find("genre");
-    if (option != options.end())
-      filter.AppendWhere(PrepareSQL("albumview.idAlbum IN (SELECT song.idAlbum FROM song JOIN song_genre ON song.idSong = song_genre.idSong JOIN genre ON genre.idGenre = song_genre.idGenre WHERE genre.strGenre like '%s')", option->second.asString().c_str()));
+    // Process artist, role and genre options together as song subquery to filter those
+    // albums that have songs with both that artist and genre
+    std::string albumArtistSQL, songArtistSQL, genreSQL;
+    ExistsSubQuery genreSub("song", "song.idAlbum = album_artist.idAlbum");
+    genreSub.AppendJoin("JOIN song_genre ON song_genre.idSong = song.idSong");
+    genreSub.AppendWhere(PrepareSQL("song_genre.idGenre = %i", idGenre));
+    ExistsSubQuery albumArtistSub("album_artist", "album_artist.idAlbum = albumview.idAlbum");
+    ExistsSubQuery songArtistSub("song_artist", "song.idAlbum = albumview.idAlbum");
+    songArtistSub.AppendJoin("JOIN song ON song.idSong = song_artist.idSong");
 
-    option = options.find("artistid");
-    if (option != options.end())
+    if (idArtist > 0)
     {
-      int idArtist = (int)option->second.asInteger();
-      filter.AppendWhere(PrepareSQL(
-      "(EXISTS (SELECT 1 FROM song JOIN song_artist ON song.idSong = song_artist.idSong"
-      " WHERE song.idAlbum = albumview.idAlbum AND song_artist.idArtist = %i %s) OR "
-      "EXISTS (SELECT 1 FROM album_artist"
-      " WHERE album_artist.idAlbum = albumview.idAlbum AND album_artist.idArtist = %i))",
-      idArtist, strRoleSQL.c_str(), idArtist));
+      songArtistSub.AppendWhere(PrepareSQL("song_artist.idArtist = %i", idArtist));
+      albumArtistSub.AppendWhere(PrepareSQL("album_artist.idArtist = %i", idArtist));
     }
-    else
+    else if (!artistname.empty())
+    { // Artist name is not unique, so could get albums or songs from more than one.
+      songArtistSub.AppendJoin("JOIN artist ON artist.idArtist = song_artist.idArtist");
+      songArtistSub.AppendWhere(PrepareSQL("artist.strArtist like '%s'", artistname.c_str()));
+      
+      albumArtistSub.AppendJoin("JOIN artist ON artist.idArtist = song_artist.idArtist");
+      albumArtistSub.AppendWhere(PrepareSQL("artist.strArtist like '%s'", artistname.c_str()));
+    }
+    if (idRole > 0)
+      songArtistSub.AppendWhere(PrepareSQL("song_artist.idRole = %i", idRole));
+    if (idGenre > 0)
+    {
+      songArtistSub.AppendJoin("JOIN song_genre ON song_genre.idSong = song.idSong");
+      songArtistSub.AppendWhere(PrepareSQL("song_genre.idGenre = %i", idGenre));
+    }
+
+    if (idArtist > 0 || !artistname.empty())
     {
-      option = options.find("artist");
-      if (option != options.end())
-        filter.AppendWhere(PrepareSQL(
-          "albumview.idAlbum IN (SELECT song.idAlbum FROM song JOIN song_artist ON song.idSong = song_artist.idSong "
-          "JOIN artist ON artist.idArtist = song_artist.idArtist "
-          "WHERE artist.strArtist like '%s' %s) OR " // All albums linked to this artist via songs
-          "albumview.idAlbum IN (SELECT album_artist.idAlbum FROM album_artist "
-          "JOIN artist ON artist.idArtist = album_artist.idArtist "
-          "WHERE artist.strArtist like '%s')", // All albums where album artists fit
-          option->second.asString().c_str(), strRoleSQL.c_str(), option->second.asString().c_str()));
-      // no artist given, so exclude any single albums (aka empty tagged albums)
+      if (idRole <= 1 && idGenre > 0)
+      { // Check genre of songs of album using nested subquery
+        genreSub.BuildSQL(genreSQL);
+        albumArtistSub.AppendWhere(genreSQL);
+      }
+      if (idRole > 1 && albumArtistsOnly)
+      {  // Album artists only with role, check AND in album_artist for same song
+         // using nested subquery correlated with album_artist
+         songArtistSub.param = "song.idAlbum = album_artist.idAlbum";
+         songArtistSub.BuildSQL(songArtistSQL);
+         albumArtistSub.AppendWhere(songArtistSQL);
+         albumArtistSub.BuildSQL(albumArtistSQL);
+         filter.AppendWhere(albumArtistSQL);    
+      }
       else
       {
-        option = options.find("show_singles");
-        if (option == options.end() || !option->second.asBoolean())
-          filter.AppendWhere(PrepareSQL("albumview.strReleaseType = '%s'", CAlbum::ReleaseTypeToString(CAlbum::Album).c_str()));
+        songArtistSub.BuildSQL(songArtistSQL);
+        albumArtistSub.BuildSQL(albumArtistSQL);
+        if (idRole < 0 || (idRole == 1 && !albumArtistsOnly))  
+        { // Artist contributing to songs, any role, check OR album artist too
+          // as artists can be just album artists but not song artists
+          filter.AppendWhere(songArtistSQL + " OR " + albumArtistSQL);
+        }
+        else if (idRole > 1)
+        { // Albums with songs where artist contributes that role (not albmartistsonly as already handled)
+          filter.AppendWhere(songArtistSQL);
+        }
+        else // idRole = 1 and albumArtistsOnly
+        { // Only look at album artists, not albums where artist features on songs
+          // This may want to be a separate option so you can choose to see all the albums where that artist 
+          // appears on one or more songs without having to list all song artists in the artists node.
+          filter.AppendWhere(albumArtistSQL);
+        }
       }
     }
+    else
+    { // No artist given 
+      if (idGenre > 0)
+      { // Have genre option but not artist
+        genreSub.param = "song.idAlbum = albumview.idAlbum";
+        genreSub.BuildSQL(genreSQL);
+        filter.AppendWhere(genreSQL);
+      }
+      // Exclude any single albums (aka empty tagged albums)
+      // This causes "albums"  media filter artist selection to only offer album artists       
+      option = options.find("show_singles");
+      if (option == options.end() || !option->second.asBoolean())
+        filter.AppendWhere(PrepareSQL("albumview.strReleaseType = '%s'", CAlbum::ReleaseTypeToString(CAlbum::Album).c_str()));
+    }
   }
   else if (type == "songs" || type == "singles")
   {
@@ -6510,45 +6579,58 @@ bool CMusicDatabase::GetFilter(CDbUrl &musicUrl, Filter &filter, SortDescription
 
     option = options.find("year");
     if (option != options.end())
-      filter.AppendWhere(PrepareSQL("songview.iYear = %i", (int)option->second.asInteger()));
+      filter.AppendWhere(PrepareSQL("songview.iYear = %i", static_cast<int>(option->second.asInteger())));
     
     option = options.find("compilation");
     if (option != options.end())
       filter.AppendWhere(PrepareSQL("songview.bCompilation = %i", option->second.asBoolean() ? 1 : 0));
 
-    option = options.find("albumid");
-    if (option != options.end())
-      filter.AppendWhere(PrepareSQL("songview.idAlbum = %i", (int)option->second.asInteger()));
-    
-    option = options.find("album");
-    if (option != options.end())
-      filter.AppendWhere(PrepareSQL("songview.strAlbum like '%s'", option->second.asString().c_str()));
-
-    option = options.find("genreid");
-    if (option != options.end())
-      filter.AppendWhere(PrepareSQL("songview.idSong IN (SELECT song_genre.idSong FROM song_genre WHERE song_genre.idGenre = %i)", (int)option->second.asInteger()));
+    if (idAlbum > 0)
+      filter.AppendWhere(PrepareSQL("songview.idAlbum = %i", idAlbum));
 
-    option = options.find("genre");
-    if (option != options.end())
-      filter.AppendWhere(PrepareSQL("songview.idSong IN (SELECT song_genre.idSong FROM song_genre JOIN genre ON genre.idGenre = song_genre.idGenre WHERE genre.strGenre like '%s')", option->second.asString().c_str()));
+    if (idGenre > 0)
+      filter.AppendWhere(PrepareSQL("songview.idSong IN (SELECT song_genre.idSong FROM song_genre WHERE song_genre.idGenre = %i)", idGenre));
 
-    option = options.find("artistid");
-    if (option != options.end())
-      filter.AppendWhere(PrepareSQL(
-        "songview.idSong IN (SELECT song_artist.idSong FROM song_artist "
-        "WHERE song_artist.idArtist = %i %s) OR " // song artists   
-        "songview.idSong IN (SELECT song.idSong FROM song JOIN album_artist ON song.idAlbum=album_artist.idAlbum "
-        "WHERE album_artist.idArtist = %i)", // album artists
-        (int)option->second.asInteger(), strRoleSQL.c_str(), (int)option->second.asInteger()));
-  
-    option = options.find("artist");
-    if (option != options.end())
-      filter.AppendWhere(PrepareSQL(
-        "songview.idSong IN (SELECT song_artist.idSong FROM song_artist JOIN artist ON artist.idArtist = song_artist.idArtist "
-        "WHERE artist.strArtist like '%s' %s) OR " // song artists
-        "songview.idSong IN (SELECT song.idSong FROM song JOIN album_artist ON song.idAlbum=album_artist.idAlbum "
-        "JOIN artist ON artist.idArtist = album_artist.idArtist WHERE artist.strArtist like '%s')", // album artists
-        option->second.asString().c_str(), strRoleSQL.c_str(), option->second.asString().c_str()));
+    std::string songArtistClause, albumArtistClause;
+    if (idArtist > 0)
+    {
+      songArtistClause = PrepareSQL("EXISTS (SELECT 1 FROM song_artist "
+        "WHERE song_artist.idSong = songview.idSong AND song_artist.idArtist = %i %s)",
+        idArtist, strRoleSQL.c_str());
+      albumArtistClause = PrepareSQL("EXISTS (SELECT 1 FROM album_artist "
+        "WHERE album_artist.idAlbum = songview.idAlbum AND album_artist.idArtist = %i)",
+        idArtist);
+    }
+    else if (!artistname.empty())
+    {  // Artist name is not unique, so could get songs from more than one.
+      songArtistClause = PrepareSQL("EXISTS (SELECT 1 FROM song_artist JOIN artist ON artist.idArtist = song_artist.idArtist "
+        "WHERE song_artist.idSong = songview.idSong AND artist.strArtist like '%s' %s)",
+        artistname.c_str(), strRoleSQL.c_str());
+      albumArtistClause = PrepareSQL("EXISTS (SELECT 1 FROM album_artist JOIN artist ON artist.idArtist = album_artist.idArtist "
+        "WHERE album_artist.idAlbum = songview.idAlbum AND artist.strArtist like '%s')",
+        artistname.c_str());
+    }
+
+    // Process artist name or id option
+    if (!songArtistClause.empty())
+    {
+      if (idRole < 0) // Artist contributes to songs, any roles OR is album artist
+        filter.AppendWhere("(" + songArtistClause + " OR " + albumArtistClause + ")");
+      else if (idRole > 1)
+      {
+        if (albumArtistsOnly)  //Album artists only with role, check AND in album_artist for same song
+          filter.AppendWhere("(" + songArtistClause + " AND " + albumArtistClause + ")");
+        else // songs where artist contributes that role.
+          filter.AppendWhere(songArtistClause);
+      }
+      else
+      {
+        if (albumArtistsOnly) // Only look at album artists, not where artist features on songs            
+          filter.AppendWhere(albumArtistClause);
+        else // Artist is song artist or album artist
+          filter.AppendWhere("(" + songArtistClause + " OR " + albumArtistClause + ")");
+      }
+    }
   }
 
   option = options.find("filter");