Re: [Django] #11670: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for lookup types in lookups across relations

2012-02-04 Thread Django 
#11670: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for 
lookup
types in lookups across relations
-+-
 Reporter:  andy@…   |Owner:  julien
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  SVN
  (models, ORM)  |   Resolution:  fixed
 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):

 * status:  new => closed
 * owner:   => julien
 * resolution:   => fixed


Comment:

 In [17450]:
 {{{
 #!CommitTicketReference repository="" revision="17450"
 Fixed #11670 -- Prevented genuine model fields named 'year', 'month',
 'gt', 'lt' etc. from being mistaken for lookup types in lookups across
 relations. Thanks to andy for the report, to jpwatts for the initial patch
 and to Anssi Kääriäinen and Alex Gaynor for the reviews.
 }}}

-- 
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: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for lookup types in lookups across relations

2012-02-04 Thread Django 
#11670: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for 
lookup
types in lookups across relations
-+-
 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):

 I'm going to commit the fix for the originally reported issue. A deeper
 refactoring would still be welcome but it should be tackled as part of a
 separate ticket. #16187 could be a good candidate.

-- 
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: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for lookup types in lookups across relations

2012-02-04 Thread Django 
#11670: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for 
lookup
types in lookups across relations
-+-
 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 akaariai):

 * needs_better_patch:  1 => 0


Comment:

 If the patch is good to go, it would be nice to get this into 1.4.

 In that case opening a new ticket about the larger refactor would be nice,
 so that the patches about the refactoring do not get lost.

 I unchecked the "patch needs improvement" box, as I think that is per the
 triaging guide: patch author removes that box when he feels he has
 something ready, reviewer rechecks it, or marks as ready for checkin as
 needed. Unfortunately, I don't have time for review just 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: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for lookup types in lookups across relations

2012-02-04 Thread Django 
#11670: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for 
lookup
types in lookups across relations
-+-
 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):

 The patch above is a slight improvement on 11670.field-lookup-
 collisions.2.diff, as the full lookup resolution now only occurs if the
 last part isn't a lookup type. That means there wouldn't be any extra
 computation for the most common lookups.

 Since it's a bit late before 1.4 gets released to do a full refactor as
 discussed earlier, I suggest we start by fixing the bug using that latest
 patch, and then look into a proper refactor later for 1.5.

-- 
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: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for lookup types in lookups across relations

2011-10-26 Thread Django 
#11670: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for 
lookup
types in lookups across relations
-+-
 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: akaariai (added)


Comment:

 I have an idea, but maybe it is not workable. So, my first idea is to get
 the patch in as is, and revisit this issue later on.

 The maybe-unworkable idea is that the code for traveling the lookups
 translates to this logic (at least I think it does):
 {{{
 while lookup_leads_to_field:
 fields.append(found_field)
 if found_field is related_field:
 related_field = found_field.related_field
 else:
 goto marker1

 if lookups_left:
 if not allow_explicit_fk:
 raise FieldError
 marker1:
 ...
 }}}

 Now, if you get a lookup like `rel_field__non_existing_field` you are
 going to raise `FieldError`. But if you instead do
 `nonrel_field__non_existing_field` you are going to say that nonrel_field
 was found and non_existing_field part wasn't looked. This is because
 nonrel_field will "goto marker1" (via NotRelatedField exception) and skip
 the if lookups_left part.

 So, the only idea I have is to return the rel_field in the
 `rel_field__non_existing_field` lookup case. So, something like this:
 {{{
 # pseudoish...
 for counter, field_name in enumerate(parts):
 try:
 if field_name == 'pk':
 field_name = opts.pk.name
 field_info = opts.get_field_by_name(field_name)
 fields += (field_info,)
 if (counter + 1) < num_parts:
 # If we haven't reached the end of the list of
 # lookups yet, then let's attempt to continue
 # traversing relations.
 related_model = get_model_from_relation(field_info[0])
 opts = related_model._meta
 except FieldDoesNotExist, NotRelatedField:
 break
 if fields:
 # We found at least one field, lets return that
 last_field = fields[-1]
 else:
 # The lookup was totally bogus - it didn't resolve any field from
 # the base model
 raise FieldError
 return parts, fields, last_field
 }}}

 The caller is then responsible to raising `FieldError` if that is the
 appropriate action, doing the explicit_fk logic and checking for existing
 aggregates, etc. I haven't tried this at all, so it is possible this idea
 is bogus, or will make the caller's job more complicate than it needs to
 be.

 As said before: getting the patch in as is could be the thing to do.
 Polish it later on when we know how custom lookups are going to be
 handled. I have a tendency to over-complicate things, and I have a feeling
 I am in over-complicating mode right 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: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for lookup types in lookups across relations

2011-10-25 Thread Django 
#11670: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for 
lookup
types in lookups across relations
-+-
 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):

 @akaariai: Thanks for your review. I've updated the branch:
 https://github.com/jphalip/django/compare/master...lookup_refactor#files_bucket

 The cache is now flushed in `add_field()`. As for your comment re: not
 passing the query to `resolve_lookup_path()`, I've tried but couldn't come
 up with a clean implementation. Have you got any specific ideas for 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: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for lookup types in lookups across relations

2011-10-13 Thread Django 
#11670: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for 
lookup
types in lookups across relations
-+-
 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):

 * needs_better_patch:  0 => 1


Comment:

 Some notes:
   - TYPO: Lookups on non-existants fields are ok, since they're ignored
   - Shouldn't the cache be cleared in add_field like the other field
 caches?
   - Is it absolutely necessary to pass in the query into
 resolve_lookup_path, it is only used in:
 {{{
 field_name not in query.query_terms and
 field_name not in query.aggregate_select.keys() and
 field_name not in query.aggregates.keys() and
 LOOKUP_SEP.join(parts) not in query.aggregates.keys()
 }}}
 It would be IMHO better to pass in two sets, one containing the first
 three lines unioned, and the other the query.aggregates.keys(). Even
 better would be to move some of this logic into query.py. One idea is just
 to ignore the whole `FieldDoesNotExist` and let the caller work out what
 the result should be. So if you resolve 'foo`__`bar_id', you would only
 resolve the foo part and let the caller figure out what to do with the
 bar_id part. There is also a related bug here: the cache key is the passed
 in path, but the result which is cached depends also on the query and
 allow_explicit_fk, so if first called with 'foo`__`bar_id', True, we would
 cache the wrong result for the call 'foo`__`bar_id', False.

 Otherwise the patch looks good to me. This is a big cleanup to lookup path
 handling in my opinion.

-- 
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: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for lookup types in lookups across relations

2011-10-09 Thread Django 
#11670: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for 
lookup
types in lookups across relations
-+-
 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):

 To make it easier to track changes, I've pushed everything to a github
 branch: https://github.com/jphalip/django/compare/master...lookup_refactor

 The benchmarks I've run with djangobench seem to show a very slight speed
 increase but nothing significant. Therefore this patch is mostly about
 simplifying and DRY-ing up the lookup system's codebase.

-- 
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: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for lookup types in lookups across relations (was: Minor limitation using fields named 'year' in related searches.

2011-10-09 Thread Django 
#11670: Model fields named 'year', 'month', 'gt', 'lt' etc. get mistaken for 
lookup
types in lookups across relations
-+-
 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|
-+-

-- 
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.