Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#7106 closed (wontfix)

django.db extra() method fails

Reported by: aribao@… Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: db database wrapper
Cc: aribao@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Hello, I use to work with PostgreSQL, and with full-text enabled. To get full-text working I used the extra() method. After updating to the last svn version with the database refactor, this was broken. I use a custom manager to get the results.

class NewsManager( models.Manager ):
    def get_query_set(self):
        return super(NewsManager, self).get_query_set().filter( status = 2 ).order_by('-dateTimeCreated')

class news( models.Model ):
    dateTimeCreated = ...
    ...
    status = ( here I define the status of the item: published, deleted, awaiting aproval... )
    
    #The visible manager only shows the news whose state is "published"
    visible = NewsManager()

There is a problem with the positions of the arguments, and autoscaping the table name.
This is the code I use:

words = 'here the words i want to search'
items = items.extra(
                    select={
                        'headline': "ts_headline(description, query,'StartSel = <strong>, StopSel = </strong>')",
                        'rank': "ts_rank_cd(tsv, query, 32)",
                        },
                    tables=["plainto_tsquery(%s) as query"],
                    where=["tsv @@ query"],
                    params=[ words ]
                    ).order_by('-rank')

This is the SQL generated:

SELECT COUNT(*) FROM "app_news" , "plainto_tsquery(2) as query" WHERE "app_news"."status_id" = E\'words to search\' AND tsv @@ query

As you can see, the position of the arguments is wrong, and "plainto_tsquery(2) as query" shouldn't be between double quotes.

Change History (4)

comment:1 Changed 7 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to closed

It's never been intended that params would work with tables in extra(). The params argument was documented as being for the select and where arguments (and that has changed slightly after the merge as a documented backwards-incompatibility). So you're doing something that wasn't supported and happened to work by accident. Now it doesn't work. Supporting it would require adding yet another parameter and doesn't really seem worth it, since there are other ways to achieve the same thing. extra() is really for simple cases and I think this has gone a bit beyond that.

Also, because of the way aliases are managed inside queries now, trying to set your own via extra(tables=...) is going to be highly problematic (there are, at least, quoting problems as well as issues with aliases being renamed in the query).

It's hard to see exactly what's going on here, since you haven't shown the whole queryset, which apparently involves a count() call and maybe some other stuff. But if you want to tweak things at the table level like you're doing, you're going to have look at the Query class. In particular the join() method and maybe the table_alias() method. A QuerySet subclass that uses its own Query class would be an appropriate approach here.

In short, I think this is wontfix because you're doing some very unsupported stuff here that would be quite hard to accommodate in the general case and there are alternative approaches that are more robust (the subclass approach).

comment:2 Changed 7 years ago by Adrián Ribao <aribao@…>

Thank you for your reply, I have no idea about how to make it work using the subclass approach, where can I find some documentation about this?

comment:3 Changed 7 years ago by mtredinnick

It's not documented in any text file, since this is deep internals stuff. But each of the methods I referred to has docstrings that explain how they're used, so a bit of poking around and looking at how QuerySet implements things will get you started.

comment:4 Changed 7 years ago by Adrián Ribao <aribao@…>

So far I have this, and it workd:

from django.db.models.sql.query import Query
from django.db import connection

class nQuery( Query ):

    def get_from_clause(self):
        r = super( nQuery, self).get_from_clause()
        r[0].append(r", plainto_tsquery(%s) as query" )
        r[1].append(r"'%s'" % (words,) )
        return r

    def full_text( self ):
        select={
            'headline': "ts_headline( %s, query,'StartSel = <strong>, StopSel = </strong>')" % ( 'content', ),
            'rank': "ts_rank_cd(tsv, query, 32)",
        }
        self.add_extra( select,None,('tsv @@ query',),None,None,None )

q = nQuery( Article, connection )
q.full_text()
from django.db.models.query import QuerySet
items = QuerySet( Article, q )

I was wondering if it would be very difficult to implement the full-text ability of postgreSQL in Django. I'd like to help or do it by myself, but I would probably need some help at some point.

Note: See TracTickets for help on using tickets.
Back to Top