Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29582 closed Bug (fixed)

SearchVector doesn't support querying non-text fields

Reported by: mickaelmarin Owned by: nobody
Component: contrib.postgres Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

I try to implement autocomplete on my place table that look like below:

class Place(models.Model):
    name = models.CharField(max_length=80, blank=True)
    num = models.IntegerField(null=True)
    street = models.CharField(max_length=100, blank=True)
    postal_code = models.IntegerField(null=True)
    district = models.CharField(max_length=30, blank=True)
    city=models.CharField(max_length=30, blank=True)
    country = models.CharField(max_length=30, blank=True)
    domicile = models.BooleanField(default=False)

    def __str__(self):
        address = "{0}, {1}, {2}, {3}".format(self.num, self.street, 
                                              self.postal_code, self.city, self.country)
        return address

and in my function that return the result for display my address on frontend is like (self.q is a GET param from ajax call):

qs = Place.objects.all()
qs.annotate(search=SearchVector('street', 'country','city', 'num')).filter(search__icontains=self.q)

I have this error when the ajax call run:

  File "/opt/coupdepouce/lib/python3.6/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
django.db.utils.DataError: invalid input syntax for integer: ""
LINE 1: ...city", '') || ' ' || COALESCE("user_place"."num", '')) AS "s...

Change History (10)

comment:1 by Tim Graham, 6 years ago

Component: Uncategorizedcontrib.postgres
Description: modified (diff)
Summary: postgresql SearchVector error when try to add integerfield to itSearchVector doesn't support querying non-text fields
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

The problem seems to be hardcoded Value(''), regardless of the field type. I'm not sure what the proper resolution is.

comment:2 by Simon Charette, 6 years ago

Has patch: set
Version: 1.11master

Not sure why the || operator was used with coalesce here; concat already does a good job at ignoring NULL values and casting non-text arguments.

https://github.com/django/django/pull/10209/

Last edited 6 years ago by Simon Charette (previous) (diff)

comment:3 by Tim Graham, 6 years ago

Triage Stage: AcceptedReady for checkin

comment:4 by Simon Charette <charette.s@…>, 6 years ago

Resolution: fixed
Status: newclosed

In 1a28dc3:

Fixed #29582 -- Fixed a crash when using SearchVector with non text-fields.

The PostgreSQL concat() function handles nulls and non-text values better than

the
operator.

in reply to:  2 comment:5 by Andrew Brown, 6 years ago

Replying to Simon Charette:

The non-obvious issue with this change is that you can't use the concat() function in a Postgresql index, as you can't use functions not marked as IMMUTABLE. Django makes a note of this in an unrelated part of the docs. So it's not possible to create an index that is usable by this new SearchVector. I'm guessing this is why the Postgresql docs all use the || operator along with COALESCE() (see the examples on: https://www.postgresql.org/docs/current/textsearch-tables.html )

I suppose a new bug should be created, but I'm not sure if this is fixable. Perhaps just update the docs on the postgresql search page with some hints or suggestions on this limitation. In particular, this sentence is no longer accurate:

In the event that all the fields you’re querying on are contained within one particular model, you can create a functional index which matches the search vector you wish to use.

comment:6 by Simon Charette, 6 years ago

Andrew, thanks for chiming in and reporting this issue.

I guess an alternate solution would be to keep wrapping the source_expressions in Coalesce but use an extra Cast(expr, CharField) if not isinstance(expr.output_field, (CharField, TextField) and revert to using the || operator. I think that should work unless I'm missing something.

Please file a new issue and mark the severity to Release Blocker against 2.2. Feel free to assign the issue to charettes as I should have a few hours to dedicate to that in the next few days.

comment:7 by Simon Charette, 6 years ago

Andrew, I just submitted a patch that reverts the function to use the || operation and add a regression test to make sure the generated expression is usable for a functional GIN index if a config is generated. Please link to it when you create the ticket.

comment:8 by Simon Charette, 6 years ago

Andrew, I went ahead and filed #30385 to track the issue.

comment:9 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

In 405c836:

Fixed #30385 -- Restored SearchVector(config) immutability.

Regression in 1a28dc3887e8d66d5e3ff08cf7fb0a6212b873e5.

The usage of CONCAT to allow SearchVector to deal with non-text fields
made the generated expression non-IMMUTABLE which prevents a functional
index to be created for it.

Using a combination of COALESCE and ::text makes sure the expression
preserves its immutability.

Refs #29582. Thanks Andrew Brown for the report, Nick Pope for the
review.

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

In 88bf635c:

[2.2.x] Fixed #30385 -- Restored SearchVector(config) immutability.

Regression in 1a28dc3887e8d66d5e3ff08cf7fb0a6212b873e5.

The usage of CONCAT to allow SearchVector to deal with non-text fields
made the generated expression non-IMMUTABLE which prevents a functional
index to be created for it.

Using a combination of COALESCE and ::text makes sure the expression
preserves its immutability.

Refs #29582. Thanks Andrew Brown for the report, Nick Pope for the
review.

Backport of 405c8363362063542e9e79beac53c8437d389520 from master

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