Re: #7560: Delegate (most) type conversion to backends
On Fri, Jul 4, 2008 at 12:27 AM, Leo Soto M. <[EMAIL PROTECTED]> wrote: > So the new [...] A quick update: After more testing with real django applications I discovered that I was breaking get_next_by_FIELD. This fourth patch[1] fixes this issue and includes tests for this use case (which wasn't exercised on the rest of the suite) [1] http://code.djangoproject.com/attachment/ticket/7560/get_db_prep_refactor-4.patch -- Leo Soto M. http://blog.leosoto.com --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: #7560: Delegate (most) type conversion to backends
On Thu, Jul 3, 2008 at 10:36 PM, Malcolm Tredinnick
<[EMAIL PROTECTED]> wrote:
> I've also read through the patch already and it seems like the right
> track. I like the approach, since it helps in a few areas, including
> field extensions and extra database backends.
>
> I haven't given it a complete fine-detail review yet to pick things out
> of individual lines, but only one thing jumped out on the initial read.
> I've been trying very hard to eliminate lines like:
>
> if settings.DATABASE_ENGINE.endswith('zxjdbc'):
>
> from places outside the database backend-specific directories. This is
> because it locks in particular backends to the main code, leading to
> extensibility problems for other backends. So rather than introduce a
> line like that, I'm always going to prefer to remove things like the
> section following it that gives SQLite3 special handling.
Yes, as I noted on the ticket description, I didn't really understood
why all that different formats were needed by each backend, so I was a
bit scared to touch that portion of code.
But now that you make me think again, that was just a lame excuse :-).
I just pushed that logic to DatabaseOperations and named it
"year_lookup_bounds".
So the new posted patch[1] is updated to svn trunk, and also fixes a
test that would broke because get_db_prep* now may return different
values for different backends.
[1]
http://code.djangoproject.com/attachment/ticket/7560/get_db_prep_refactor-3.patch
--
Leo Soto M.
http://blog.leosoto.com
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---
Re: #7560: Delegate (most) type conversion to backends
On Thu, 2008-07-03 at 16:57 -0400, Leo Soto M. wrote:
> Hi all!,
>
> I've posted a patch[1] with a small refactor to some Field's
> get_db_prep_* methods. Basically, for non-basic field types, the
> database-prepared values is not computed by the Fields anymore, but
> delegated to DatabaseOperations functions. As the current default
> logic was copied to the default methods implementations on
> DatabaseOperations, this should not break anything (I fully tested it
> a week ago, with the four official backends and indeed it didn't broke
> anything).
I've also read through the patch already and it seems like the right
track. I like the approach, since it helps in a few areas, including
field extensions and extra database backends.
I haven't given it a complete fine-detail review yet to pick things out
of individual lines, but only one thing jumped out on the initial read.
I've been trying very hard to eliminate lines like:
if settings.DATABASE_ENGINE.endswith('zxjdbc'):
from places outside the database backend-specific directories. This is
because it locks in particular backends to the main code, leading to
extensibility problems for other backends. So rather than introduce a
line like that, I'm always going to prefer to remove things like the
section following it that gives SQLite3 special handling.
Plus, if somebody wants to have a real database, the name needs to look
like they didn't just fall on their keyboard and "zxjdbc" doesn't pass
on those grounds. Looking like we threw up in the code is uncool. :-)
Mentally, I've queued it up behind Honza's validation and normalisation
changes, because I want to see how it interacts with that. I've got a
couple of more important bugs to fix immediately, but I'd like to get to
Honza's stuff as soon as possible and this weekend might be when I have
time. So, if Russell gets there first, no skin off my nose, otherwise I
might deal with it in the next few days.
Regards,
Malcolm
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---
Re: #7560: Delegate (most) type conversion to backends
On Thu, Jul 3, 2008 at 8:27 PM, Russell Keith-Magee <[EMAIL PROTECTED]> wrote: > > On Fri, Jul 4, 2008 at 4:57 AM, Leo Soto M. <[EMAIL PROTECTED]> wrote: >> >> Hi all!, >> >> I've posted a patch[1] with a small refactor to some Field's >> get_db_prep_* methods. Basically, for non-basic field types, the >> database-prepared values is not computed by the Fields anymore, but > > Hi Leo, > > I've had a quick look at this patch. On the surface, it looks like a > good idea and a pretty good implementation, but I'll need to spend a > bit more time with it before I pass final judgement. Sure, and thanks for setting a few time aside to do this first review! >> This patch is part of my work of running Django on Jython, and in >> general helps anybody who maintains an external django database >> backend. > > Completely aside from that, it may actually fix a few other problems > we've been having - for example, MySQL's habit of returning 1/0 from > boolean fields. I'm not sure about it. The patch improves the Python -> DB conversion, not the other way around. -- Leo Soto M. http://blog.leosoto.com --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: #7560: Delegate (most) type conversion to backends
On Fri, Jul 4, 2008 at 4:57 AM, Leo Soto M. <[EMAIL PROTECTED]> wrote: > > Hi all!, > > I've posted a patch[1] with a small refactor to some Field's > get_db_prep_* methods. Basically, for non-basic field types, the > database-prepared values is not computed by the Fields anymore, but Hi Leo, I've had a quick look at this patch. On the surface, it looks like a good idea and a pretty good implementation, but I'll need to spend a bit more time with it before I pass final judgement. > This patch is part of my work of running Django on Jython, and in > general helps anybody who maintains an external django database > backend. Completely aside from that, it may actually fix a few other problems we've been having - for example, MySQL's habit of returning 1/0 from boolean fields. It is also a nice cleanup of the handling of time fields. Thanks for your work on this. Yours, Russ Magee %-) --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
