Re: DecimalField model validation

2011-10-05 Thread Paul McMillan 
> .. (A) silent rounding issue:
>    when a decimal value is saved in db
>    its decimal digits exceeding decimal_places are rounded off using
>    .quantize(). Rounding defaults to ROUND_HALF_EVEN (python default).
>    There is no mention of this behavior in docs.
Docs patches welcomed. This rounding behavior is generally superior to
ROUND_UP (what most of us are taught in gradeschool) for a number of
reasons that are inappropriate to get into here. If we're going to
round, that's the behavior we should use as default.

> .. (B) no model validation for decimal digits:
>    .full_clean() does not raise any exception:
>       - if the value has too many decimal digits
>       - or if value has too many digits
>    other fields, like CharField, do not validate values that exceed fields
> constraints
There's no clearly defined way to deal with overlong CharFields.
Rounding overlong floating point numbers is common and not unexpected.
Decimals work similarly to floats, and so it's not unreasonable to
have similar behavior.

Reading the docs on Python's decimal module, it makes sense to think
of the decimal_places as an analog to python's decimal notion of
precision.
http://docs.python.org/library/decimal.html#module-decimal

> .. (C) errors on save:
>   decimal values exceeding field's digits boundaries (decimal or total) make
> .save() raise decimal.InvalidOperation exceptions
This does sound like an error in validation. If it's possible to pass
a value through form validation to the point where it has errors on
save, that is a bug.

> In my opinion they should be fixed in a backwards-incompatible way!:
Thank you for your opinion, we don't do backwards-incompatible fixes.
There's usually a compatible way to fix behavior.

>  (A) django shuld not round decimal values silently before saving, if the
> value has too many decimal digits,
>      raise an exception (just like for values that have too many digits)
In the general use case, the existing behavior is more user friendly.
Many people would run out and re-add the rounding behavior if we
changed it. Users will enter over-long strings into decimal fields.
It's really unfriendly to paste an 32 place decimal number in and be
told it can only be 17 decimal digits long (and then have to go count
out till you get the correct number). Since there's not a good way to
make this the default value and retain backwards compatibility, this
is unlikely to change. This behavior should be documented.

That said, a no_rounding kwarg sounds like a perfectly reasonable feature.

>  (B) decimalfield shoud not validate values that exceed its max_digits and
> decimal_places constraints
I agree that decimalfield should not validate values that exceed
max_digits. If it does, that is a bug we should fix. We need to be
careful not to create situations where very large numbers validate but
very small but precise decimal numbers get rounded unexpectedly.
Unfortunately, these two parameters overlap in difficult ways. I
disagree about decimal_places, since the expected behavior is rounding
in most other real-world circumstances.

Does the combination of documenting the existing rounding behavior,
fixing the error on save() with max_digits, and adding the no_rounding
feature address your concerns?

-Paul

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: login_required decorator and user.is_active (yes, again)

2011-10-05 Thread Brian Neal 
On Oct 5, 2:59 pm, ptone  wrote:
> 13125 was wontfixed by Jacob at the end of the summer, this relates to
> the login_required decorator not checking for user.is_active
>
> I had opened a duplicate ticket 16996 before catching it as a dupe
>
> I'm going to dredge this one back up.

...

> I think that there would be pretty universal agreement about the
> expectation of what should happen if you make a previously active user
> inactive with regard to their access to @login_required protected
> views.  Shortening session times, seems like a really unpalatable
> workaround for sites that have chosen longer/default session
> expiration times.

If we are talking about work-arounds, have you seen this middleware?
This does the job perfectly for me and it is so simple:

http://djangosnippets.org/snippets/1105/

Regards,
BN

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

login_required decorator and user.is_active (yes, again)

2011-10-05 Thread ptone 

13125 was wontfixed by Jacob at the end of the summer, this relates to
the login_required decorator not checking for user.is_active

I had opened a duplicate ticket 16996 before catching it as a dupe

I'm going to dredge this one back up.

At a minimum, the current, counter-intuitive behavior of
login_required needs to be documented, but before I open a doc ticket
for that, I wanted to give this another shot here.

On Apr 15 2010, 5:55 pm, Russell Keith-Magee 
wrote:
> On Thu, Apr 15, 2010 at 11:20 PM, subs...@gmail.com  wrote:
> > Thanks for breaking it down.
>
> > On Mar 17, 7:45 am, Russell Keith-Magee 
> > wrote:
>
> >>  1) Don't touch the code. It's an annoying edge case, but it can be
> >> caught by shortening session timeouts and making more use of
> >> permissions. However, we should document the edge case with
> >>login_required, as it is certainly contrary to obvious usage.

Note, this is not yet documented in trunk.

> As I indicated previously in this thread, there are two use cases that
> need to be handled:
>
>  1) Normal Django authentication, honoring the is_activeflag. For
> this use case, you are completely correct - the current behavior is
> wrong.
>
>  2) Custom authentication backends that *don't* honor the is_active
> flag. This is entirely legal, and documented as such.

