ResultSet set expression API limits size of unions and intersections

Bug #242813 reported by James Henstridge on 2008-06-25
2
Affects Status Importance Assigned to Milestone
Storm
Low
James Henstridge

Bug Description

The set expression API of ResultSet allows you to combine two result sets together. However, using it multiple times results in expression object hierarchies like Union(Union(Union(select1, select2), select3, select4). Compiling this expression uses a few stack frames per Union(), so it is possible to hit the recursion limit long before producing a query that'd cause PostgreSQL trouble.

It'd be nice if the ResultSet level API made it possible to generate an expression object like Union(select1, select2, select3, select4, ...) for such cases, which would not hit the same recursion problems.

This applies for Union and Intersection, but maybe not Except (which really is a binary op).

Related branches

Changed in storm:
importance: Undecided → Low
James Henstridge (jamesh) wrote :

I think I am wrong about the API being a limiting factor here: with smarter code, we could pretty easily avoid the recursion problems by changing Store._set_expr() to something like this:

    ...
    left_select = self._get_select()
    right_select = other._get_select()
    # Try to flatten nested set expressions.
    if (isinstance(left_select, expr_cls) and left_select.all == all and
        left_select.limit is Undef and left_select.offset is Undef):
        subexprs = left_select.exprs + (right_select,)
    else:
        subexprs = (left_select, right_select)
    expr = expr_cls(*subexprs, all=all)
    ...

The idea being to take advantage of left associativity of set operations to generate a shallower expression tree. It looks like this would bring the same benefit to the SQLObject compatibility layer with no changes.

Changed in storm:
assignee: nobody → James Henstridge (jamesh)
milestone: none → 0.16
James Henstridge (jamesh) wrote :

Fix merged to trunk as r334.

Changed in storm:
status: New → Fix Committed
Jamu Kakar (jkakar) on 2009-11-29
Changed in storm:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers