Bug 11007 - Adding all tracks to playlist incorrectly sorts albums
: Adding all tracks to playlist incorrectly sorts albums
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Database
: 7.3.3
: PC Windows XP
: P3 normal (vote)
: 7.6.0
Assigned To: Jim McAtee
: new_schema
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-08 03:07 UTC by Jim McAtee
Modified: 2011-05-09 10:16 UTC (History)
6 users (show)

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jim McAtee 2009-02-08 03:07:09 UTC
Change 8212 for bug 3629 originally fixed this and looks to be correct.  I can't track down where exactly the change was made that broke it.

The fix is to change server/Slim/Control/Commands.pm, line 2853 from

my $albumSort = "concat(album.titlesort, '0'), me.disc, me.tracknum, concat(me.titlesort, '0')";

back to the original

my $albumSort = "album.titlesort, me.disc, me.tracknum, me.titlesort";


Appending 0 to the titlesort causes names like

Greatest Hits
Greatest Hits II

to sort incorectly, since 0 is sorted after the space character.
Comment 1 Moonbase 2009-02-08 03:28:56 UTC
I wonder what the original reason was to introduce this? (appending a zero)

Maybe people switching "natural sorting" on and off in Windows? If changed back, this needs to be duly tested. For all other locale-dependent sorting, appending nothing (or *maybe* a blank) should be sufficient, I think.
Comment 2 Adrian Smith 2009-02-08 03:29:24 UTC
The change in question is 8427 which looks to have been in response to bug 3729 - can't play all music for a year (but I'm guessing slightly as there's 4 bugs against that commit)

Should we make 0 the lowest sorting character in the character set?  [not sure what this - space is probably the safest?]
Comment 3 Jim McAtee 2009-02-08 03:48:44 UTC
I see a lot of other code in other modules where

concat(xxx.titlesort, '0')

was later changed to

concat('0', xxx.titlesort)

Maybe this was just overlooked?  Frankly, I can't figure out why you'd need either.  The second version should be a noop.

Bug 2563 was a request for natural sorting.  I can't tell if the appending of a '0' was a failed attempt at a natural sort, or if there was some other reason for it.
Comment 4 James Richardson 2009-02-09 07:26:05 UTC
is bug 11008 possible a dupe of this bug?
Comment 5 Jim McAtee 2009-02-09 11:14:37 UTC
(In reply to comment #4)
> is bug 11008 possible a dupe of this bug?

I don't believe so.
Comment 6 James Richardson 2009-02-23 14:13:00 UTC
KDF: your comments on this topic?
Comment 7 Jim McAtee 2009-02-23 14:27:50 UTC
I wonder if maybe this concatenation was a holdover from SQLite.  There are some old comments in the code about this forcing a numeric sort, which I'm pretty certain MySQL does not do when you append or prepend a zero to a character column.
Comment 8 KDF 2009-02-23 14:54:28 UTC
Many of the concat code was being added while MySQL was already in use.  This is not my area of expertise.  All of the db code during this time was handled by Dan.  I avoided the db code completely aside from capturing logs of the key data. With all of the branching and svn re-organisation it is hard to get an idea of the exact flow of changes around that time (I keep getting errors when requesting annotated versions of old revs).  Only Dan could fill in the blanks, if he remembers exactly why those changes were made.

Bug 11008 could be related to the same kind of code issues. The browse results aren't always a direct link to the results for a play command as the results are regenerated during the playXcommand call.
Comment 9 Chris Owens 2009-03-02 09:17:34 UTC
Andy notes that Alan was talking about this last week, in the context of needing to change statements using concat for sqlite.

Alan does that mean that this behavior is all going to change and we should just plan to review it after that work is done?
Comment 10 Alan Young 2009-03-02 09:58:22 UTC
I presume that this would indeed all change with new-schema.
Comment 11 Blackketter Dean 2009-06-09 12:46:30 UTC
JJ: Is this still happening with the latest build?
Comment 12 Jim McAtee 2009-06-09 13:10:13 UTC
Sorts correctly in the SQLite branch, incorrectly in the 7.4 Trunk with MySQL.
Comment 13 Ben Klaas 2009-08-26 07:52:28 UTC
this is an administrative shuffle on priority fields to help make better judgment on the top end of the priority list. P4->P5, P3->P4, and P2->P3.
Comment 14 Michael Herger 2009-10-02 03:46:58 UTC
Is this still an issue?
Comment 15 Chris Owens 2010-03-15 18:06:47 UTC
7.4.x milestone is in the past
Comment 16 Paul Chandler 2011-05-09 10:16:34 UTC
ADded all tracks to playlist----sorted by album in the correct order  7.6.32390