However, as of 1.3, the expectation is that backends will "handle" an
inactive user.

It is unclear from the current docs, what exactly the expectation is,
but it implies that any custom auth backend  needs to support the
is_active attr on the user_obj.

"Django 1.5 will assume that every backend supports inactive users
being passed to the authorization methods."

Right now, it is confusing as to how the is_active flag is treated by
contrib.auth. While the auth system involves several layers, there is
basically two types of implementation: "the built in default" and
"custom" (anything other than the builtin).  It is inconssistant to
have one part of the built in defaults (the login form) check this
attribute, but have other parts not check it (the decorator).  I know
that the defaults aren't explicitly a "set" in this way, none the
less, I think generally that people consider the out of box experience
to be coherent.

I think that there would be pretty universal agreement about the
expectation of what should happen if you make a previously active user
inactive with regard to their access to @login_required protected
views.  Shortening session times, seems like a really unpalatable
workaround for sites that have chosen longer/default session
expiration times.

During the deprecation, the decorator could check
user.backend.supports_inactive_user, as unlike at the time of the
above thread, the user object is not annotated with the backend.

-Preston

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #10863 - Email full stack traces (feedback please!)

2011-10-05 Thread Cal Leeming [Simplicity Media Ltd] 
That's pretty nice - something like this would be a nice additional feature.

Any thoughts core devs??

On Wed, Oct 5, 2011 at 8:26 PM, Patryk Zawadzki wrote:

> On Wed, Oct 5, 2011 at 12:40 PM, Cal Leeming [Simplicity Media Ltd]
>  wrote:
> > Could I bring the latest comment on the following ticket to the attention
> of
> > you guys:
> > https://code.djangoproject.com/ticket/10863#comment:17
> > Any feedback would be much appreciated.
>
> I'm not a core dev but since you're asking for feedback…
>
> Instead of emailing people with HTML I'd like to see a _useful_ stack
> trace in text format instead. I suggest you take a look at a debugging
> tool I wrote some time ago, it manages to get useful info without
> resorting to any rich formatting (in fact it only uses the logging
> library):
>
> https://github.com/patrys/great-justice
>
> --
> Patryk Zawadzki
> I solve problems.
>
> --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #10863 - Email full stack traces (feedback please!)

2011-10-05 Thread Patryk Zawadzki 
On Wed, Oct 5, 2011 at 12:40 PM, Cal Leeming [Simplicity Media Ltd]
 wrote:
> Could I bring the latest comment on the following ticket to the attention of
> you guys:
> https://code.djangoproject.com/ticket/10863#comment:17
> Any feedback would be much appreciated.

I'm not a core dev but since you're asking for feedback…

Instead of emailing people with HTML I'd like to see a _useful_ stack
trace in text format instead. I suggest you take a look at a debugging
tool I wrote some time ago, it manages to get useful info without
resorting to any rich formatting (in fact it only uses the logging
library):

https://github.com/patrys/great-justice

-- 
Patryk Zawadzki
I solve problems.

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

#16572 — looking for directions

2011-10-05 Thread Patryk Zawadzki 
Hi,

As you can see in the ticket¹, I've identified the source of the
exception and provided a naïve patch. I am however not familiar with
internals of the SQL compiler enough to judge whether it's the correct
place to patch. Can someone with more knowledge hold my hand here?

I am interested in this bug as it couses us quite a headache in the
Satchless project. Being unable to select_related() we currently have
to choose between making an extra query (ouch) and ignoring the ORM
(eww).

Cheers,

¹ https://code.djangoproject.com/ticket/16572

-- 
Patryk Zawadzki
I solve problems.

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: UPPER vs. ILIKE for __icontains

2011-10-05 Thread Ted Gruenloh 
Yeah, I've played around with the trigram indexes a little in 9.1, with mixed 
results.  For example, they didn't appear to improve the queries I used in my 
examples below.

Anyway, I think the case for using ILIKE for __icontains on postgres is 
worthwhile, and it's dead simple to change the 2 lines of code required.  I 
hope you'll consider it for the next release.

thanks
ted

+


Though apparently pg 9.1's trigram indexes can be used for ILIKE 
searches (but aren't a win for all query patterns). It seems to me it'd 
be worthwhile to have a setting to tell the backend whether to use ILIKE vs. 
LIKE LOWER, though personally I'm not using __icontains so I don't 
have a dog in this fight :)



From: Ted Gruenloh 
To: "django-developers@googlegroups.com" 
Sent: Tuesday, October 4, 2011 12:37 PM
Subject: UPPER vs. ILIKE for __icontains

Jonas:

Thanks for the response.  The ticket you referenced refers to __iexact.   In 
that case, I agree with the use of UPPER.  My question/suggestion is about 
__icontains, which must either use '...LIKE UPPER()' or ILIKE.  In my quick 
tests, ILIKE wins by about 10% on a table with 1 million records.  ILIKE gets 
better the more columns you're comparing, too (30% better, in my example).  See 
my examples below.


