is_subquery is not always set

Issue #513 resolved
paj created an issue

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.

Comments (7)

  1. Mike Bayer repo owner

    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.

  2. paj reporter

    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.

  3. Mike Bayer repo owner

    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.

  4. Mike Bayer repo owner

    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.

  5. Log in to comment