Opened 7 years ago

Closed 3 years ago

Last modified 3 years ago

#7580 closed New feature (duplicate)

Support ORDER BY BINARY in MySQL

Reported by: Paul Kenjora <pkenjora@…> Owned by: anonymous
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: mysql, binary, order by
Cc: mmitar@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by mtredinnick)

The Problem:

MySQL supports both ORDER BY and ORDER BY BINARY.

ORDER BY does NOT match the string sort used by Python but ORDER BY BINARY DOES. Lots of issues arise when returned list from DB needs to be in proper order.

The problem is described well at: http://blog.awarelabs.com/?p=18 (article by the reporter of this ticket)

The Proposed Fix:

Add a __binary tag to the end of any order_by('column') caluse, like so order_by('column__binary').
Strip the __binary tag before any processing and ensure 'BINARY ' is inserted into generated SQL.
The above solution ensures existing behavior is not altered. It does carry the side effect of making __binary a reserved word which come to think of it is reserved in SQL so no issue there.

Attachments (1)

binary_patch.diff (2.2 KB) - added by Paul Kenjora <pkenjora@…> 7 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 7 years ago by Paul Kenjora <pkenjora@…>

  • Owner changed from pkenjora to anonymous
  • Status changed from new to assigned

Changed 7 years ago by Paul Kenjora <pkenjora@…>

comment:2 Changed 7 years ago by Paul Kenjora <pkenjora@…>

  • Triage Stage changed from Unreviewed to Ready for checkin

comment:3 Changed 7 years ago by Alex

  • milestone 1.0 alpha deleted
  • Triage Stage changed from Ready for checkin to Design decision needed

comment:4 Changed 7 years ago by mtredinnick

  • Description modified (diff)

This doesn't feel like the right solution. We just do the Right Thing, rather than introduce extra options. If doing case-sensitive ordering is felt to be the right thing, then let's just do that always. I don't really see the need to be able to flip back and forth. Also, the fact that it removes "binary" as a potential field name counts against this patch (the fact that it's reserved in MySQL has nothing to do with it; we quote field names for that very reason). We should try to avoid doing stuff like that since it's difficult to tell who it will affect.

All told, I'm -1 on this approach to the problem (introducing a new option). Instead something that only added the "binary" extra bit for ordering when using MySQL as the engine (via a connection.ops feature, not via a test for MySQL in the Query class) would stand a better chance. You probably want to start a discussion on django-dev first, however, to see what the consensus might be as far as the natural ordering to use. Let's move discussion to the mailing list for now.

(Fixed wiki formatting problems in the description. The "preview" button is your friend.)

comment:5 Changed 7 years ago by Paul Kenjora <pkenjora@…>

Sounds good, I'm only familiar with MySQL on this, does anyone else know if the same issues exist on other databases? If it does then we may place it in the general query constructor, if not I completely agree that it should go into the MySQL specific manager.

Also ORDER BY BINARY is not intuitive when displaying lists to users, it only makes sense for sorting strings like python does. I would seriously consider allowing developers to keep ORDER BY, as 95% of the time ORDER BY behavior is probably desired (see article link above). The other 5% of the time ORDER BY BINARY is indispensable. I strongly urge the community to consider the making both optional, otherwise I worry we'll spawn more tickets than we close.

comment:6 Changed 7 years ago by anonymous

#6498 proposes a "*" prefix for case insensitive ordering.

Instead of adding more string magic, couldn't you just pass tupels or some OrderByClause object (that could be a subclass of Q or F (#7210), haven't thought this through)?

.order_by(('foo', 'DESC', 'BINARY'))

or

.order_by((F('foo'), 'DESC', 'BINARY'))

or

.order_by(OrderByClause(F('foo'), direction='DESC', collation='BINARY'))

comment:7 Changed 7 years ago by Paul Kenjora <pkenjora@…>

I was trying to stick with the convention of something, like in or gt, those are reserved words, binary would be in the same category of query modifiers. This approach also keeps the code clean and the modifier intuative.

As far as avoiding string magic, great suggestion, the patch above does some "binary_extra + rel" string magic, in the fix I'll probably make it "binary_extra, rel". Should keep things cleaner and saner.

comment:8 Changed 4 years ago by lukeplant

  • Severity set to Normal
  • Type set to New feature

comment:9 Changed 3 years ago by Alex

  • Easy pickings unset
  • Resolution set to duplicate
  • Status changed from assigned to closed
  • UI/UX unset

I'm going to close as a dupe of #6498, and re-purpose that ticket for being for general ways to order by alternative things.

comment:10 Changed 3 years ago by mitar

  • Cc mmitar@… added
Note: See TracTickets for help on using tickets.
Back to Top