select_related() changes

[dcr...@gmail.com]

Code Source: http://dpaste.com/hold/4538/

This replaces django/db/query.py and adds two arguments for
select_related():

depth=N, the recursion depth, by default, infinite, follows any key
where blank isn't True
fields=[], a list of fields, right now only supports fields from the
base table. I'd like to extend this to be able to do
relatedfield__fieldname, as well. if fields is not empty it will change
depth to 1

I think i've worked out all the kinks and if a few of you could give it
a workout it'd be great. I'd like this to become core functionality as
I feel its needed.

[dcr...@gmail.com]

Updated paste: http://dpaste.com/hold/4539/

Should be no problems now :)

[David Cramer]

http://dpaste.com/hold/4541/

:)

[Michael Radziej]

dcr...@gmail.com schrieb:
> Updated paste: http://dpaste.com/hold/4539/

Thanks for your code, and I would have good use for some
improvements on select_related. But it's hard for me to see what
changed in the paste.

The common way to suggest or discuss changes on this list is to
open a ticket with an attached diff and then start a discussion
refering to the ticket number. It's much easier to deal with
this, and the contributions don't get lost. Emails come and go,
tickets stay ;-)

If you have problems creating a diff or so, let us know and
someone will surely assist.

Cheers,

Michael

--
noris network AG - Deutschherrnstraße 15-19 - D-90429 Nürnberg -
Tel +49-911-9352-0 - Fax +49-911-9352-100

http://www.noris.de - The IT-Outsourcing Company

[David Cramer]

What's the svn command for generating the diff?

On Jan 10, 11:07 am, Michael Radziej <m...@noris.de> wrote:
> dcra...@gmail.com schrieb:
>
> > Updated paste:http://dpaste.com/hold/4539/Thanks for your code, and I would have good use for some

[Honza Král]

On 1/10/07, David Cramer <dcr...@gmail.com> wrote:
>
> What's the svn command for generating the diff?

svn diff

you go into the top-level directory of your checked-out django, do svn
diff and capture the output....

--
Honza Král
E-Mail: Honza...@gmail.com
ICQ#: 107471613
Phone: +420 606 678585

[David Cramer]

Any links to an example ticket so I can keep a normal format for what
im going to post? :)

On Jan 10, 11:40 am, "Honza Král" <honza.k...@gmail.com> wrote:
> On 1/10/07, David Cramer <dcra...@gmail.com> wrote:
>
>
>
> > What's the svn command for generating the diff?svn diff

>
> you go into the top-level directory of your checked-out django, do svn
> diff and capture the output....
>
>
>
>
>
> > On Jan 10, 11:07 am, Michael Radziej <m...@noris.de> wrote:
> > > dcra...@gmail.com schrieb:
>

> > > > Updated paste:http://dpaste.com/hold/4539/Thanksfor your code, and I would have good use for some

> > > improvements on select_related. But it's hard for me to see what
> > > changed in the paste.
>
> > > The common way to suggest or discuss changes on this list is to
> > > open a ticket with an attached diff and then start a discussion
> > > refering to the ticket number. It's much easier to deal with
> > > this, and the contributions don't get lost. Emails come and go,
> > > tickets stay ;-)
>
> > > If you have problems creating a diff or so, let us know and
> > > someone will surely assist.
>
> > > Cheers,
>
> > > Michael
>
> > > --
> > > noris network AG - Deutschherrnstraße 15-19 - D-90429 Nürnberg -
> > > Tel +49-911-9352-0 - Fax +49-911-9352-100
>
> > >http://www.noris.de-The IT-Outsourcing Company--
> Honza Král

> E-Mail: Honza.K...@gmail.com

[Michael Radziej]

David Cramer schrieb:

> Any links to an example ticket so I can keep a normal format for what
> im going to post? :)

General information about contributions:
http://www.djangoproject.com/documentation/contributing/

There are plenty of tickets in the timeline:
http://code.djangoproject.com/timeline

Submitting a new ticket:
http://code.djangoproject.com/newticket

Michael

--
noris network AG - Deutschherrnstraße 15-19 - D-90429 Nürnberg -
Tel +49-911-9352-0 - Fax +49-911-9352-100

http://www.noris.de - The IT-Outsourcing Company

[Nikolaus Schlemm]

> Any links to an example ticket so I can keep a normal format for what
> im going to post? :)

take a look at the tickets with patches:

http://code.djangoproject.com/report/12
--
cheers,

Nikl

[David Cramer]

http://code.djangoproject.com/ticket/3275

Thanks :)

On Jan 10, 2:50 pm, Nikolaus Schlemm <n...@nikl.net> wrote:
> > Any links to an example ticket so I can keep a normal format for what

> > im going to post? :)take a look at the tickets with patches:

[Waylan Limberg]

On 1/10/07, dcr...@gmail.com <dcr...@gmail.com> wrote:
>
> I'd like this to become core functionality as I feel its needed.