Anyway, I changed it in my implementation because it helped significantly in my 
specific case.  Hope this helps.

thanks
ted


sdb=# explain analyze select * from activity where msg ilike '%exploit abc 
akndkf%';
  QUERY 
PLAN  
--
 Seq Scan on activity  (cost=0.00..42147.00 rows=1 width=192) (actual 
time=1802.493..1802.493 rows=0 loops=1)
   Filter: ((msg)::text ~~* '%exploit abc akndkf%'::text)
 Total runtime: 1802.553 ms
(3 rows)

sdb=# explain analyze select * from activity where upper(msg) like 
upper('%exploit abc akndkf%');
   QUERY 
PLAN   

 Seq Scan on activity  (cost=0.00..44647.00 rows=100 width=192) (actual 
time=2074.101..2074.101 rows=0 loops=1)
   Filter: (upper((msg)::text) ~~ '%EXPLOIT ABC AKNDKF%'::text)
 Total runtime: 2074.158 ms
(3 rows)



sdb=# explain analyze select * from activity where msg ilike '%123%' or 
text_src ilike '%123%' or text_dst ilike '%123%' order by msg;
    QUERY 
PLAN 
---
 Sort  (cost=48622.98..48646.30 rows=9327 width=192) (actual 
time=2241.154..2270.481 rows=3774 loops=1)
   Sort Key: msg
   Sort Method: external merge  Disk: 808kB
   ->  Seq Scan on activity  (cost=0.00..47147.00 rows=9327 width=192) (actual 
time=11.701..2178.329 rows=3774 loops=1)
 Filter: (((msg)::text ~~* '%123%'::text) OR ((text_src)::text ~~* 
'%123%'::text) OR ((text_dst)::text ~~* '%123%'::text))
 Total runtime: 2298.757 ms
(6 rows)

sdb=# explain analyze select * from activity where upper(msg) like 
upper('%123%') or upper(text_src) like upper('%123%') or upper(text_dst) like 
upper('%123%') order by msg;
 QUERY 
PLAN  
-
 Sort  (cost=85617.58..85905.74 rows=115264 width=192) (actual 
time=3117.558..3147.699 rows=3774 loops=1)
   Sort Key: msg
   Sort Method: external merge  Disk: 808kB
   ->  Seq Scan on activity  (cost=0.00..54647.00 rows=115264 width=192) 
(actual time=8.239..3064.163 rows=3774 loops=1)
 Filter: ((upper((msg)::text) ~~ '%123%'::text) OR 
(upper((text_src)::text) ~~ '%123%'::text) OR (upper((text_dst)::text) ~~ 
'%123%'::text))
 Total runtime: 3177.240 ms
(6 rows)






- Original Message -
From: Jonas H. 
To: django-developers@googlegroups.com
Cc: 
Sent: Tuesday, October 4, 2011 11:19 AM
Subject: Re:

On 10/04/2011 05:51 PM, Ted Gruenloh wrote:
> The django online documentation mentions that the SQL equivalent for  
> __icontains is something like:
>
>      SELECT ... WHERE headline ILIKE '%Lennon%';
> However, for postgresql - one of the dbs that actually supports ILIKE - I 
> noticed __icontains was actually performing something similar to:
>      SELECT ... WHERE LOWER(headline) LIKE LOWER('%Lennon%');
>
> The ILIKE is obviously much faster [...]

DecimalField model validation

2011-10-05 Thread Marco Paolini 

Hi,

I would like to share some thoughts regarding django.db.models.DecimalField:

.. (A) silent rounding issue:
when a decimal value is saved in db
its decimal digits exceeding decimal_places are rounded off using
.quantize(). Rounding defaults to ROUND_HALF_EVEN (python default).
There is no mention of this behavior in docs.

.. (B) no model validation for decimal digits:
.full_clean() does not raise any exception:
   - if the value has too many decimal digits
   - or if value has too many digits
other fields, like CharField, do not validate values that exceed fields 
constraints

.. (C) errors on save:
   decimal values exceeding field's digits boundaries (decimal or total) make 
.save() raise decimal.InvalidOperation exceptions

In my opinion they should be fixed in a backwards-incompatible way!:
  (A) django shuld not round decimal values silently before saving, if the 
value has too many decimal digits,
  raise an exception (just like for values that have too many digits)
  (B) decimalfield shoud not validate values that exceed its max_digits and 
decimal_places constraints
  (C) leave as is, but if you solve (A) and (B) it's not that bad

I have a small django testcase that better explains the problem and proposed 
solution(s)

What do you think?


Marco

--
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

#10863 - Email full stack traces (feedback please!)

2011-10-05 Thread Cal Leeming [Simplicity Media Ltd] 
Could I bring the latest comment on the following ticket to the attention of
you guys:

https://code.djangoproject.com/ticket/10863#comment:17

Any feedback would be much appreciated.

Thanks

Cal

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.