Opened 5 years ago

Closed 3 years ago

Last modified 2 weeks ago

#30446 closed Cleanup/optimization (fixed)

Automatically resolve Value's output_field for stdlib types.

Reported by: Ozan Gerdaneri Owned by: Simon Charette
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: SearchVector, SearchVectorField, Value
Cc: Matt Westcott, Sergey Fedoseev 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

Hi,
I have a model of AModel. AModel has a SearchVectorField named search_vector. I want to update this vector by indexing a string that is not in any other field.

from django.db.models import Value
from django.contrib.postgres.search import SearchVector

AModel.objects.filter(pk=1).update(search_vector=SearchVector(Value("a string to be indexed and inserted to search_vector field")))

This code generates this error:

FieldError: Cannot resolve expression type, unknown output_field

It seemed to be a bug since I found similar usages in forums..

Change History (20)

comment:1 Changed 5 years ago by Ozan Gerdaneri

Summary: SerachVectorField cannot be updated pure stringSerachVectorField cannot be updated with pure string

comment:2 Changed 5 years ago by Simon Charette

Summary: SerachVectorField cannot be updated with pure stringAutomatically resolve Value's output_field for stdlib types.
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Specifying an explicit output_field for Value should resolve your issue.

value = Value(
    "a string to be indexed and inserted to search_vector field",
    output_field=models.TextField(),
)
AModel.objects.filter(pk=1).update(
    search_vector=SearchVector(value),
)

I guess Value._resolve_output_field could be made smarter for some stdlib types such as str, float, int, Decimal, date, datetime so I'm tentatively accepting on this basis.

comment:3 Changed 5 years ago by Simon Charette

Owner: changed from nobody to Simon Charette
Status: newassigned

comment:4 Changed 5 years ago by Simon Charette

Has patch: set

comment:5 Changed 5 years ago by Simon Charette

Patch needs improvement: set

comment:6 Changed 4 years ago by Matt Westcott

Cc: Matt Westcott added

In case it hasn't already been noted - the example above did previously work in 2.2, and the error is a regression in 2.2.1, specifically this commit: https://github.com/django/django/commit/88bf635c356b4d3a47e88dc4142b90060ce3c2ef . As such, I think this might justify being upgraded to a bug.

(FWIW, we encountered this independently here: https://github.com/wagtail/wagtail/issues/5547)

comment:7 Changed 3 years ago by Simon Charette

Patch needs improvement: unset

comment:8 Changed 3 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In ea3beb4:

Refs #30446 -- Defined default output_field of text database functions.

This prevented the default behavior of
BaseExpression._resolve_output_field from error'ing out when such
functions accepted both expressions from mixed types
(e.g. SubStr(CharField, IntegerField, IntegerField)).

comment:9 Changed 3 years ago by Sergey Fedoseev

Cc: Sergey Fedoseev added

comment:10 Changed 3 years ago by Mariusz Felisiak

Patch needs improvement: set

comment:11 Changed 3 years ago by Simon Charette

Patch needs improvement: unset

comment:12 Changed 3 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In 1b7623fd:

Refs #30446 -- Defined output_field of BoundingCircle() GIS database function.

This prevented the default behavior of
BaseExpression._resolve_output_field from error'ing out when such
functions accepted both expressions from mixed types
(e.g. BoundingCircle(Polygon, num_seg=12)).

comment:13 Changed 3 years ago by Mariusz Felisiak

Triage Stage: AcceptedReady for checkin
Version: 2.2master

comment:14 Changed 3 years ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 1e38f11:

Fixed #30446 -- Resolved Value.output_field for stdlib types.

This required implementing a limited form of dynamic dispatch to combine
expressions with numerical output. Refs #26355 should eventually provide
a better interface for that.

comment:15 Changed 3 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In 156a2138:

Refs #30446 -- Removed unnecessary Value(..., output_field) in docs and tests.

comment:16 Changed 3 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In 40894f2:

Refs #30446 -- Added tests for resolving output_field of CombinedExpression.

comment:17 Changed 3 weeks ago by Mariusz Felisiak <felisiak.mariusz@…>

In 8b1acc04:

Refs #30446, Refs #34944 -- Fixed crash when adding GeneratedField with string Value().

This should allow smarter output_field inferring in functions dealing
with text expressions.

Regression in f333e3513e8bdf5ffeb6eeb63021c230082e6f95.

comment:18 Changed 3 weeks ago by Mariusz Felisiak <felisiak.mariusz@…>

In 73869a51:

[5.0.x] Refs #30446, Refs #34944 -- Fixed crash when adding GeneratedField with string Value().

This should allow smarter output_field inferring in functions dealing
with text expressions.

Regression in f333e3513e8bdf5ffeb6eeb63021c230082e6f95.

Backport of 8b1acc0440418ac8f45ba48e2dfcf5126c83341b from main

comment:19 Changed 2 weeks ago by Mariusz Felisiak <felisiak.mariusz@…>

In de4884b1:

Reverted "Refs #30446, Refs #34944 -- Fixed crash when adding GeneratedField with string Value()."

This reverts commit 8b1acc0440418ac8f45ba48e2dfcf5126c83341b.

comment:20 Changed 2 weeks ago by Mariusz Felisiak <felisiak.mariusz@…>

In 5b1d0a6:

[5.0.x] Reverted "Refs #30446, Refs #34944 -- Fixed crash when adding GeneratedField with string Value()."

This reverts commit 8b1acc0440418ac8f45ba48e2dfcf5126c83341b.

Backport of de4884b114534f43c49cf8c5b7f10181e737f4e9 from main

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