Am 06.04.2011 um 23:29 schrieb Michal Petrucha:
> On Wed, Apr 06, 2011 at 06:22:33AM +0200, Johannes Dollinger wrote:
>>
>> Am 06.04.2011 um 02:45 schrieb Michal Petrucha:
>>
>> [snip]
>>
>>> unique and db_index
>>> ~~~~~~~~~~~~~~~~~~~
>>> Implementing these will require some modifications in the backend code.
>>> The table creation code will have to handle virtual fields as well as
>>> local fields in the table creation and index creation routines
>>> respectively.
>>>
>>> When the code handling CompositeField.unique is finished, the
>>> models.options.Options class will have to be modified to create a unique
>>> CompositeField for each tuple in the Meta.unique_together attribute. The
>>> code handling unique checks in models.Model will also have to be updated
>>> to reflect the change.
>>
>> Instead of auto-creating CompositeFields for each composite index
>> you could just append to _meta.unique_together in CompositeFields.
>> This prevents cluttering the set of field for introspection. Or is
>> there a benefit in using full CompositeFields for each index?
>
> I proposed this change for the sake of API uniformity. I mean,
> currently there is one way to specify the unique constraint for single
> fields, a similar way to create an index and also a very similar way
> to specify the primary key. But if you want to specify a unique
> constraint on several fields, you have to use a completely different
> mechanism and an analogy for composite indexes is missing altogether.
>
> Since I'm proposing an analogous API to specify a primary key
> consisting of several fields, and since it can also be extended to
> specify indexes the same way without much effort, it seems kind of
> "right" to me to unify the API. (I'm talking only about the public API
> here.)
>
> I'll just note that this was not my idea, I just included it in my
> proposal when I was reading that thread because I consider it a good
> idea. See [47].
The only downside is that you'll have to pick a name for the index – even if
you don't really care (that's historically been a problem with `related_name`).
But anyway, since Meta.unique_together probably cannot be deprecated any time
soon, that's just a -0 from me.
> As for the set of fields, I don't think having a few more fields in
> _meta.virtual_fields would hurt that much. It's not like every model
> is going to have tens of surpluss CompositeFields...
>
>>> Retrieval and assignment
>>> ~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Jacob has actually already provided a skeleton of the code that takes care
>>> of this as seen in [1]. I'll only summarize the behaviour in a brief
>>> example of my own.
>>>
>>> class SomeModel(models.Model):
>>> first_field = models.IntegerField()
>>> second_field = models.CharField(max_length=100)
>>> composite = models.CompositeField(first_field, second_field)
>>>
>>>>>> instance = new SomeModel(first_field=47, second_field="some string")
>>>>>> instance.composite
>>> SomeModel_composite(first_field=47, second_field='some string')
>>>>>> instance.composite = (74, "other string")
>>>>>> instance.first_field, instance.second_field
>>> (74, 'other string')
>>
>> Just a small bikeshed: CompositeField should take the fields as a
>> tuple instead of *varargs.
>
> Both solutions look equally "good" to me, even the difference in the
> implementation is just a single asterisk. (-; If it were up to me, I'd
> choose the *varargs version only because it would save the user a pair
> of parantheses.
You get to paint it :)
>>> QuerySet filtering
>>> ~~~~~~~~~~~~~~~~~~
>>>
>>> This is where the real fun begins.
>>>
>>> The fundamental problem here is that Q objects which are used all over the
>>> code that handles filtering are designed to describe single field lookups.
>>> On the other hand, CompositeFields will require a way to describe several
>>> individual field lookups by a single expression.
>>>
>>> Since the Q objects themselves have no idea about fields at all and the
>>> actual field resolution from the filter conditions happens deeper down the
>>> line, inside models.sql.query.Query, this is where we can handle the
>>> filters properly.
>>>
>>> There is already some basic machinery inside Query.add_filter and
>>> Query.setup_joins that is in use by GenericRelations, this is
>>> unfortunately not enough. The optional extra_filters field method will be
>>> of great use here, though it will have to be extended.
>>>
>>> Currently the only parameters it gets are the list of joins the
>>> filter traverses, the position in the list and a negate parameter
>>> specifying whether the filter is negated. The GenericRelation instance can
>>> determine the value of the content type (which is what the extra_filters
>>> method is used for) easily based on the model it belongs to.
>>>
>>> This is not the case for a CompositeField -- it doesn't have any idea
>>> about the values used in the query. Therefore a new parameter has to be
>>> added to the method so that the CompositeField can construct all the
>>> actual filters from the iterable containing the values.
>>>
>>> Afterwards the handling inside Query is pretty straightforward. For
>>> CompositeFields (and virtual fields in general) there is no value to be
>>> used in the where node, the extra_filters are responsible for all
>>> filtering, but since the filter should apply to a single object even after
>>> join traversals, the aliases will be set up while handling the "root"
>>> filter and then reused for each one of the extra_filters.
>>
>> As you noted, the problem with GFK, CompositeFields, and virtual
>> fields in general is mostly that support for filtering (read: custom
>> lookups) requires changes deep inside the ORM. I have a half-baked
>> proposal that would make such fields much simpler to implement but
>> would require a large refactoring of the ORM. It might well be that
>> it turns out to be impractical, impossible, or too big to be
>> included in your summer of code project. That being said, here's the
>> plan:
>>
>> Lookups are handled by the field class. That involves moving
>> significant bits of code from db.models.sql.query into the field
>> classes. Ideally, Query knows anything about fieldtypes. It just
>> delegates lookups (including related field lookups) to the field
>> implementation via:
>>
>> def add_lookup(self, query, alias, lookup_bits, value, **kwargs):
>> """
>> - `query` is the db.sql.query.Query instance
>> - `alias` is the table alias of the "current" table;
>> that is: the table which this lookup must be
>> performed on
>> - `lookup_bits` is a sequence of lookups, e.g.
>> "related_field__some_field__icontains".split("__")
>> - `value` is the filter argument
>> - `**kwargs`: It might be necessary to pass more
>> information, but certainly not arbitrary data. I
>> just haven't thought that through yet.
>> """
>>
>> Examples:
>> * CharField will assert that it receives at most one lookup bit. It
>> adds a WHERE clause for `alias`.`fieldname`.
>> * ForeignKey looks for field `f` in its containing model with name
>> lookup_bits[0]. If such a field exists it sets up a join and
>> passes the resulting table alias along with lookup_bits[1:] to
>> f.add_lookup().
>> * CompositeField checks if the first lookup bit is the name of one
>> of its constituent fields. If that's the case, the lookup is
>> delegated to that field. Otherwise, it adds a composite WHERE
>> clause for its constituent fields.
>> * Reverse lookups would require that reverse descriptors are
>> implemented as virtual fields.
>>
>> This refactoring, which involves cleaning up (and splitting)
>> db.models.sql.query and db.models.fields.*, would probably be full
>> of dragons. Finding a clean public API for Field and Query alone
>> would be major task.
>> But it could result in a decoupled and comparably trivial
>> implementation of GFKs and Composite fields and would opens the ORM
>> for other virtual fields and custom lookups.
>
> This idea looks interesting. The work to implement this might not even
> be as difficult as it may seem, just tedious since it would require
> shuffling around massive amounts of code and exhaustive regression
> tests.
>
> Anyway, I'd rather leave it out of my proposal, because it would
> (IMHO) require a larger discussion since it is a pretty serious design
> decision and we definitely won't be able to discuss it until Friday.
> (-;
>
> One thing that I don't like very much about this idea is that the
> fields would have to be aware of low-level SQL notions, such as
> aliases and joins. I think there was a reason these things were buried
> deep under several layers of code under the public API.
>
> For the purpose of this project the extra_filters should do just fine
> once extended to also take the value.
I don't want to push my proposal into your GSoC project. It'd require more
thought and discussion before I'd suggest that.
To address your concern: fields are a central component of the ORM. But
currently, instead of using methods to encapsulate field specific logic,
everything is stuffed in a single monolithic class (Query), that has to know
every possible field implementation. This leads to code that is hard to
maintain and requires coupling with code that needs more control than standard
fields (read: contenttypes).
And since `add_lookup` and a bit of public API on Query would make it possible
to implement most of your proposal without touching Django, I wanted to propose
it soon enough to have a possible influence on design decisions.
Regardless, I look forward to CompositeFields. Good luck with your application!
__
Johannes
--
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.
