GNOME Bugzilla – Bug 547078
Parentheses in search queries are not handled correctly
Last modified: 2008-08-16 10:48:26 UTC
0. Find an album with parentheses in its name. My example is "Lost Dogs (Disc 2)" 1. Enter this query in the search field (replace the album name of course): album=="Lost Dogs (Disc 2)" Expected results: Only tracks with that album name appear in the track list. Actual results: No tracks appear in the track list. This is due, it seems, to improper parsing of the search text, perhaps because parentheses can be used as grouping operators? I looked at line 303 of DatabaseTrackListModel.cs to see what SQL query was being generated by the search text. Here is a good example (no parentheses): FROM CoreTracks,CoreAlbums,CoreArtists WHERE CoreArtists.ArtistID = CoreTracks.ArtistID AND CoreAlbums.AlbumID = CoreTracks.AlbumID AND CoreAlbums.AlbumID = CoreTracks.AlbumID AND CoreTracks.PrimarySourceID = 1 AND (CoreAlbums.TitleLowered = 'inka show' AND CoreAlbums.TitleLowered IS NOT NULL) ORDER BY CoreAlbums.ArtistNameLowered ASC, CoreAlbums.TitleLowered ASC, CoreTracks.Disc ASC, CoreTracks.TrackNumber ASC And here is what is produced when my search text is "Lost Dogs (Disc 2)": FROM CoreTracks,CoreAlbums,CoreArtists WHERE CoreArtists.ArtistID = CoreTracks.ArtistID AND CoreAlbums.AlbumID = CoreTracks.AlbumID AND CoreAlbums.AlbumID = CoreTracks.AlbumID AND CoreTracks.PrimarySourceID = 1 AND ((CoreAlbums.TitleLowered = 'lost dogs ' AND CoreAlbums.TitleLowered IS NOT NULL) And (((CoreArtists.NameLowered LIKE '%disc%' AND CoreArtists.NameLowered IS NOT NULL) OR (CoreAlbums.TitleLowered LIKE '%disc%' AND CoreAlbums.TitleLowered IS NOT NULL) OR (CoreTracks.TitleLowered LIKE '%disc%' AND CoreTracks.TitleLowered IS NOT NULL)) And ((CoreArtists.NameLowered LIKE '%2%' AND CoreArtists.NameLowered IS NOT NULL) OR (CoreAlbums.TitleLowered LIKE '%2%' AND CoreAlbums.TitleLowered IS NOT NULL) OR (CoreTracks.TitleLowered LIKE '%2%' AND CoreTracks.TitleLowered IS NOT NULL))) And ((CoreArtists.NameLowered LIKE '%ï¿¿%' AND CoreArtists.NameLowered IS NOT NULL) OR (CoreAlbums.TitleLowered LIKE '%ï¿¿%' AND CoreAlbums.TitleLowered IS NOT NULL) OR (CoreTracks.TitleLowered LIKE '%ï¿¿%' AND CoreTracks.TitleLowered IS NOT NULL))) ORDER BY CoreAlbums.ArtistNameLowered ASC, CoreAlbums.TitleLowered ASC, CoreTracks.Disc ASC, CoreTracks.TrackNumber ASC I tested this query to make sure it would work: sqlite> select * from CoreTracks,CoreAlbums,CoreArtists ...> where CoreArtists.ArtistID = CoreTracks.ArtistID and ...> CoreAlbums.AlbumID = CoreTracks.AlbumID and ...> CoreTracks.PrimarySourceID = 1 and ...> CoreAlbums.TitleLowered = 'lost dogs (disc 2)'; Sure enough, it worked fine. So I think there is a legitimate bug in the UserQueryParser. By the way, you might also notice that the condition "CoreAlbums.AlbumID = CoreTracks.AlbumID" occurs twice in the WHERE clause of the query that is generated. This bug blocks a perfect resolution to bug #428849.
I'm seeing the same behavior. I'll post a patch that should fix this.
Created attachment 116253 [details] [review] Fix handling of special characters in quotes Parentheses and other special characters should not terminate a string if the string is enclosed in quotes. This leads to a simplification of the IsStringTerminationChar method. I can't run the unit test on my system, so someone should run them with this patch before committing.
Cool, I'll review and test when I get a chance. Probably some new test(s) should be added to cover this case, too.
No new test failures with this patch.
Created attachment 116294 [details] [review] Test for proper parsing behavior Quickie test that the correct SQL fragment is generated for the query: artist=="foo (disc 2)" There should probably be some tests that grouping works as expected, too.
After a long and ugly fight with nunit 2.4, I'm now able to run the unit tests. So I can confirm that Sandy's test case does detect the bug and the fact that my patch fixes it.
Ok, looks good guys. Would you change the test to check that the correct user-query-language is generated instead of checking the SQL? The SQL is much more likely to change in the future than the user-query language. Commit away!
Committed both, taking into account Gabriel's comment.