Opened 5 years ago

Closed 5 years ago

Last modified 4 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 Changed 5 years ago by Tim Graham

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 Changed 5 years ago by Simon Charette

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 5 years ago by Simon Charette (previous) (diff)

comment:3 Changed 5 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:4 Changed 5 years ago by Simon Charette <charette.s@…>

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.

comment:5 in reply to:  2 Changed 4 years ago by Andrew Brown

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 Changed 4 years ago by Simon Charette

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 Changed 4 years ago by Simon Charette

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 Changed 4 years ago by Simon Charette

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

comment:9 Changed 4 years ago by Mariusz Felisiak <felisiak.mariusz@…>

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 Changed 4 years ago by Mariusz Felisiak <felisiak.mariusz@…>

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