Code

Opened 3 months ago

Closed 3 months 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: master
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@… 3 months ago.
Performance of ILIKE without index
case2.PNG (97.9 KB) - added by shmishleniy@… 3 months ago.
Performance of LIKE UPPER without index
case3.PNG (103.1 KB) - added by shmishleniy@… 3 months ago.
Performance of ILIKE with the relevant index
case4.PNG (103.0 KB) - added by shmishleniy@… 3 months ago.
Performance of LIKE with index from case 3 (case sensitive search)
case5.PNG (100.6 KB) - added by shmishleniy@… 3 months ago.
Performance of LIKE UPPER with the relevant index

Download all attachments as: .zip

Change History (13)

comment:1 Changed 3 months ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from orm limitations to Allow modifying the SQL generated by lookups
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.6 to master

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 follow-up: Changed 3 months ago by mjtamlyn

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).

Changed 3 months ago by shmishleniy@…

Performance of ILIKE without index

Changed 3 months ago by shmishleniy@…

Performance of LIKE UPPER without index

Changed 3 months ago by shmishleniy@…

Performance of ILIKE with the relevant index

Changed 3 months ago by shmishleniy@…

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

Changed 3 months ago by shmishleniy@…

Performance of LIKE UPPER with the relevant index

comment:3 Changed 3 months ago by shmishleniy@…

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

comment:4 in reply to: ↑ 2 Changed 3 months ago by shmishleniy@…

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 Changed 3 months ago by mjtamlyn

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 Changed 3 months ago by akaariai

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 Changed 3 months ago by akaariai

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 Changed 3 months ago by akaariai

  • Resolution set to wontfix
  • Status changed from new to closed

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.