Opened 11 years ago

Closed 11 years ago

#21757 closed Cleanup/optimization (wontfix)

Allow modifying the SQL generated by lookups

Reported by: shmishleniy@… Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: postgresql like ilike icontains
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi!
I know that you guys rejected all issues to implement LIKE and ILIKE
Sorry but I would like to try one more time because I don't understand this limitation of your orm system.

  1. We need to update fieldlookup-contains docs . There is no specified about your guys fixes of icontains (UPPER LIKE instead of ILIKE) on Postgressql database.
  1. Regarding postgres backend should optimize iexact searches task (GitHub source code) we have auto optimization that forced us to use UPPER LIKE. For fully functional orm system is good practice to give your users possibility to "easy" build whatever they like request. Now on postgresql database "out of the box" we can't use ILIKE at all. We always need to add custom lookup to manage with this problem.

Example of ILIKE/UPPER LIKE usage ypou can find in my comment

Attachments (5)

case1.PNG (97.8 KB ) - added by shmishleniy@… 11 years ago.
Performance of ILIKE without index
case2.PNG (97.9 KB ) - added by shmishleniy@… 11 years ago.
Performance of LIKE UPPER without index
case3.PNG (103.1 KB ) - added by shmishleniy@… 11 years ago.
Performance of ILIKE with the relevant index
case4.PNG (103.0 KB ) - added by shmishleniy@… 11 years ago.
Performance of LIKE with index from case 3 (case sensitive search)
case5.PNG (100.6 KB ) - added by shmishleniy@… 11 years ago.
Performance of LIKE UPPER with the relevant index

Download all attachments as: .zip

Change History (13)

comment:1 by Anssi Kääriäinen, 11 years ago

Summary: orm limitationsAllow modifying the SQL generated by lookups
Triage Stage: UnreviewedAccepted
Version: 1.6master

This issue will be hopefully fixed with custom lookups (#16187, https://github.com/django/django/pull/2019). Using custom lookups you could write your own lookup 'ilike', or override how icontains works (either by writing a custom IContains lookup, or by overriding the default implementation with @add_implementation).

I'm going to mark this as accepted, with the meaning that we should allow some easier way for users to override the SQL generated for icontains (and for other lookups for that matter). I don't think we should go alter the default implementation, I believe the optimization could lead to performance problems for other users.

comment:2 by Marc Tamlyn, 11 years ago

Reading your EXPLAIN output, the easiest thing to do will be for you to add a PG functional index to that column on UPPER(value). According to the benchmarks in the old ticket (though it is possible PG's optimisations have changed since then) this will actually come out faster than your ILIKE query you have at the moment. You would need to create that index by hand (or using South or db.migrations in 1.7). It would be interesting to see an updated version of the benchmarks from #3575 as they may be different with Postgres 9 than it was with whatever version of PG we had 6 years ago. I would still be nervous about changing the default though as this could break optimisations other people have put in place.

A full benchmark would need to show:

  • Performance of ILIKE without index
  • Performance of LIKE UPPER without index
  • Performance of ILIKE with the relevant index
  • Performance of LIKE UPPER with the relevant index

At the moment I believe your tests are showing the second and third in that list, which is a little unfair as one utilises an index and the other does a sequential scan.

I do not believe the documentation needs to be updated - this kind of optimisation is an advanced use case.

There are a couple of ideas in progress which will result in making this kind of thing easier. One is custom lookups (#16187), the other is custom indexes (can't remember the number).

by shmishleniy@…, 11 years ago

Attachment: case1.PNG added

Performance of ILIKE without index

by shmishleniy@…, 11 years ago

Attachment: case2.PNG added

Performance of LIKE UPPER without index

by shmishleniy@…, 11 years ago

Attachment: case3.PNG added

Performance of ILIKE with the relevant index

by shmishleniy@…, 11 years ago

Attachment: case4.PNG added

Performance of LIKE with index from case 3 (case sensitive search)

by shmishleniy@…, 11 years ago

Attachment: case5.PNG added

Performance of LIKE UPPER with the relevant index

comment:3 by shmishleniy@…, 11 years ago

Thanks to all for response.
Custom lookups it's great feature.

Create two indexes for normal case and for UPPER case not good solution for me becouse I will waste 2 time more space and no increase performance.

Cases:

  1. Performance of ILIKE without index
  2. Performance of LIKE UPPER without index
  3. Performance of ILIKE with the relevant index
  4. Performance of LIKE with index from case 3 (case sensitive search)
  5. Performance of LIKE UPPER with the relevant index

in reply to:  2 comment:4 by shmishleniy@…, 11 years ago

Replying to mjtamlyn:

At the moment I believe your tests are showing the second and third in that list, which is a little unfair as one utilises an index and the other does a sequential scan.

Yes you are right. Added screenshots in attachments :)

comment:5 by Marc Tamlyn, 11 years ago

Seems there's not much difference in the indexed case these days, but there is in the non indexed case which is likely what people will do a lot. So that resolves that the default behaviour will stay as is, this ticket is not just about making it possible to change how __icontains etc work for your use case as akaariai dsicussed.

comment:6 by Anssi Kääriäinen, 11 years ago

Custom lookups are now implemented. I believe you can alter the feature to work in the way you wish by doing something like this:

from django.db.models import Lookup, Field

class Like(Lookup):
    lookup_name = 'like'

    def as_sql(self, qn, connection):
        lhs, lhs_params = self.process_lhs(qn, connection)
        rhs, rhs_params = self.process_rhs(qn, connection)
        return '%s LIKE %s' % (lhs, rhs), lhs_params + rhs_params
Field.register_lookup(Like)


class ILike(Lookup):
    lookup_name = 'ilike'

    def as_sql(self, qn, connection):
        lhs, lhs_params = self.process_lhs(qn, connection)
        rhs, rhs_params = self.process_rhs(qn, connection)
        return '%s ILIKE %s' % (lhs, rhs), lhs_params + rhs_params
Field.register_lookup(ILike)

I haven't actually tried the lookups mentioned above. Try it yourself, any feedback of the lookup system is very welcome.

Assuming the lookups mentioned above implement what is wanted we could just close this ticket.

comment:7 by Anssi Kääriäinen, 11 years ago

Actually, even if the lookup system doesn't allow writing the like/ilike lookups for some odd reason, we should improve the lookup system instead so that what is wanted in this ticket will be possible. So, closing as wontfix. If the lookup system doesn't work for your use case, then please open a new ticket for suggestions of how to improve the lookup system.

comment:8 by Anssi Kääriäinen, 11 years ago

Resolution: wontfix
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top