auto-correlation has reverse defaults with Query vs. select()

[Mike Bayer]

call to correlate=False inside of _compile_context() takes effect for subquery(), statement, as_scalar(). This is inconsistent vs. that of select().

since we already pulled the trigger on 0.7 need to push this into 0.8.

[Mike Bayer]

patch:

diff -r ba299476b827ada34d01360e3024f87dd56dc967 lib/sqlalchemy/orm/query.py
--- a/lib/sqlalchemy/orm/query.py   Thu May 26 13:30:26 2011 -0400
+++ b/lib/sqlalchemy/orm/query.py   Thu May 26 17:21:16 2011 -0400
@@ -2473,7 +2473,7 @@
                         context.whereclause,
                         from_obj=froms,
                         use_labels=labels,
-                        correlate=False,
+                        #correlate=False,
                         order_by=context.order_by,
                         **self._select_args
                     )
@@ -2536,7 +2536,7 @@
                             from_obj=froms,
                             use_labels=labels,
                             for_update=for_update,
-                            correlate=False,
+                            #correlate=False,
                             order_by=context.order_by,
                             **self._select_args
                         )
diff -r ba299476b827ada34d01360e3024f87dd56dc967 lib/sqlalchemy/orm/strategies.py
--- a/lib/sqlalchemy/orm/strategies.py  Thu May 26 13:30:26 2011 -0400
+++ b/lib/sqlalchemy/orm/strategies.py  Thu May 26 17:21:16 2011 -0400
@@ -720,7 +720,7 @@

         # reformat the original query
         # to look only for significant columns
-        q = orig_query._clone()
+        q = orig_query._clone().correlate(None)

         # TODO: why does polymporphic etc. require hardcoding 
         # into _adapt_col_list ?  Does query.add_columns(...) work

[Former user]

(original author: diana)

[Former user]

(original author: diana) The above tests really only cover the second #correlate=False case in _compile_context().

I couldn't for the life of me come up with tests that failed (and then passed) by changing the first #correlate=False (if context.multi_row_eager_loaders and self._should_nest_selectable). I did come up with test cases that exercised that code path (by adding a joinedload and a limit by), but it didn't seem to matter if correlate was false or true.

The orig_query._clone().correlate(None) case is covered a bit by test.orm.test_subquery_relations.EagerTest.test_orderby_related.

I also had tests that tested .label('address_count') instead of as_scalar() wrt. auto-correlate, but I suspect they're really testing the same code path (though I didn't actually verify that), so I didn't include them in the patch.

I think this is as far as I can go with this one, without help -- I'm not sure how to exercise the other two cases. I did try! :)

Cheers.

[Former user]

(original author: diana) Toying with pull requests instead of patches.

https://bitbucket.org/sqlalchemy/sqlalchemy/pull-request/5/make-auto-correlation-the-same-for-query

[Mike Bayer]

8b16cd876cb157d6f06863f2e345ff3e1d56972f in the 0.8 branch

[Mike Bayer]

Removing milestone: 0.8.0b1 (automated comment)