Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-10-08 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
 Reporter:  andy@…   |Owner:
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by julien):

 Yeah using tuples does the trick (updated in latest patch). That removes
 my only concern with the patch. I'm pretty sure this refactoring would
 increase speed in the Python code processing. I'll try to do some
 benchmarking.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-10-08 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
 Reporter:  andy@…   |Owner:
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 Why not cache and return tuples? That way there is no way the caller can
 modify them. If caller needs to do it, the caller can turn them into
 lists. However, I doubt you can show any real speed difference, as clone
 will take all the time in the interesting cases.

 I would like to review this, but I don't know when I will have the time to
 do it.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-10-08 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
 Reporter:  andy@…   |Owner:
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by julien):

 * needs_better_patch:  1 => 0


Comment:

 All tests are now passing. I'm reasonably happy of where the patch is at
 now. I'm just wondering whether the values fetched from the
 resolve_lookup_path cache should systematically copied (as currently in
 the the patch) or if it's should be the calling code's responsibility to
 make copies before modifying the values -- if the later, then there would
 probably be a slight speed increase.

 At this point I'd really like to get the patch reviewed. Any feedback is
 welcome. Thanks!

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-10-07 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
 Reporter:  andy@…   |Owner:
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by julien):

 * needs_better_patch:  0 => 1


Comment:

 I've pushed the refactoring further, notably by modifying setup_joins. Now
 a given lookup wouldn't be parsed multiple times -- only once. I'm still
 getting 6 test errors that I haven't got the time to get to the bottom of
 yet, but I think we're getting pretty close now.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-10-07 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
 Reporter:  andy@…   |Owner:
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by julien):

 Actually, I can post my patch right now (see above).

 As suggested by akaariai I've created a new
 `Options.resolve_lookup_path()` method and used it in multiple places,
 which allows to simplify the code quite a bit, in particular in the admin.
 I've tried to make the algorithm as efficient as possible and used some
 caching mechanism too.

 Feedback welcome. Thank you!

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-10-07 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
 Reporter:  andy@…   |Owner:
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by julien):

 Thanks, I'll like this idea. I've made good progress writing a new patch
 for that and I'll try to post it here tomorrow.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-10-06 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
 Reporter:  andy@…   |Owner:
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 Well, the idea would be that when for example order by is called, we could
 check if the order is valid at that point. It would be an trivial check to
 do:
 {{{
 for field in fields:
 if self.resolve_lookup_path(field)[LOOKUP_PART]:
 raise ValueError('Invalid order by!')
 }}}
 and that is pretty much it. We should first remove the leading '-'
 though...

 But I also think it really belongs to model._meta. Why does sql/query.py
 need to know how to travel the fields? IMHO it doesn't need to know that.
 Abstract that away in a model._meta method and just call it when needed.
 There are also other parts in Django which would benefit from this. Take a
 look at django/contrib/admin/options.py:L236.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-10-06 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
 Reporter:  andy@…   |Owner:
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by julien):

 * needs_better_patch:  1 => 0


Comment:

 Thanks for your review and ideas! Splitting that code out into a separate
 method sounds interesting -- could you elaborate on where else and how it
 would be useful?

 I'm removing the Patch needs improvement flag, since I think this patch
 fixes the original bug, which appears to be the highest priority at this
 stage -- Larger refactors could be tracked elsewhere like in #16187. The
 patch would still need a close review before it can proceed, though.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-10-06 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
 Reporter:  andy@…   |Owner:
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by akaariai):

 * cc: anssi.kaariainen@… (added)


Comment:

 I read somewhat quickly through the patch. This looks simple and
 efficient.

 I think this could be extended in the future if need be (referring for
 example to ticket #16187).

 However, I wonder if it would be better to make this a separate method.
 This could be usable in checking the validitity of other calls, order_by
 for example. I would actually go so far as to make this model._meta
 method, returning 3-tuple field_path, final_field (which can be in
 field_path or not), and lookup_parts. This could be useful outside of
 sql.query, and this would make for proper separation of concerns.

 I don't have time for more thorough review at the moment. I am not
 changing patch needs improvement. I wonder if it is set for the latest
 patch or just forgotten to checked?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-10-04 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
 Reporter:  andy@…   |Owner:
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by julien):

 OK so I've updated the patch simplifying the code's algorithm and the
 tests.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-08-27 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
   Reporter:  andy@… |  Owner:
   Type:  Bug| Status:  new
  Milestone: |  Component:  Database layer