For that to happen, you will likely need to do everything listed here
[1]. That includes writing some unit tests for the new functionality
and documentation explaining how to use it. In both cases they should
be submitted as diffs, just like the code. You can attach each as a
different file, or include everything all in one diff file.

[1]: http://www.djangoproject.com/documentation/contributing/#patch-style

--
----
Waylan Limberg
way...@gmail.com

[Michael Radziej]

Waylan Limberg schrieb:

I'm willing to join in and help out, but I'm really waiting for
feedback from Adrian.

Adrian, does this have a chance? Or is the interface too limited?

[Jacob Kaplan-Moss]

On 1/10/07 9:07 AM, Waylan Limberg wrote:
> On 1/10/07, dcr...@gmail.com <dcr...@gmail.com> wrote:
>> I'd like this to become core functionality as I feel its needed.

On the surface it looks pretty good, so I'd say you're pretty close to getting
this considered for inclusion.

> For that to happen, you will likely need to do everything listed here
> [1]. That includes writing some unit tests for the new functionality
> and documentation explaining how to use it.

Exactly. We don't check anything in without associated tests and
documentation. If your patch is good enough, someone else *might* write the
docs/tests for you, but the best way to ensure your patch gets the attention
it deserves is to write those things yourself.

> In both cases they should
> be submitted as diffs, just like the code. You can attach each as a
> different file, or include everything all in one diff file.

Actually, I much prefer one single diff file. Make all the changes you need
in your source tree, and then from the root of the tree just type "svn diff".
A single monolithic patch from the root of the tree is very easy to apply
and test.

Jacob

[David Cramer]

Not quite sure what to write in terms of the documentation :)

On Jan 10, 4:07 pm, "Waylan Limberg" <way...@gmail.com> wrote:
> On 1/10/07, dcra...@gmail.com <dcra...@gmail.com> wrote:
>
>
>
> > I'd like this to become core functionality as I feel its needed.For that to happen, you will likely need to do everything listed here

[Jacob Kaplan-Moss]

On 1/10/07 9:23 AM, David Cramer wrote:
> Not quite sure what to write in terms of the documentation :)

Essentially you just need to add a few paragraphs and examples to the db_api
doc (http://www.djangoproject.com/documentation/db_api/#select-related)
explaining what the parameters are, and what they do.

Jacob

[Michael Radziej]

Hi David,

if you want me to write the docs or the tests, let me know. I
could deliver within a day or two.

[David Cramer]

I can do the docs, but it'd be a great time saver if you could do the
tests. I'm up to my elbows in work at the moment so :)

[Michael Radziej]

David Cramer schrieb:

> I can do the docs, but it'd be a great time saver if you could do the
> tests. I'm up to my elbows in work at the moment so :)

Fine for me, then I take the tests.

Michael

--
noris network AG - Deutschherrnstraße 15-19 - D-90429 Nürnberg -
Tel +49-911-9352-0 - Fax +49-911-9352-100

http://www.noris.de - The IT-Outsourcing Company

[David Cramer]

It seems there are some issues with the code. Although I'm not sure why
it's happening, this was causing servers to get extremely loaded, and
the SQL queries werent executing properly. I had no issues when running
it on runserver before we pushed it onto a live environment though.

Going to look into it some more tomorrow, if anyone has any ideas let
me know.

On Jan 10, 5:45 pm, Michael Radziej <m...@noris.de> wrote:
> David Cramer schrieb:
>
> > I can do the docs, but it'd be a great time saver if you could do the

> > tests. I'm up to my elbows in work at the moment so :)Fine for me, then I take the tests.

[David Cramer]

I retract my statement, we had some other random server configuration
error that just happened to appear at the same time as us putting this
live :)

[David Cramer]

It still seems to have a bug when just doing .select_related(depth=1),
sometimes its filling the field w/ the wrong data, looking into it.

[Boris Erdmann]

The main problem with depth=1 seems to be, that the query generator
still spans over too
many tables (watching the mysql.log i can see tables fully joined that
definitvly do not
belong to level 1). Thus the returned data set will surely confuse
django.

Looking at django/db/models/query.py reveals:

def fill_table_cache(opts, select, tables, where, old_prefix,
cache_tables_seen, max_depth=0, cur_depth=0):
"""
Helper function that recursively populates the select, tables and
where (in
place) for select_related queries.
"""

# If we've got a max_depth set and we've exceeded that depth, bail
now.
if max_depth and cur_depth > max_depth:
return None
[ ... ]
fill_table_cache(f.rel.to._meta, select, tables, where,
db_table, cache_tables_seen, max_depth, cur_depth+1)

So by just looking at it one can see that with max_depth=1 and
cur_depth initialized to zero the code will recurse
into level 2, generating a too broad query.

declaring
def fill_table_cache(opts, select, tables, where, old_prefix,
cache_tables_seen, max_depth=0, cur_depth=1):

fixes things for me. Interestingly enough the same observation doesnt
seem to yield for

def get_cached_row(klass, row, index_start, max_depth=0, cur_depth=0):

It seems to be neccessary within this function to go one level
deeper...

I am commenting rev 4661.

Boris