Bug on sqlite __year comparison

A patch that fixes sqlite year lookup

Shouldn't the format modifier after converting to int (line 512) to be %d, not %s on line 515?

Replying to jarek.zgoda@gmail.com:

Shouldn't the format modifier after converting to int (line 512) to be %d, not %s on line 515?

Maybe, but I cut code from base classes, and I think it was for a reason

For no apparent reason. Python does str() on the int object in such case so this doesn't end with ValueError?.

Pulling this back; not clear if we can repeat this without the patch on current trunk, really.

msaelices, it looks as if your suggested patch breaks the standard test suite under sqlite 3.4.X, in the current trunk. I cannot seem to repeat that with sqlite 3.3.X (I've tested both 3.3.4 and 3.3.7), but I cannot seem to repeat your bug either :-(

If you still have access to 3.3.5, can you check what the standard "basic" test, if used with and without your patch:

./runtests.py --settings ... basic

Ok I'll try with sqlite 3.3.5

I run all tests with sqlite 3.3.5 with this command:

  ./runtests.py

The result is patch doesn't break any tests. There are seven tests failed but no from django.db.

Firstly, there should be no tests failing. Otherwise you might be having other problems.

Secondly, do any extra tests (particularly the "basic" one -- just run that on its own as Fredrik indicated) fail when you do not have the patch in place?

The problem we are trying to work through is that without this patch, we can't get the existing tests to fail on the SQLite version Fredrik and I have tried, so it isn't clear that this is fixing a real problem. It may turn out to be tied to some particular bug in 3.3.5, in which case that's going to be hard cheese for users of 3.3.5. Or it might be something else, but right now, we don't have a clear demonstration of something that fails before the patch is applied and then starts working afterwards.

Replying to mtredinnick:

Firstly, there should be no tests failing. Otherwise you might be having other problems.

Ok. I consider it's a Django problem, because many failed tests refers (I think) to have differents locales that people that usually runs all tests (things like Expected: July, Got: Julio. Other fails test refer to new file session backend, and I've considered as usual in a sprint like this, with many changes in a little time.

It's another question, but I attached this tests report on this ticket.

Secondly, do any extra tests (particularly the "basic" one -- just run that on its own as Fredrik indicated) fail when you do not have the patch in place?

Here I know that you said to locate patch lines in another situation Field.get_db_prep_lookup. But, I thought previous comments of failing tests was made with my patch and possibly location doesn't matter. Sorry If I was wrong.

The problem we are trying to work through is that without this patch, we can't get the existing tests to fail on the SQLite version Fredrik and I have tried, so it isn't clear that this is fixing a real problem. It may turn out to be tied to some particular bug in 3.3.5, in which case that's going to be hard cheese for users of 3.3.5. Or it might be something else, but right now, we don't have a clear demonstration of something that fails before the patch is applied and then starts working afterwards.

Sorry, I havent logged before posting my comment

Replying to anonymous:

Replying to mtredinnick:

Firstly, there should be no tests failing. Otherwise you might be having other problems.

Ok. I consider it's a Django problem, because many failed tests refers (I think) to have differents locales that people that usually runs all tests (things like Expected: July, Got: Julio. Other fails test refer to new file session backend, and I've considered as usual in a sprint like this, with many changes in a little time. It's another question, but I attached this tests report on this ticket.

Secondly, do any extra tests (particularly the "basic" one -- just run that on its own as Fredrik indicated) fail when you do not have the patch in place?

Here I know that you said to locate patch lines in another situation Field.get_db_prep_lookup. But, I thought previous comments of failing tests was made with my patch and possibly location doesn't matter. Sorry If I was wrong.

The problem we are trying to work through is that without this patch, we can't get the existing tests to fail on the SQLite version Fredrik and I have tried, so it isn't clear that this is fixing a real problem. It may turn out to be tied to some particular bug in 3.3.5, in which case that's going to be hard cheese for users of 3.3.5. Or it might be something else, but right now, we don't have a clear demonstration of something that fails before the patch is applied and then starts working afterwards.

I started working on this during the sprint and was trying to be more conservative to avoid problems with other backends. Thought I'd go ahead and toss it in as an alternative.

Replying to mtredinnick:

Firstly, there should be no tests failing. Otherwise you might be having other problems. Secondly, do any extra tests (particularly the "basic" one -- just run that on its own as Fredrik indicated) fail when you do not have the patch in place? The problem we are trying to work through is that without this patch, we can't get the existing tests to fail on the SQLite version Fredrik and I have tried, so it isn't clear that this is fixing a real problem. It may turn out to be tied to some particular bug in 3.3.5, in which case that's going to be hard cheese for users of 3.3.5. Or it might be something else, but right now, we don't have a clear demonstration of something that fails before the patch is applied and then starts working afterwards.

I posted a duplicate of this problem and my own patches at #6141. The issue here is that when you store a date in SQLite, it's being stored as a string:

sqlite> create table foo( d date null );
sqlite> insert into foo(d) values( '2008-01-01' );
sqlite> select d, typeof(d) from foo;
2008-01-01|text

See the SQLite documentation and my post to the SQLite forum.

Since dates are stored as text in SQLite and '2008-01-01' is lexigraphicly less than '2008-01-01 00:00:00', the record in question falls outside the bounds of the query.

No tests are failing because they're not testing the problem, which lies with 'date' columns in SQLite, not datetime columns. I'm attaching a patch call test.diff that will expose the problem when run against SQLite. It patches tests/modeltests/basic/models.py, adding a class and a test at the bottom of the file.

Hope this helps clarify why this is a problem on SQLite.

Adds a test to identify the problem.

(In [7150]) Fixed #3689, #5223 -- Fixed "1st of January" comparisons for SQLite without breaking the other backends.

Based on a patch from raminf and tests from Nebojsa Djordjevic.