Opened 7 years ago

Closed 6 years ago

Last modified 2 years ago

#10142 closed (fixed)

Doctests failing on aggregation

Reported by: Vinay Sajip <vinay_sajip@…> Owned by:
Component: Database layer (models, ORM) Version: master
Severity: Keywords: doctest failure
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I'm getting a failure on running the test suite. I looked for an existing ticket with this problem, but didn't see one; sorry if I missed it.

Environment
===========
This is a checkout of SVN rev 9791.
OS: Ubuntu 2.6.15-53-386 #1 PREEMPT Mon Nov 24 17:50:35 UTC 2008 i686 GNU/Linux
Python: Python 2.4.3 (#2, Jul 31 2008, 21:56:52) [GCC 4.0.3 (Ubuntu 4.0.3-1ubuntu5)] on linux2
Database: postgresql (in settings.py), PostgreSQL 8.2 on Windows
Test Output
===========
FAIL: Doctest: regressiontests.aggregation_regress.models.__test__.API_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.4/site-packages/django/test/_doctest.py", line 2180, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for regressiontests.aggregation_regress.models.__test__.API_TESTS
  File "/home/vinay/projects/django/upstream/tests/regressiontests/aggregation_regress/models.py", line unknown line number, in API_TESTS

----------------------------------------------------------------------
File "/home/vinay/projects/django/upstream/tests/regressiontests/aggregation_regress/models.py", line ?, in regressiontests.aggregation_regress.models.__test__.API_TESTS
Failed example:
    Book.objects.aggregate(StdDev('pages'))
Expected:
    {'pages__stddev': 311.46...}
Got:
    {'pages__stddev': 341.19344464199003}
----------------------------------------------------------------------
File "/home/vinay/projects/django/upstream/tests/regressiontests/aggregation_regress/models.py", line ?, in regressiontests.aggregation_regress.models.__test__.API_TESTS
Failed example:
    Book.objects.aggregate(StdDev('price'))
Expected:
    {'price__stddev': 24.16...}
Got:
    {'price__stddev': 26.467595848508797}
----------------------------------------------------------------------
File "/home/vinay/projects/django/upstream/tests/regressiontests/aggregation_regress/models.py", line ?, in regressiontests.aggregation_regress.models.__test__.API_TESTS
Failed example:
    Book.objects.aggregate(Variance('pages'))
Expected:
    {'pages__variance': 97010.80...}
Got:
    {'pages__variance': 116412.96666666666}
----------------------------------------------------------------------
File "/home/vinay/projects/django/upstream/tests/regressiontests/aggregation_regress/models.py", line ?, in regressiontests.aggregation_regress.models.__test__.API_TESTS
Failed example:
    Book.objects.aggregate(Variance('price'))
Expected:
    {'price__variance': 583.77...}
Got:
    {'price__variance': 700.53363000000002}


----------------------------------------------------------------------
Ran 594 tests in 634.307s

FAILED (failures=1)

Change History (10)

comment:1 Changed 7 years ago by ikelly

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

This appears to be an old postgresql bug where stddev and variance were computed incorrectly:

http://archives.postgresql.org/pgsql-jdbc/2007-07/msg00033.php

The bug does not occur with postgresql 8.2.11, so I surmise it was fixed some time before then.

comment:2 Changed 7 years ago by ikelly

Er, this was the link I intended to include:

http://archives.postgresql.org/pgsql-bugs/2007-07/msg00046.php

comment:3 Changed 7 years ago by russellm

  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Unreviewed to Accepted

Based on Ian's link, this certainly appears to fall into the 'update your version of Postgres' basket. However, we should document the exact version(s) of Postgres that might be affected by this bug. We already document a number of known problems with SQLite and MySQL, so we shouldn't leave Postgres out of the fun :-)

Looking at the Postgres release notes, it looks like this was introduced in 8.2 and fixed in 8.2.5

Vinay - can you confirm that this matches you Postgres installation (i.e., you have an installed version somewhere between 8.2 and 8.2.4)?

comment:4 Changed 7 years ago by mtredinnick

This is fairly dangerous for users (cool bug, though!). It will appear to work and give wildly incorrect answers. When postgreSQL gets it wrong, they clearly don't stoop to half-measure. This isn't exactly a rounding error.

Most of the database-specific problems we've got manifest themselves as fatal errors when you encounter them (all the SQLite ones do. The MySQL collation issue is the only trappy case that doesn't). This one is going to be much harder to detect.

We might be best served by adding a "supports variance" DatabaseOperations check that returns True always, except for particular PostgreSQL versions. Then we can error out, fatally, if code encounters that. I'm really worried about quietly succeeding and returning the wrong answer and nobody until far too late (the numbers aren't so wrong that they'll look intuitively broken) . PostgreSQL 8.2 is hardly the most uncommon server on the planet.

comment:5 Changed 7 years ago by mtredinnick

By the way, for comparison with our existing practices, I'll point to django/db/backends/oracle/base.py have a look at regex_lookup_9() (a.k.a "no regular expressions for you") and regex_lookup_10(). That's actually a case of us being nice, since we could just continue on and error out later. But I think the same approach can be used in this case.

comment:6 Changed 6 years ago by Koen Biermans <koen.biermans@…>

For your information, not all sqlite bugs give fatal errors. There is a testcase on ticket #10113, where an old sqlite version (v3.3.4) was giving incorrect results for an aggregation (without raising an error, just returning a wrong result).

comment:7 Changed 6 years ago by Vinay Sajip <vinay_sajip@…>

Replying to russellm:

Looking at the Postgres release notes, it looks like this was introduced in 8.2 and fixed in 8.2.5

Vinay - can you confirm that this matches you Postgres installation (i.e., you have an installed version somewhere between 8.2 and 8.2.4)?

Yes, I can confirm that the failing Postgres version is 8.2 (no point release) on Windows Server 2003. I will upgrade this machine's Postgres to 8.3.5 shortly.

+1 to Malcolm's idea about failing fast rather than returning results which are plausible but incorrect. The wrong answers weren't that far off the correct ones, so I suspect a casual sanity check wouldn't help in many cases.

comment:8 Changed 6 years ago by russellm

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

(In [9804]) Fixed #10142 -- Added docs and an exploding error message to highlight an error present in the implementation of STDDEV_POP and VAR_POP in PostgreSQL 8.2-8.2.4 that will give incorrect answers (the database function mistakenly returns sample, rather than population deviation/variance). Thanks to Vinay Sajip <vinay_sajip@…> for the report, and Ian Kelly for pointing out the cause of the problem.

comment:9 Changed 6 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:10 Changed 2 years ago by akaariai

  • Component changed from ORM aggregation to Database layer (models, ORM)
Note: See TracTickets for help on using tickets.
Back to Top