Re: [Django] #27732: django.contrib.postgres.search SearchRank doesn't handle SearchVectorField references

[Django]

#27732: django.contrib.postgres.search SearchRank doesn't handle 
SearchVectorField
references
--+
 Reporter:  Matt Hoskins  |Owner:  (none)
 Type:  Bug   |   Status:  new
Component:  contrib.postgres  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by Andrii Soldatenko):

 What's the next steps, because I've found also this issue, and I reinvent
 the same workaround as Marc showed with wrapping using `F`.
 I think we should at least fix documentation to not confuse users or fix
 behavior. Any thoughts?

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/070.d74705c2acbe6c67c9407e11e238987c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #27732: django.contrib.postgres.search SearchRank doesn't handle SearchVectorField references

[Django]

#27732: django.contrib.postgres.search SearchRank doesn't handle 
SearchVectorField
references
--+
 Reporter:  Matt Hoskins  |Owner:  (none)
 Type:  Bug   |   Status:  new
Component:  contrib.postgres  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by Tim Graham):

 * stage:  Unreviewed => Accepted


Comment:

 Tentatively accepting, even though it seems some design decisions remain.
 If no code changes are made, perhaps the documentation could be clarified.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/070.1d96bdbd8d0a68f54e07c080e0ec752b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #27732: django.contrib.postgres.search SearchRank doesn't handle SearchVectorField references

[Django]

#27732: django.contrib.postgres.search SearchRank doesn't handle 
SearchVectorField
references
--+--
 Reporter:  Matt Hoskins  |Owner:  (none)
 Type:  Bug   |   Status:  new
Component:  contrib.postgres  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Unreviewed
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+--

Comment (by Matt Hoskins):

 After further musing and taking into account:
   * The documentation describes passing in a vector and a query into
 SearchRank
   * The example included with the above shows passing in SearchVector and
 a SearchQuery objects only (and not strings)
   * Marc's point "I think supporting the simple string version in both
 cases (when it is or is not a SearchVector) already is somewhat confusing
 personally"
   * My feeling that the intuitive way to reference a SearchVectorField as
 the first argument to SearchRank would be to pass in a string (given other
 functions, like the DateTime ones, accept a string if you want to
 reference a field of the correct type, although I note that they do list
 their first argument as "expression").
   * The DateTime functions use resolve_expression to validate the type
 that their first argument resolves into
   * It's possible in postgresql to have a column of type tsquery - I can't
 think why anyone would ever want to (no use case seems likely), but there
 would be some logical consistency in allowing the second argument of
 SearchRank to be a reference to a related query column via the potential
 to implement SearchQueryField

 ... then one course of action that springs to mind is to deprecate passing
 in strings to SearchRank except where a string for vector resolves into a
 SearchVectorField:
   * Perhaps apply my patch in a point release (although it's possible, as
 Marc suggested, to work around using "F()" for now)
   * In the next version (minor/major) use my patch but also emit a
 deprecation warning if the resolving of the vector argument doesn't result
 in a SearchVectorField type result and also emit a deprecation warning if
 the query argument isn't a SearchQuery
   * For the documentation of SearchVectorField perhaps mention that a
 string can be passed in to reference one in SearchRank to make it clear
 that's how to do it
   * Once deprecation is complete perhaps check the types of what the
 vector and query arguments resolve into and, like with the DateTime
 functions, raise ValueError if they don't resolve into the correct type.

 There may be good reasons for adopting a different approach of course
 (such as the simplest case of just documenting Marc's "work around" as the
 official way to reference a SearchVectorField as the vector argument to
 SearchRank), these are just my musings!

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/070.8184bcb611bd6a0f6bdd4f43abbb94ce%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #27732: django.contrib.postgres.search SearchRank doesn't handle SearchVectorField references

[Django]

#27732: django.contrib.postgres.search SearchRank doesn't handle 
SearchVectorField
references
--+--
 Reporter:  Matt Hoskins  |Owner:  (none)
 Type:  Bug   |   Status:  new
Component:  contrib.postgres  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Unreviewed
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+--

