Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  closed
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by jeremyt):

 Thanks akaarial for censoring this ticket throughout it's lifetime. Your
 constant feedback and contributions was very much appreciated and a huge
 part of getting this done.. 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 unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  closed
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"9c4882b391157f6aa3c0ade309638ffcd3bdf684"]:
 {{{
 #!CommitTicketReference repository=""
 revision="9c4882b391157f6aa3c0ade309638ffcd3bdf684"
 Fixed Oracle specific failures in multicolumn joins

 Refs #19385
 }}}

-- 
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 unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  closed
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"0e0eb8a9553bb89347ceb159537ccc417cb5e7f5"]:
 {{{
 #!CommitTicketReference repository=""
 revision="0e0eb8a9553bb89347ceb159537ccc417cb5e7f5"
 Fixed field.rel.field_name handling

 This is a regression fix to multicolumn joins. Refs #19385.
 }}}

-- 
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 unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  closed
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"97774429aeb54df4c09895c07cd1b09e70201f7d"]:
 {{{
 #!CommitTicketReference repository=""
 revision="97774429aeb54df4c09895c07cd1b09e70201f7d"
 Fixed #19385 again, now with real code changes

 The commit of 266de5f9ae9e9f2fbfaec3b7e4b5fb9941967801 included only
 tests, this time also code changes included...
 }}}

-- 
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 unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  closed
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:  fixed
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by akaariai):

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


Comment:

 This got fixed in [266de5f9ae9e9f2fbfaec3b7e4b5fb9941967801].

 I did still some improvements to how GenericRelation works, it is now pure
 virtual field and it isn't m2m at all. Things seem to work nicely.

 I guarantee that the commit will introduce regressions, and that there are
 things that do not work as wanted with multicolumn joins and/or when using
 plain ForeignObject. Still, I (obviously) think that commit was the right
 thing to do, as writing a perfect patch would have meant refactoring
 model._meta field representations, and even bigger refactor to
 fields/related.py. 700 lines of changed ORM code in one patch is enough...

 A big thank you to Jeremy Tillman for the patch!

-- 
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 unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 Further problems found: what to do in Model initialization? It should be
 possible to initialize fields using ForeignObject based fields, but at the
 same time this should not interfere with loading from database (*args
 based initialization with only concrete fields). The resolution for now is
 to do *args based initialization using concrete fields, but !**kwargs
 based initialization using all fields.

 ModelForms now skip any non-concrete fields for ModelForms. This isn't a
 nice solution, but works for now. If one wishes to create ModelForms for
 models having ForeignObjects, then manual removal of the concrete fields
 and insertion of a form field for the ForeignObject field will need to be
 done. That is, if you want a select widget of persons instead of
 first_name and last_name text fields you will need to do that yourself.

 Currenly the ForeignObject does not integrate with other parts of Django.
 It is possible to do some ORM tricks with it but that's it. The rest of
 virtual field support needs to be fleshed out later on. As such
 documenting ForeignObjects at all seems like a bad idea. At least the
 representation in model._meta needs to change later on.

 Interestingly using @cached_attribute gave a small speed increase to model
 initialization, so performance problems in that part of the patch aren't a
 concern.

 So, the ForeignObject isn't public and it will be only barely usable in
 1.6. It is very likely that many parts of Django will crash if trying to
 deal with a model having a ForeignObject field. Still, I think commit to
 1.6 is worth it, ForeignObject as is allows for some nice things already,
 and this way there is possibility to find out the problem spots without
 committing to public API yet. The related fields implementation is a tiny
 bit cleaner, and public API use should not suffer (though some regressions
 are likely). As long as a project doesn't play with ForeignObjects
 directly there should not be much problems.

 A squashed version of the patch is available from
 
https://github.com/akaariai/django/commit/4810cefd0a99f6963ad57415dd8771e8b2b4a279.
 I will still need to do a final review of the patch, and confirm that
 commit of only ORM parts of multicolumn joins makes sense.

