Getting rid of get_db_prep_lookup on Field subclasses?
After fixing some corner cases of #7560[1], and realizing that it is too easy to write a wrong get_db_prep_lookup method (example: #7448[2]), I have reached the conclusion that there is no reason to have *two* methods for doing the python -> db conversion (``get_db_prep_save`` and ``get_db_prep_lookup``). Not for most of the fields, at least. What I'm talking about is that, to ensure the proper type conversion before querying the database[3], all of django core Fields should currently do this: def get_db_prep_save(self, value): return some_transformation(value) def get_db_prep_lookup(self, value, lookup_type): if hasattr(value, 'as_sql'): sql, params = value.as_sql() return QueryWrapper(('(%s)' % sql), params) if lookup_type in ('range', 'in'): value = [some_transformation(v) for v in value] elif lookup_type in ('exact', 'gt', 'gte', 'lt', 'lte'): value = some_transformation(v) return Field.get_db_prep_lookup(self, value) Which blatantly violates DRY. Also note that the documentation for custom fields, suggest to reuse get_db_prep_save on get_db_prep_lookup[4]. Following that advice, I propose to add a ``get_db_prep_value`` method to the Field API, to make the common case much simpler. Then, almost all field can do: def get_db_prep_value(self, value): return some_transformation(value) [Or just implement the transformation in the body, if it is not trivial. I didn't want to reuse get_db_prep_save as the generic transformation method, because the 'save' part seems to imply more than just value transformation]. Then, on the base Field we would use this method to implement ``get_db_prep_save`` and ``get_db_prep_lookup``, and stop repeating ourselves. Does that make sense? Should I add this change to the patch on #7560? [1] http://code.djangoproject.com/attachment/ticket/7560 [2] http://code.djangoproject.com/attachment/ticket/7448 [3] This is needed by some backends which are not "string-happy", such as JDBC backends. [4] http://www.djangoproject.com/documentation/custom_model_fields/#get-db-prep-lookup-self-lookup-type-value -- 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: Getting rid of get_db_prep_lookup on Field subclasses?
On Tue, 2008-07-15 at 11:49 -0400, Leo Soto M. wrote: > After fixing some corner cases of #7560[1], and realizing that it is > too easy to write a wrong get_db_prep_lookup method (example: > #7448[2]), I have reached the conclusion that there is no reason to > have *two* methods for doing the python -> db conversion > (``get_db_prep_save`` and ``get_db_prep_lookup``). Not for most of the > fields, at least. I think it will take me a few days to think through all the implications here. However, I suspect the least intrusive change is to keep db_prep_lookup and db_prep_save, but have them call a common method (which could be called db_prep_value if you like). I can think of some slightly twisted cases where lookup and insert values will be different. For example, if I make a "month" field type, I would use an underlying date field column and the insert would be a full-fledged date value, whilst the lookup would be a call to a date extraction function in SQL. There quite possibly exist some legitimate cases like this as well (GIS stuff springs to mind as an area to look, for example). So, sharing common code would be good. Forcing everything to use the same code would probably come back and bite us. I think we're too close to an irreversible boundary for the API to make that type of change now. It often takes a few months, or longer, for all the corner cases to fall out. Thus, I'd keep the current API for the boundary between fields and the backends, but factor out the common bits more explicitly. 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: Getting rid of get_db_prep_lookup on Field subclasses?
On Tue, Jul 15, 2008 at 12:38 PM, Malcolm Tredinnick <[EMAIL PROTECTED]> wrote: > > > On Tue, 2008-07-15 at 11:49 -0400, Leo Soto M. wrote: >> After fixing some corner cases of #7560[1], and realizing that it is >> too easy to write a wrong get_db_prep_lookup method (example: >> #7448[2]), I have reached the conclusion that there is no reason to >> have *two* methods for doing the python -> db conversion >> (``get_db_prep_save`` and ``get_db_prep_lookup``). Not for most of the >> fields, at least. > > I think it will take me a few days to think through all the implications > here. However, I suspect the least intrusive change is to keep > db_prep_lookup and db_prep_save, but have them call a common method > (which could be called db_prep_value if you like). Sure, that was what I was proposing. Seems like I didn't explained myself well, but I don't want to kill Field.get_db_prep_{save,lookup}. I want to avoid overriding them in the same repetitive way on the common cases. So looks like we agree on the general idea, and I'm going to incorporate it to #7560. -- 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 -~--~~~~--~~--~--~---