Version:  SVN|  (models, ORM)
 Resolution: |   Severity:  Normal
   Triage Stage:  Accepted   |   Keywords:
Needs documentation:  0  |  Has patch:  1
Patch needs improvement:  1  |Needs tests:  0
  UI/UX:  0  |  Easy pickings:  0
-+-

Comment (by ramiro):

 See also ticket:8424#comment:13 for a similar problem and #16187 (possibly
 a long term solution). There is also another ticket or thread I can't find
 right now that proposed changing lookup names to all uppercase.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-08-27 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
   Reporter:  andy@… |  Owner:
   Type:  Bug| Status:  new
  Milestone: |  Component:  Database layer
Version:  SVN|  (models, ORM)
 Resolution: |   Severity:  Normal
   Triage Stage:  Accepted   |   Keywords:
Needs documentation:  0  |  Has patch:  1
Patch needs improvement:  1  |Needs tests:  0
  UI/UX:  0  |  Easy pickings:  0
-+-
Changes (by julien):

 * ui_ux:   => 0
 * easy:   => 0


Comment:

 This was again reported in #16714. Note that the same issue occurs with
 fields named 'month' and 'day' as well as 'year'.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2011-04-16 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
-+-
   Reporter:  andy@… |Owner:
   Type:  Bug|   Status:  new
  Milestone: |Component:  Database layer
Version:  SVN|  (models, ORM)
 Resolution: | Severity:  Normal
   Triage Stage:  Accepted   | Keywords:
Needs documentation:  0  |Has patch:  1
Patch needs improvement:  1  |  Needs tests:  0
-+-
Changes (by julien):

 * needs_better_patch:  0 => 1


Comment:

 The tests would need to be rewritten using unittests since this is now
 Django's preferred way.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2010-10-01 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
---+
  Reporter:  a...@hauntedwisconsin.com | Owner: 
Status:  new   | Milestone: 
 Component:  Database layer (models, ORM)  |   Version:  SVN
Resolution:|  Keywords: 
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  0 |  
---+
Changes (by jpwatts):

  * owner:  jpwatts =>

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2010-03-12 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
---+
  Reporter:  a...@hauntedwisconsin.com | Owner:  jpwatts
Status:  new   | Milestone: 
 Component:  Database layer (models, ORM)  |   Version:  SVN
Resolution:|  Keywords: 
 Stage:  Accepted  | Has_patch:  1  
Needs_docs:  0 |   Needs_tests:  0  
Needs_better_patch:  0 |  
---+
Changes (by jpwatts):

  * owner:  nobody => jpwatts
  * has_patch:  0 => 1
  * version:  1.1 => SVN

Comment:

 I'm attaching a patch that fixes this problem by restricting which kinds
 of lookups will be attempted on related fields.  I've included a
 regression test and verified that the rest of Django's test suite passes
 (with the exception of the test that isn't passing on trunk either).

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2010-03-12 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
---+
  Reporter:  a...@hauntedwisconsin.com | Owner:  nobody
Status:  new   | Milestone:
 Component:  Database layer (models, ORM)  |   Version:  1.1   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  0 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Changes (by jpwatts):

 * cc: j...@joelwatts.com (added)

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #11670: Minor limitation using fields named 'year' in related searches.

2010-02-02 Thread Django 
#11670: Minor limitation using fields named 'year' in related searches.
---+
  Reporter:  a...@hauntedwisconsin.com | Owner:  nobody
Status:  new   | Milestone:
 Component:  Database layer (models, ORM)  |   Version:  1.1   
Resolution:|  Keywords:
 Stage:  Accepted  | Has_patch:  0 
Needs_docs:  0 |   Needs_tests:  0 
Needs_better_patch:  0 |  
---+
Changes (by russellm):

  * needs_better_patch:  => 0
  * stage:  Unreviewed => Accepted
  * needs_tests:  => 0
  * needs_docs:  => 0

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.