Django

Code

Ticket #7560 (closed: fixed)

Opened 5 months ago

Last modified 4 months ago

Delegate (most) type conversion to backends

Reported by: leosoto Assigned to: mtredinnick
Milestone: 1.0 beta Component: Database layer (models, ORM)
Version: SVN Keywords: jython db-be-api
Cc: Triage Stage: Unreviewed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

As discussed on the Mailing list and on IRC, some backends would be greatly simplified if they stop receiving string values for non-basic types, such as dates and decimal.

This patch delegates most of the get_db_prep_* logic of these fields to backend methods (named value_to_db_something where something is date, time, decimal, etc) It also implements get_db_prep_* for basic field types to avoid leaking string values.

For Jython/zxJDBC compatibility purposes, an special check was made on Field.get_db_prep_lookup and 'year' lookup. I was not able to understand it, so I choose to introduce as few modifications as possible there. It could be a good idea to delegate that logic to the backend too, but I think a second opinion is needed.

Finally, PhoneNumberField doesn't inherits from IntegerField anymore. There was no point on such inheritance, as phones are mapped to varchar fields on every backend. Not to mention that every method declared by IntegerField was overriden by PhoneNumberField.

This was tested against mysql, postgresql, oracle and sqlite3, and didn't broke anything (that wasn't already broken on trunk, at least).

Attachments

get_db_prep_refactor.patch (18.7 kB) - added by leosoto on 06/27/08 17:25:35.
get_db_prep_refactor2.patch (18.1 kB) - added by adamv on 07/01/08 21:25:34.
Patch against revision 7818
get_db_prep_refactor-3.patch (23.7 kB) - added by leosoto on 07/03/08 23:24:58.
Updated to current trunk. Also refactors year lookup code.
get_db_prep_refactor-4.patch (28.9 kB) - added by leosoto on 07/04/08 17:05:17.
Also fixes to_python on Time and DateTime? fields. Includes tests for this addition.
get_db_prep_refactor-5.patch (28.8 kB) - added by leosoto on 07/07/08 12:40:45.
Avoid failing test if the backend doesn't support usecs
7560_get_db_prep_refactor.diff (30.7 kB) - added by leosoto on 07/15/08 22:54:41.
7560_get_db_prep_refactor.2.diff (29.3 kB) - added by leosoto on 07/21/08 08:39:02.
Updated to svn r8011
7560_get_db_prep_refactor.3.diff (31.1 kB) - added by seanl on 07/22/08 16:27:25.
Fixes for TimeField?.to_python and extra tests

Change History

06/27/08 17:25:35 changed by leosoto

  • attachment get_db_prep_refactor.patch added.

07/01/08 21:25:34 changed by adamv

  • attachment get_db_prep_refactor2.patch added.

Patch against revision 7818

07/03/08 10:00:28 changed by anonymous

  • needs_better_patch changed.
  • needs_docs changed.
  • needs_tests changed.
  • milestone set to 1.0 beta.

Setting 1.0beta milestone, as Jython (and others Python VM such as PyPy) compatibility is on the 1.0 maybe list.

(follow-up: ↓ 3 ) 07/03/08 15:01:02 changed by jacob

This looks good at first glance. There's some commented-out code and # XXX comments that need to be removed/resolved; I'll read the patch in more detail a bit later and leave more feedback then.

In the meantime, Leo: can you post a summary of this ticket and the change to django-dev and make sure nobody sees problems with the general approach? Now that you have a working patch a final discussion of weather we should be doing this needs to happen real quick.

(in reply to: ↑ 2 ) 07/03/08 15:46:19 changed by leosoto

Replying to jacob:

This looks good at first glance. There's some commented-out code and # XXX comments that need to be removed/resolved;

Oh. Well, the commented out code was accidental. But the #XXXs where left on purpose, to make sure that the question doesn't remain unanswered when/if the commit occurs.

07/03/08 23:24:58 changed by leosoto

  • attachment get_db_prep_refactor-3.patch added.

Updated to current trunk. Also refactors year lookup code.

07/04/08 16:24:51 changed by leosoto

Hmm. I discovered that this refactor breaks _get_next_or_previous_by_FIELD on DateTime and Time fields. That's because Model._get_next_or_previous_by_FIELD always coerces the "current" value as string, then datetimes (and times) are converted to a string with usecs that can't be parsed later by DateTimeField.to_python.

So I'm going to improve the to_python method on both DateTimeField? and TimeField?.

07/04/08 17:05:17 changed by leosoto

  • attachment get_db_prep_refactor-4.patch added.

Also fixes to_python on Time and DateTime? fields. Includes tests for this addition.

07/06/08 12:03:35 changed by ramiro

  • keywords changed from jython to jython db-be-api.

07/07/08 12:40:45 changed by leosoto

  • attachment get_db_prep_refactor-5.patch added.

Avoid failing test if the backend doesn't support usecs

07/15/08 22:54:41 changed by leosoto

  • attachment 7560_get_db_prep_refactor.diff added.

07/15/08 23:00:45 changed by leosoto

Yet another patch update. This time, I've also removed some code duplication of almost all get_db_prep_save/get_db_prep_lookup methods, as discussed on http://groups.google.com/group/django-developers/browse_thread/thread/29f95436ed433d85.

07/21/08 08:39:02 changed by leosoto

  • attachment 7560_get_db_prep_refactor.2.diff added.

Updated to svn r8011

07/21/08 08:42:51 changed by leosoto

BTW, after the get_db_prep_value addition, I included changes to the documentation on the patch. I'm sure that it needs many changes as my English is not on par with the quality of Django docs.

07/22/08 16:27:25 changed by seanl

  • attachment 7560_get_db_prep_refactor.3.diff added.

Fixes for TimeField?.to_python and extra tests

07/28/08 11:56:57 changed by mtredinnick

  • owner changed from nobody to mtredinnick.

I started reviewing this last night. Will get it committed today or tomorrow.

07/29/08 00:09:30 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [8131]) Fixed #7560 -- Moved a lot of the value conversion preparation for loading/saving interactions with the databases into django.db.backend. This helps external db backend writers and removes a bunch of database-specific if-tests in django.db.models.fields.

Great work from Leo Soto.

Add/Change #7560 (Delegate (most) type conversion to backends)




Change Properties
Action