Comment (by Matt Hoskins):

 The example for SearchRank in the documentation does only shows passing in
 a SearchVector() as the first argument to SearchRank and a SearchQuery()
 for the second so I was a little surprised when looking at the code to
 find it was behaving such that if you passed in a string for the first
 argument that it assumed you'd just missed off wrapping it in a
 SearchVector() rather than it potentially being a reference to a
 SearchVectorField.

 If SearchRank were left as is (i.e. not supporting passing in a string
 which references a SearchVectorField) then I think it would be useful for
 the documentation for SearchVectorField to set out the F() "work around"
 as the way to use a SearchVectorField in SearchRank. E.g. just have a bit
 saying "To use a SearchVectorField in SearchRank then do..." and given an
 example too. It takes a little bit of figuring out otherwise (particularly
 as the documentation makes no mention that you can pass in a string as the
 first argument and it'll wrap it in a SearchVector for you) and I have to
 admit wrapping it in F() to prevent that behaviour is not a leap I'd made
 yet as most database functions in django just take field references as
 strings if the type of the field is already suitable.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/070.e7300ff590949ff53ac886b03e656efd%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #27732: django.contrib.postgres.search SearchRank doesn't handle SearchVectorField references

[Django]

#27732: django.contrib.postgres.search SearchRank doesn't handle 
SearchVectorField
references
--+--
 Reporter:  Matt Hoskins  |Owner:  (none)
 Type:  Bug   |   Status:  new
Component:  contrib.postgres  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Unreviewed
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+--

Comment (by Marc Tamlyn):

 I had to make a choice as to the "default" behaviour of SearchRank (and
 for that matter SearchVector). I made the latter match the former - so
 that the query `SearchRank('body_text', SearchQuery('django'))` worked. I
 think supporting the simple string version in both cases (when it is or is
 not a `SearchVector`) already is somewhat confusing personally, especially
 as I believe the "work around" is:

 `SearchRank(F('dialogue_search_vector'), SearchQuery('brave sir robin'))`

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/070.d1dc74eddfb9a96cde3f0fea9e1250a1%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #27732: django.contrib.postgres.search SearchRank doesn't handle SearchVectorField references

[Django]

#27732: django.contrib.postgres.search SearchRank doesn't handle 
SearchVectorField
references
--+--
 Reporter:  Matt Hoskins  |Owner:  (none)
 Type:  Bug   |   Status:  new
Component:  contrib.postgres  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Unreviewed
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+--
Changes (by Simon Charette):

 * cc: Marc Tamlyn (added)
 * version:  1.10 => master


Comment:

 Marc, any thoughts on that?

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/070.17200bc15b2134e0b76bf034f0c82f39%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #27732: django.contrib.postgres.search SearchRank doesn't handle SearchVectorField references

[Django]

#27732: django.contrib.postgres.search SearchRank doesn't handle 
SearchVectorField
references
--+--
 Reporter:  matt-hoskins  |Owner:  (none)
 Type:  Bug   |   Status:  new
Component:  contrib.postgres  |  Version:  1.10
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Unreviewed
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+--
Changes (by matt-hoskins):

 * Attachment "searchrankvectorfieldfix.diff" added.


--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/070.c857240f98842c8836cbcdc5d97d7c95%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

[Django] #27732: django.contrib.postgres.search SearchRank doesn't handle SearchVectorField references

[Django]

#27732: django.contrib.postgres.search SearchRank doesn't handle 
SearchVectorField
references
+
   Reporter:  matt-hoskins  |  Owner:  (none)
   Type:  Bug   | Status:  new
  Component:  contrib.postgres  |Version:  1.10
   Severity:  Normal|   Keywords:
   Triage Stage:  Unreviewed|  Has patch:  0
Needs documentation:  0 |Needs tests:  0
Patch needs improvement:  0 |  Easy pickings:  0
  UI/UX:  0 |
+
 For search performance I'm using SearchVectorField on a model. However if
 I reference that field in the vector argument to SearchRank I get an
 error. This is because SearchRank always wraps a vector argument that
 lacks a "resolve_expression" attribute as a SearchVector even if it would
 end up resolving to a SearchVectorField.

 If I modify tests/postgres_tests/test_search.py and add the following to
 the SearchVectorFieldTest class:
 {{{#!python
 def test_existing_vector_ranking(self):
 Line.objects.update(dialogue_search_vector=SearchVector('dialogue'))
 searched = Line.objects.filter(character=self.minstrel).annotate(
 rank=SearchRank('dialogue_search_vector', SearchQuery('brave
 sir robin')),
 ).order_by('rank')
 self.assertSequenceEqual(searched, [self.verse2, self.verse1,
 self.verse0])
 }}}

 Then this results in the following error:
 {{{
 ProgrammingError: function to_tsvector(tsvector) does not exist
 LINE 1: ... "postgres_tests_line"."dialogue_config", ts_rank(to_tsvecto...
  ^
 HINT:  No function matches the given name and argument types. You might
 need to add explicit type casts.
 }}}

 As the enforced wrapping of the vector argument in a SearchVector is then
 causing the sql to wrap the tsvector field in a coalesce (which doesn't
 cause an issue) and then a to_tsvector call.

 I've not delved into the Expressions code before (my last rummaging around
 in the bowels of the query code was a few years ago) but from my quick
 look at existing code and the APIs then perhaps a solution is for
 SearchRank to not wrap the vector argument as it comes into __init__ but
 instead have a resolve_expression call which checks the first resolved
 expression on the result of calling super's resolve_expression is or isn't
 of type SearchVectorField and then if it isn't then resolve a wrapped
 version.

 I've attached a patch which both adds the test I mention above (it may be
 that it should live in the rank test rather than the search vector field
 test, or have its own test class which covers the combination of both) and
 also modifies SearchRank in the way I've suggested above. The postgres
 tests pass on my test system with this patch.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/055.dec0dc5f540ea7cd185568a67565177d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.