is_subquery is not always set

[paj]

Sometimes subqueries do not get the is_subquery flag set to True. I noticed this in testlimit (orm.mapper.EagerTest) for MSSQL, as that produces a query with a subquery that has an ORDER BY, cause MSSQL to fail. I've attached a minimal test case that triggers the behaviour.

The query seems to become a subquery at the point that a condition that uses it is added to our main query. In Select._append_condition, the _wherecorrelator traverses the added condition, marking is_subquery as appropriate. However, nothing traverses any conditions added in _process_froms. I've created a patch with adds a _fromcorrelator that marks is_subquery, but does not correlate tables in the from clause. I've added calls to this from _process_froms.

This causes another problem, in that get_children is now called in situations it wasn't previously. I've done a minor knock-on change to fix that.

The patch fixes my problem, and all the unit tests still pass for sqlite.

[Mike Bayer]

whats the "bug" we're trying to fix here ? in fact if i apply the patch, now sqlite cannot execute the following query:

st = select(['*']('*'), crit)

because the labeled inner "id" column conflicts with t1's "id" column (the only place is_subquery is used is for a sqlite-needed column labeling hack in ansisql.py). i actually cant get any query to work with it.

the bigger question, what do you need to use is_subquery for ? id want to make it dedicated for informational purposes only and add some other flag for its current sqlite-fixing purpose if you need it for something.

[paj]

Ok, the root of all this is MSSQL not allowing an ORDER BY clause in any subquery. I produced a patch to rectify this, that uses is_subquery to know whether to suppress the order by. The problem with that was some queries still failed. My original reckoning was this was a bug on is_subquery.

Well, now you've explained the purpose of is_subquery, I think we need to flags. One for from-clause subqueries and one for others. In fact, how about we have: is_subquery - true if the select is ANY kind of subquery is_fromquery - true if the select is in the from clause of another select

Recode sqlite to say "is_subquery and not is_fromquery".

If that's ok with you, I'll prepare a patch for this. I'll include a unit test for the bug my previous patch introduced.

[Mike Bayer]

i kind of figured thats what you needed it for. lets name the new flag "is_selected_from" and I believe the sqlite hack inside of ansisql.py needs to take effect if this flag is true.

[paj]

Proposed patch

[Mike Bayer]

worked around these changes in changeset:2496. note that the ms-sql test was moved to a string-based test in select.py. if you are testing the actual format of generated SQL strings, its best to test the strings directly ! also changed the behavior of the "mini labels" to go based only on is_selected_from, which is all sqlite needs, so now we have fewer superfluous column labels generated for subqueries that dont really need it.

[Mike Bayer]

Removing milestone: 0.3.7 (automated comment)