Opened 2 years ago

Closed 5 weeks ago

#33517 closed Bug (fixed)

Extracting seconds also returns fractional seconds on PostgreSQL and Oracle.

Reported by: Joey Lange Owned by: Hisham Mahmood
Component: Database layer (models, ORM) Version: 4.0
Severity: Normal Keywords: oracle postgresql
Cc: Mohamed Nabil Rady Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In the Django documentation (https://docs.djangoproject.com/en/4.0/ref/models/querysets/#second) the __second extractor is implied to be dealing with integer values for second in a date time. In at least PostgreSQL, the values from EXTRACTing second from a date time will include "any fractional seconds" (https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-EXTRACT) which means it could be more specific than an integer.

Because of the implication in Django's documentation, I had expected this filter to work as expected for a row whose date_created and date_modified are within a second of each other but a couple of milliseconds off:

.filter(date_created__second=F("date_modified__second"))

However, that ends up not being true given the Postgres behavior.

My recommendation is merely a documentation update to highlight the discrepancy of behavior between a value coalesced to Python from the DB data, and how the column values are perceived on the database side within a query. I'm struggling for adequate language to explain in such a context, though :-P happy to discuss further and work together toward this improvement.

Change History (20)

comment:1 by Mariusz Felisiak, 2 years ago

Component: DocumentationDatabase layer (models, ORM)
Summary: Implied behavior of `__second` extractor inside of `F()` expressions does not match documented behavior for at least PostgreSQLExtracting seconds also returns fractional seconds on PostgreSQL.
Triage Stage: UnreviewedAccepted

Thanks for the report! We should have consistent behavior on all backends. What do you think about adding DATE_TRUNC() in this case? For example:

  • django/db/backends/postgresql/operations.py

    diff --git a/django/db/backends/postgresql/operations.py b/django/db/backends/postgresql/operations.py
    index 399c1b24e7..fd90b95fb8 100644
    a b class DatabaseOperations(BaseDatabaseOperations):  
    3636            return "EXTRACT('isodow' FROM %s)" % field_name
    3737        elif lookup_type == 'iso_year':
    3838            return "EXTRACT('isoyear' FROM %s)" % field_name
     39        elif lookup_type == 'second':
     40            return f"EXTRACT('second' FROM DATE_TRUNC('second', {field_name}))"
    3941        else:
    4042            return "EXTRACT('%s' FROM %s)" % (lookup_type, field_name)
    4143

comment:2 by Mohamed Nabil Rady, 2 years ago

Cc: Mohamed Nabil Rady added
Easy pickings: set
Has patch: set
Owner: changed from nobody to Mohamed Nabil Rady
Status: newassigned
Triage Stage: AcceptedReady for checkin

comment:3 by Tim Graham, 2 years ago

Easy pickings: unset
Triage Stage: Ready for checkinAccepted

"Ready for checkin" is set by a patch reviewer (and there's no need to set "Easy pickings").

comment:4 by Mariusz Felisiak, 2 years ago

Patch needs improvement: set

comment:5 by Mariusz Felisiak, 2 years ago

Summary: Extracting seconds also returns fractional seconds on PostgreSQL.Extracting seconds also returns fractional seconds on PostgreSQL and Oracle.

comment:6 by Mariusz Felisiak, 2 years ago

Keywords: oracle postgresql added
Patch needs improvement: unset

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In b7f26355:

Refs #33517 -- Prevented second lookup from returning fractional seconds on PostgreSQL.

comment:8 by Mariusz Felisiak, 2 years ago

Has patch: unset
Owner: Mohamed Nabil Rady removed
Status: assignednew

This is still an issue on Oracle. We could use EXTRACT(SECOND FROM CAST({field_name} AS TIMESTAMP(0))) but it won't work with DurationField.

comment:9 by Yash Singhal, 2 years ago

Owner: set to Yash Singhal
Status: newassigned

comment:10 by Joey Lange, 2 years ago

Great suggestion, Mariusz! You've convinced me that consistent and clear behavior is more important here! Let me know if I can be of assistance as this moves forward -- sorry I missed the earlier notification(s).

comment:11 by Yash Singhal, 2 years ago

hey joey Lange, I'm interested in solving this issue, can I solve it, or its already been assigned to someone?

comment:12 by Joey Lange, 2 years ago

Looks like we're already PR'd and merged, pal! Thanks for the initiative, though!

https://github.com/django/django/pull/15450

comment:13 by Mariusz Felisiak, 2 years ago

This is still an issue on Oracle, see comment.

comment:14 by Yash Singhal, 2 years ago

@joey Lange, "it is still an issue on oracle" as told by @Mariusz Felisiak, so can I work on this?

comment:15 by Joey Lange, 2 years ago

Truthfully I'm just the reporter of this issue, I have no official stake in the maintenance of Django as a whole -- so, go ahead, though I'm not necessarily the person (if there is one) to be giving permission.

comment:16 by Carlton Gibson, 2 years ago

Hi Yash — you don't need permission. Please do take a look! Thanks

comment:17 by Yash Singhal, 2 years ago

Hey Mariusz Felisiak, can you please guide me, like on which file I can found this and what the code is look like and do I need to create a test too? As I'm new to it so I have a little knowledge about this.

Last edited 2 years ago by Yash Singhal (previous) (diff)

comment:18 by Hisham Mahmood, 5 weeks ago

Has patch: set
Owner: changed from Yash Singhal to Hisham Mahmood

comment:19 by GitHub <noreply@…>, 5 weeks ago

In bbfbf0a:

Refs #33517 -- Prevented second lookup from returning fractional seconds on Oracle.

comment:20 by Mariusz Felisiak, 5 weeks ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top