-- 
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 unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 Reading through the patch I found a couple of things in need of fixing or
 at least closer inspection before this can be committed:
  - column_fields, local_column_fields -> concrete_fields,
 local_concrete_fields, do these need better caching for `__init__` speed?
  - requires_unique_target validation must check unique_together indexes,
 too
  - Model.`__init__`, "# This field will be populated on request" if
 condition cleanup
  - possible problems from related field lookup value resolving (the
 _pk_trace() change). The new code seems to be doing better job than the
 old code (validating that the related model is of right class, not blindly
 using PK instead of the relation's real target field). If regressions
 happen here they should be somewhat straightforward to fix.
  - native_related_fields -> better name?

 I was thinking of renaming all [*_]related_fields to [*_]relation_fields.
 Also, rename native_related_fields to local_relation_fields,
 foreign_related_fields to remote_relation_fields. Similar for the
 get_..._value() methods.

 Otherwise I think this patch is ready. split_exclude() at least doesn't
 yet work correctly for multicol joins, but that can be fixed later on.
 There are likely other places needing some work, too. All in all there is
 somewhat big risk of regressions but IMO the risk is worth it.

 If anybody wants to do a review of this now is the time. Doing a quick API
 level check would be good. Commit is going to happen real soon now.

 Abstract of the patch for those not wanting to read through the whole
 patch: the patch adds models.fields.related.ForeignObject which is a
 virtual field marking a connection from_fields -> to_fields. In addition
 the patch adds support for traversing these multicolumn connections into
 ORM. When plain ForeignObject is used it is added to ._meta.fields. Those
 fields which have a concrete database column will be present in
 ._meta.concrete_fields, too (still named column_fields in the patch).
 ForeignKey and GenericRelation are special cases of ForeignObject,
 ForeignKey has just single from_fields and to_fields entries, and also has
 a concrete column. GenericRelation is still using ._meta.virtual_fields
 only. After the patch it will be possible to make the ORM handle
 multicolumn relations even if multicolumn ForeignKey isn't available using
 public APIs. Examples of what can be done is in
 tests/foreign_objects/models.py and tests.py.

-- 
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 unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 I did a rebase of the patch. While working on the patch I saw three things
 worth mention:
   - Generally looking very good
   - Unused imports, other PEP-8 code cleanup required
   - Some naming issues in ForeignObject.*_related_fields (I find them a
 bit hard to remember). The naming issues might be OK to postpone after
 commit...

 The rebase commit tells also what else apart of just making this run was
 done. Most important change is using ForeignObject as base of
 GenericRelation, too.

 I was too lazy to do the rebase per commit, so the commits before HEAD
 aren't passing tests.

 I will be on holiday next week, but after that I think I will just do
 final cleanup & commit. Polishing this outside master will just lead to
 more rebase conflicts. Meanwhile, using SubqueryConstraint in
 split_exclude() isn't forbidden... :)

 Rebase available from:
 https://github.com/akaariai/django/compare/ticket_19385_2_rebased

-- 
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 unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 I have done some testing, and this is looking excellent. You have
 convinced me that the ForeignObject is a good idea. I tried the
 ForeignObject with ActiveTranslationField (a "reverse foreign key"
 fetching current active language's translations). There is need for couple
 of simple additions to ForeignObject, but otherwise things seems to just
 work.

 The tests & changes for translation fields are here:
 
https://github.com/akaariai/django/commit/790fc1693744fe3a8f407080527a0c70b1f4a759.
 This is not intended for inclusion in your patch as is, but I would like
 to add the ability to do the reverse joining stuff at some point. Only
 minor changes.

 The get_extra_join_sql() method could likely be replaced with
 get_extra_filter(). The get_extra_filter() could work for both the
 descriptor and extra_join_restriction cases. In addition this would be
 much better than generating raw SQL for custom backend support. This is
 not needed in this ticket, but definitely something to think about for
 future work.

 Unfortunately it seems I will be very busy still for the couple next
 weeks, so I can't promise a full review & commit soon. What I have managed
 to review so far suggests that there isn't much work to do.

-- 
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 unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 Sorry for not replying sooner, I've been busy lately...

 I just took a quick look at the second approach and I think I like it. It
 seems the ForeignObject could have more uses than just ForeignKey, so it
 is a good addition. I will try to take a closer look soon. In any case,
 this looks to be on good track for inclusion in 1.6.

 If you have interest in reviewing / working on other ORM tickets that
 would be a good way to make sure I will have time to review this ticket...

-- 
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 unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by jeremyt):

 I've made a seperate branch and pull request with the second option as
 discussed above. It may be commented on and I will polish this version up
 with comments and so forth.

 Branch: [https://github.com/jtillman/django/compare/ticket_19385_2]
 Pull Request: [https://github.com/django/django/pull/680]

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by jeremyt):

 Pull Request updated concerning comments.

 To the point of the contrib portion, I would like to put this
 functionality in related.py. I have extra commits that moves this to a
 ForeignObject field. ForeignObject is basically the join_field of a
 JoinInfo tuple as ForeignKey is today. It is represented as a virtual
 field so if preferred, we can integrate konk's change. ForeignKey will
 inherit ForeignObject and add properties to make the field non-virtual to
 retain it being used as a column.

 If we want to got this route (my vote goes here), I'm ready to pull the
 trigger so let me know.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by jeremyt):

 Pull Request created [https://github.com/django/django/pull/656]

 I haven't made changes for previous comment yet but just pulled what I
 have committed. I will move the ForeignKeyEx out and placing the
 functionality as 'JoinField' which will always be the type of field for
 PathInfo.join_field. Please comment away and I will continuously integrate
 them.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 Could you make a pull request? There are various pieces of code I'd like
 to comment on, and the best tool to do that is the github PR on-line
 commenting. (While at it, could you remove the whitespace errors, git diff
 --check is your friend here).

 I did spend already some time reviewing the code. I think the changes are
 mostly good, though there are a couple of issues I'd like to tackle, best
 reviewed in a PR.

 Also, I still think the ForeignKeyEx doesn't belong into contrib, it
 should be defined in a test module. It is a good test case to see we
 really have working multicolumn joins, but it is unlikely that such a
 field would be accepted to django contrib. A discussion at django-
 developers is needed at least.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 On a quick glance, looks good!

 However, I don't think we want to add a new field to contrib. We either
 want to make ForeignKey work with multicolumn joins (even if there is no
 possibility for multicolumn PKs yet) or we want to leave the multicolumn
 foreign key implementation to external apps.

 We are basically in plumbing stage here - we are improving the internals.
 After done, the patch that adds the support in public APIs should be
 somewhat more manageable. Still, I think it is worth it to test the
 internals. So, maybe we could add the contrib field in a test module
 instead?

 Making ForeignKeys work properly will likely need the virtual fields
 support konk has been working on. We really should wait and see if he
 can continue his work.

 In any case, the branch contains multiple improvements that we should get
 committed. Likely the whole stuff targeting models/sql and fields is good
 to go. I will try to find the time to review the patch as soon as
 possible.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by jeremyt):

 [https://github.com/jtillman/django/compare/ticket_19385] rebased with
 recent updated.

 All above plans have been implemented.

 What Changed:
  *To and From field on PathInfo was removed and replaced with
 'target_fields'
  *Trimming can happen on multicolumns
  *Added 'get_lookup_constraint' to ForeignKey Field. It can build
 constraints based on its target
  *Sending a query object to get_lookup_constraint will add a
 SubqueryConstraint that may use EXISTS if multiple targets

 Unit tests are all passing

 I did find some interesting inconsistencies though that currently exist in
 the framework, especially in subqueries.
 This seems to be a bug when using to_field (Using
 regressiontests.queries:ToFieldTests.test_recursive_fk):
  *Node.objects.filter(parent=node1) => SELECT "queries_node"."id",
 "queries_node"."num", "queries_node"."parent_id" FROM "queries_node" WHERE
 "queries_node"."parent_id" = 42
   -but-
  *Node.objects.filter(parent=Node.objects.filter(id=node1.id)) => SELECT
 "queries_node"."id", "queries_node"."num", "queries_node"."parent_id" FROM
 "queries_node" WHERE "queries_node"."parent_id" =  (SELECT U0."id" FROM
 "queries_node" U0 WHERE U0."id" = 1 )

  -I believe the subquery will alway be incorrect when using a to_field
 that isn't 'pk' of the target model. If this isn't a known issue then I'll
 open a ticket for it, but this has been fixed with my change.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by jeremyt):

 [https://github.com/jtillman/django/compare/ticket_19385] rebased with
 recent updated. Good additions btw.

 Here are my thoughts/plans for the current list:
 * IN lookups:
  * Multicolumn could be done in mysql with multicolumn IN with format
 "SELECT * FROM T1 WHERE (T1.col1, T1.col2) IN ((11, 12), (21, 22))" but
 this isn't widely support by all databases so I'm staying away.
  * This could be done by converting the IN filter into a Q object which
 explains the IN using ANDs and ORs of single fields as done in
 prefetching. Either we could create a field lookup that return a list of
 constraints or create a method on the field that takes in the query and
 adds the constraints itself..(I favor the latter).
 * Subquery support
  * Could use multicolumn IN here too but no for same reason
  * I agree with the use of EXISTS over IN but I'm a little hesitant on
 this because mysql struggles with dependent subqueries (supposed to be
 fixed in 5.6). An option would be to special case single column relations
 and use IN and use exists for multicase scenario.
 * Join trimming
  * Shouldn't change much but we'll have to return a list of columns in
 trim_joins I'm thinking???
 * Representation of the foreign key in the models's _meta
  * I'm currently not sure why we need this one.

 Thoughts?

 All feedback/discussion welcome.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"f80a1934cdb465c70ecc215fdf867e555a670623"]:
 {{{
 #!CommitTicketReference repository=""
 revision="f80a1934cdb465c70ecc215fdf867e555a670623"
 Fixed GIS regression in get_default_columns()

 I changed the normal compiler's get_default_columns() but didn't change
 the copy-pasted code in GIS compiler's get_default_columns().

 Refs #19385
 }}}

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 The names_to_path_to_fields stuff is in master now.

 I think the next step is the possibility to have multiple columns per
 join, then try to fix all the possible places which break under
 multicolumn joins in the ORM. Here is a partial list:
   - IN lookups
   - What other lookups to support and how to do this
   - Subquery support (we should use EXISTS instead of IN subqueries)
   - Join trimming
   - Representation of the foreign key in the model's _meta.

 I am sure I am missing half of the complications.

 I think we will want to abandon the get_join_clause() approach. This means
 there will be too little information available in the ORM, and we would
 need to move stuff like "can we trim this join" into fields which doesn't
 seem right. However, we will likely want to support the case of _no_ join
 columns, so that one can generate the whole clause in the
 field.get_extra_join_sql(). (Also, we might want to reconsider the name
 AND implementation of the get_extra_join_sql() method before 1.6...)

 It seems the work done in
 https://github.com/jtillman/django/compare/ticket_19385 is a good basis
 for doing further work on the ORM changes. We should also see what
 konk has done so that we won't be duplicating or invalidating his work
 here.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"807eff74396faba24ad420236d83e9716feffe1e"]:
 {{{
 #!CommitTicketReference repository=""
 revision="807eff74396faba24ad420236d83e9716feffe1e"
 Made use of PathInfo.direct flag in trim_joins

 Refs #19385
 }}}

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"ce3c71faf1ff83494193b48a6f99256d0628b2a2"]:
 {{{
 #!CommitTicketReference repository=""
 revision="ce3c71faf1ff83494193b48a6f99256d0628b2a2"
 Minor improvement to proxy model handling

 Refs #19385
 }}}

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"9ef3cab40b1806d99172e94954976413f860a476"]:
 {{{
 #!CommitTicketReference repository=""
 revision="9ef3cab40b1806d99172e94954976413f860a476"
 Made sure join_field is always available in .join()

 Refs #19385
 }}}

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"68985db48212c701a3d975636123a5d79bdc006f"]:
 {{{
 #!CommitTicketReference repository=""
 revision="68985db48212c701a3d975636123a5d79bdc006f"
 Added Query.join_parent_model()

 This simplifies especially compiler.py a lot, where almost the same
 code was repeated multiple times.

 Refs #19385
 }}}

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"4511aeb6b8b843ee913fb43a37c9686980210948"]:
 {{{
 #!CommitTicketReference repository=""
 revision="4511aeb6b8b843ee913fb43a37c9686980210948"
 Moved join path generation to Field

 Refs #19385
 }}}

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by jeremyt):

 Nice. I like the field being able to explain the path no matter how
 many table jumps it has. Is there a reason we do 'if hasattr(field,
 'get_path_info'):' instead of 'if issubclass(field, RelatedField)' 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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 jeremyt: Your version adds information about multiple columns into the
 compiler.py. This seems like a good straightforward approach which allows
 for multicolumn joins with minimal hassle.

 The patch from https://github.com/akaariai/django/compare/ticket_19385
 moves the whole ON clause generation to the field. I like the idea of
 completely abstracting the related join handling to the field. The worry
 is that once all cases (like join trimming, IN filtering,
 prefetch_related, ...) are handled by the field, then the API between ORM
 and the relation fields will be too complicated to actually use.

 Will have to think about this.

 Anyways, I cherry-picked the most obvious changes from the patch for this
 ticket into separate branch. See
 https://github.com/akaariai/django/compare/names_to_path_to_fields. I plan
 to commit this branch separately, it is a major cleanup to parent model
 joining, and to names_to_path().

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by jeremyt):

 I've updated my patch to work with the new changes in master, found at
 [https://github.com/jtillman/django/compare/ticket_19385]. I also I have
 branch that I run which is based off of 1.4.3 [
 https://github.com/jtillman/django/tree/MultiColumnJoin].

 To your point about the IN statement, this doesn't currently work in the
 case of multi-column because of the use of _pk_trace. I'm thinking of
 doing the same method as I do for create_prefetch_query and using IN for
 single columns and complex queries for multiple columns while change
 _pk_trace to return a tuple of values.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  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:

 Now with basic select_related and order_by support. The field <-> ORM
 interface contains a bit more stuff if you want to support
 select_related(), but it isn't huge by any standard:
   - four attributes: model, null, unique, name
   - get_path_info() and get_join_sql() as described in above comments
   - select_related_descent() - should this field be descended into?
 Parameters contain info about what is requested.
   - get_cache_name() and get_related_cache_name() - when doing
 select_related fetches to what attribute should the fetched value be
 saved.

 However, I had to do a lot of hackery to fake model._meta support in
 tests. So, model._meta refactor would be needed next.

 Note that the goal is not to make the Field <-> ORM interface public
 anytime soon. I am afraid making it stable would make it hard to do
 further work on the ORM.

 As for `IN` support etc - it seems there could be some hard problems
 awaiting in that path regarding partially NULL composite values and so on.

 I think polishing the current work and then pushing the work to master
 could be a good plan... But, enough of this ticket for now, time to
 concentrate on other issues for a while.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 Some more work... I moved names_to_path path generation to
 Field.get_path_info(). This seems to be a really nice cleanup and also
 makes the abstraction between fields and the ORM a lot better.

 I also added multicolumn join tests. The test models are _really_ ugly
 because the model._meta doesn't have any support for custom relation
 fields. Even with this limitation the basic joining seems to work. There
 isn't any support for select_related, defer or stuff like that, and there
 isn't support for customrelfield=val filters. Still, to me the tests seem
 like a strong indication that the work is on the right path. The interface
 needed for basic join support is really simple: get_path_info() and
 get_join_sql().

 In my opinion the next step would be to add proper ._meta support for
 custom relation fields. This would likely mean cherry-picking stuff from
 #373. Then the next steps would be to find problematic queries in ORM, fix
 em, repeat. Finally make sure other parts of Django support multicolumn
 fields.

 I am wondering if it might be a good idea to do some polish to what I have
 now and commit it. To me it seems the code in
 https://github.com/akaariai/django/compare/ticket_19385 is a step forward
 even if multicolumn joins aren't considered at all.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 Yes, I bet there are a bunch of places which will not work, not least
 subquerying support.

 Anyways, I have created a patch for generating the ON clause in the field
 itself. It seems to work pretty nicely, and is IMO a minor cleanup to
 current code. While there I spotted some other cleanups, too, which are
 included in the patch. See:
 https://github.com/akaariai/django/compare/ticket_19385

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Alex):

 Yes, this is clearly going to be non-trivial under any circumstances, for
 example `field__in` can no longer generate a SQL `IN` clause, it needs to
 be a chain of `OR`s.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 I am somewhat interested in the idea of doing the "ON ..." clause
 generation in the field. This would allow for a lot of freedom in what the
 join clause would be and would be fairly straightforward to implement. As
 of commit of #10790 the get_extra_join_sql() already allows adding custom
 stuff to the join clause, but not altering it totally.

 It seems that for generic foreign keys we will need the ability to
 generate fully custom ON clauses for multicolumn foreign key support (see
 #373, comment 111). And, this SQL will need to be backend specific.

 For now I see two basic approaches:
   - Just change the lhscol, rhscol in query datastructures to lists, that
 is lhscols, rhscols. Deal with the generic foreign key in some other way
 (basically, add a hack for it).
   - Generate the SQL in the field (get_join_clause() or somesuch). This
 might need to just postpone the clause creation to the used connection.
 This way generic foreign keys wouldn't be a special case for the compiler,
 and it would be possible to implement exotic join clauses in custom fields
 (time range join restrictions for example).

 I guess we can't implement the ForeignKey(to_field=a list of fields) just
 yet, there are some complications along that path - mainly how do we
 represent the ForeignKey in the model._meta as it doesn't have just a
 single backing field. I guess #373 will need to answer that part.

 Good ideas welcome. Testing different things will likely be a good
 solution here.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Alex):

 I think Anssii, is correct that the right solution is to support "virtual
 fields" (and with the multi column primary keys), and have that work with
 foreign keys. However, there could be an argument made that ForeignKey
 needs some support of its own, specifically with the `to_field` otpion,
 which lets you point a foreign key at something besides a primary key,
 it's not unreasonable to say that `to_field` should accept a list of
 fields. I suspect however that doing this correctly would still require
 most of the work of #373, so it should happen afterwards.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:  Accepted
 Keywords:  join |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by akaariai):

 * stage:  Unreviewed => Accepted


