Opened 5 years ago

Closed 4 years ago

Last modified 5 months 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 by Ozan Gerdaneri, 5 years ago

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

comment:2 by Simon Charette, 5 years ago

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

Owner: changed from nobody to Simon Charette
Status: newassigned

comment:4 by Simon Charette, 5 years ago

Has patch: set

comment:5 by Simon Charette, 5 years ago

Patch needs improvement: set

comment:6 by Matt Westcott, 5 years ago

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

Patch needs improvement: unset

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

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 by Sergey Fedoseev, 4 years ago

Cc: Sergey Fedoseev added

comment:10 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set

comment:11 by Simon Charette, 4 years ago

Patch needs improvement: unset

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

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 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin
Version: 2.2master

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

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

In 156a2138:

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

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 40894f2:

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

comment:17 by Mariusz Felisiak <felisiak.mariusz@…>, 6 months ago

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 by Mariusz Felisiak <felisiak.mariusz@…>, 6 months ago

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 by Mariusz Felisiak <felisiak.mariusz@…>, 5 months ago

In de4884b1:

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

This reverts commit 8b1acc0440418ac8f45ba48e2dfcf5126c83341b.

comment:20 by Mariusz Felisiak <felisiak.mariusz@…>, 5 months ago

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