On 30 elo, 23:24, Shai Berger <[email protected]> wrote: > Hi Ian, Jacob, Anssi and all, > > Thanks for the quick and positive responses. > > On Wednesday 29 August 2012, Ian Kelly wrote: > > >https://github.com/ikelly/django/commit/086f502a1c2acb9db27531f7df78c... > > > Shai, if you would be willing to try this patch, I'd like to hear your > > results. Please bear in mind that this is currently only a POC, so it > > could be buggy -- I've only run it through the basic tests. > > This looks very promising, I will try it next week. On a first look, there is > just one thing about it that I don't like, and it is related to Jacob's > comments -- > > On Wed, Aug 29, 2012 at 10:34 AM, Jacob Kaplan-Moss <[email protected]> > wrote:> I'm generally adverse to introducing more settings -- settings creep > > is a real problem -- so a couple of alternate ideas you might consider > > in producing a patch: > > > 1. Just pick one correct behavior, probably for each type. My guess > > would be that ints and floats should get the fast behavior, and > > decimals the slow-but-correct one. If you're using a decimal field > > it's because you care about precision, so correctness makes more sense > > there IMO. > > This would definitely be preferrable to a setting, but it doesn't cover all > cases -- we have some raw queries where expressions are being selected. The > default number type in this case -- in current code as well as Ian's patch -- > is to return the data as a string and convert it to int or Decimal. Our code, > which was written against other database systems, tries to use the result in > arithmetic with floats, and this fails (you can't add Decimals to floats). > > I could, of course, prevent the exception by converting the value to float in > my code, but that feels like the wrong solution -- I know, in my case, that > float is enough, and I'd much rather tell the backend "give me floats" than > have > it go through float->str->Decimal->float. > > > 2. If you must introduce a setting, do it in DATABASES[OPTIONS] > > instead of a top-level ORACLE_FAST_NUMBERS or whatever. > > That is indeed one good option. The alternative I'm considering -- less > convenient for me, but perhaps more flexible generally -- is, since we're > dealing with raw SQL anyway, to put some API for this on the cursor; perhaps a > property, so that setting it would be a no-op on other backends. > > As per Anssi's comment, I have no backwards-compatibility requirements with > respect to cx_Oracle, but we are still running Django 1.3. Trying to call the > performance improvement a "bugfix" so it can be backported, while changing > cx_Oracle requirements, would be pushing my luck, right?
1.3 is out of the question as only security fixes go into that release. Even 1.4 is out because this isn't a bugfix. Even if this was backported, requiring library upgrades for Django's minor version updates should be avoided if at all possible. For 1.4 -> 1.5 upgrade it is in my opinion OK. It should be possible to subclass Django's Oracle backend, override parts of the backend with the changes that go into 1.5, and use that in 1.3. - Anssi -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to [email protected]. 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.