Comment:

 IMO it is a good plan to lift the assumption about single column joins
 from the ORM internals.

 There are some cases the patch doesn't seem to deal with (split_exclude()
 -> subquery support, `__in` lookups, there are probably more). But, we can
 remove the single column limitations one at a time. We don't have to
 guarantee that multicolumn joins work in all cases.

 We can't do these changes to 1.4. 1.5 is also out, doing big changes to
 sql/query.py internals this close to release is IMO a bad idea - even if
 the sql/query.py is 100% private API we should try to keep it stable from
 RC stage on. In master we are free to change sql/* if need be.

 But, you don't want to write a patch for master just yet, #10790 is going
 to change setup_joins() a lot, so there will be a lot of conflicts.

 I am marking this as accepted in the meaning that we should do incremental
 work and remove assumptions about single-column joins from the ORM
 internals. We should not aim for public API for multicolumn primary keys &
 joins in this ticket, #373 deals with that.

 We should also check what konk's summer of code work has to offer
 (https://github.com/konk/django/tree/soc2011/composite-fields). There
 is also another branch for foreign keys:
 https://github.com/konk/django/tree/auxiliary_fk_fields.

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:  join |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by jtillman):

 I've found that the major challenge here is the global assumption that
 model relations will always have single column comparisons. The majority
 of the patch tries to change this assumption by directing all relations to
 depend on ForeignKey.get_related_columns in some fashion. If the
 'include_related' argument was removed and the premise of
 get_related_columns was more wide spread, would this seem more of a
 reasonable patch to enable the subclassing scenarios?

-- 
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 https://groups.google.com/groups/opt_out.

Re: [Django] #19385: Add support for multiple-column join

[Django]

#19385: Add support for multiple-column join
-+-
 Reporter:  cseibert |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:  join |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by akaariai):

 * needs_better_patch:   => 0
 * needs_tests:   => 0
 * needs_docs:   => 0


Comment:

 My feeling is that Django should not offer the functionality out of the
 box, but should allow somewhat easy subclassing of ForeignKeys & other
 relation fields to achieve this functionality.

 You might be interested to take a look at what is going on in master
 currently, specifically in ticket #10790,
 https://github.com/akaariai/django/tree/ticket_10790_final and commit
 
https://github.com/akaariai/django/commit/f6041f47328fc26b7cdbe9d6f604e1b1f1f4b3a9.

 For this ticket, the interesting part is the addition of join_field to the
 alias_map. If the join_field (that is, the FK / reverse relation used for
 the join) defines get_extra_join_sql(connection, qn, lhs_alias, rhs_alias)
 then the method is called in compiler.get_from_clause() and the SQL
 returned is added to the join condition. This seems to be a bit more
 generic approach than what you have done.

 The work in #10790 isn't enough to allow the functionality you are after.
 It seems some more hooks will be needed. And there are some more places to
 change, at least the select_related stuff in compiler.py needs to take the
 join_field in account.

 In addition I think we want to abstract the interaction between fields and
 the query generation code somewhat. Currently the query generation code
 knows too much of the implementation of the fields which leads to somewhat
 hard to understand code in setup_joins(). Abstraction would also allow for
 easier creation of custom relation fields.

 I would be very interested to hear what you have to say about the approach
 taken in #10790.

 I am not sure what to do with this ticket. I definitely support the idea
 of allowing ForeignKey subclasses with the functionality you are looking
 after. But I don't think we want to add those subclasses directly into
 django-core, at least not before knowing a lot more how they will function
 in practice, and which ones are the commonly used ones.

-- 
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 https://groups.google.com/groups/opt_out.