Code

Opened 3 years ago

Last modified 5 months ago

#15559 new Bug

Distinct queries will cause errors with some custom model fields

Reported by: urandom Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: mir Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Starting with 1.3 rc1, some queries are marked as distinct by the system.

If a model now uses a custom field, that relies on a special DB column (like Postgresql's 'point' column),
the database will immediately throw an error, since such a column cannot be compared, and thus unique values cannot be found.

It would probably be a good idea to allow fields to specify whether they cannot be distinct:
custom = MyCustomField(distinctless=True)

which would turn this ordinary distinct query:
SELECT DISTINCT id, number, custom FROM foo;

into:
SELECT DISTINCT ON(id, number) FROM FOO;

Discussion link:
https://groups.google.com/forum/#!topic/django-developers/iO8z3iLpgg4

Attachments (0)

Change History (15)

comment:1 Changed 3 years ago by mir

  • Cc mir added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by kmtracey

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Keywords regression blocker added
  • milestone set to 1.3
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.3-beta to 1.3-rc1

Not sure we want to be adding a new option between now and 1.3 final, but seems like we need to do something (minimum document the change as a backwards-incompatibility?).

comment:3 follow-up: Changed 3 years ago by lukeplant

We could also revert the fix in [15607], and either:

  • leave #11707 open
  • Fix #11707 by doing duplicate elimination in Python

comment:4 Changed 3 years ago by ikelly

I discovered another problem with the distinct queries from [15607]. Using Oracle, it breaks foreign keys to models that have TextFields. This is because Oracle won't let you do SELECT DISTINCT with a select list that includes a LOB column.

+1 for reverting.

comment:5 follow-up: Changed 3 years ago by mir

ikelly, does Oracle support SELECT DISTINCT ON (id) if there's a LOB column in the SELECT list?

comment:6 in reply to: ↑ 5 Changed 3 years ago by ikelly

Replying to mir:

ikelly, does Oracle support SELECT DISTINCT ON (id) if there's a LOB column in the SELECT list?

No. The analytic query that emulates DISTINCT ON works using a regular DISTINCT query and selecting for each column the first value in that column for the corresponding DISTINCT ON partition in place of the actual value for the row. The result is that rows that are equivalent over the DISTINCT ON partition become equivalent over the entire row and so are filtered out by the DISTINCT keyword. That means you're still effectively doing a SELECT DISTINCT with a LOB column.

comment:7 Changed 3 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from new to closed

In [15791]:

Fixed #15559 - distinct queries introduced by [15607] cause errors with some custom model fields

This patch just reverts [15607] until a more satisfying solution can be
found.

Refs #11707

comment:8 Changed 3 years ago by lukeplant

In [15792]:

[1.2.X] Fixed #15559 - distinct queries introduced by [15607] cause errors with some custom model fields

This patch just reverts [15607] until a more satisfying solution can be
found.

Refs #11707

Backport of [15791] from trunk.

comment:9 Changed 3 years ago by lukeplant

As you can see, I decided to revert [15607], because of the support for reverting listed above and on the mailing list, and because of the closeness of the 1.3 release.

comment:10 follow-up: Changed 3 years ago by lukeplant

  • Severity set to Normal
  • Type set to Uncategorized

There are other places where we are using distinct (see [15526]), so it is possible that this bug is only fixed in the one instance where the reporter experienced a regression. A more satisfying general solution is probably still needed, but a new bug should be opened for that.

comment:11 in reply to: ↑ 10 Changed 3 years ago by carljm

  • Keywords regression blocker removed
  • milestone 1.3 deleted
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Type changed from Uncategorized to Bug
  • Version changed from 1.3-rc1 to SVN

Replying to lukeplant:

There are other places where we are using distinct (see [15526]), so it is possible that this bug is only fixed in the one instance where the reporter experienced a regression. A more satisfying general solution is probably still needed, but a new bug should be opened for that.

I don't see any need to open a new bug - it would just be a copy-paste of this one. The description for this bug is still fully applicable (and was never fixed, just worked around). Reopening and removing 1.3-blocker keywords.

comment:12 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:13 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:14 Changed 16 months ago by aaugustin

  • Status changed from reopened to new

comment:15 in reply to: ↑ 3 Changed 5 months ago by charstring

Replying to lukeplant:

We could also revert the fix in [15607], and either:

  • leave #11707 open
  • Fix #11707 by doing duplicate elimination in Python

I'm revisiting #11707 on a sprint. I implemented the duplicate elimination 2 years ago with ojii, but it never got RFC'ed.

Since then I thought: DISTINCT logically equivalent with a GROUP BY on all fields. Not all databases support DISTINCT ON, but a GROUP BY on just the pk should do the trick (at least in #11707).

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.