Code review comment for lp:~jamesh/storm/bug-242813

The tests pass, and this fixes a problem I've run into, so I'm eager to see this pushed into mainline.

A few questions:

Should there be a test to explicitly check that heterogeneous set expressions are not flattened? (ie exercising the negative branch of the second if statement)

Is it necessary to skip expressions containing an order_by? If the order_by is the same on each expr, shouldn't the result of the nested and un-nested expressions be equivalent? (I may very well be totally wrong about this, but I can't seem to cook up a counter-example at the moment)

It appears that in addition to addressing https://bugs.edge.launchpad.net/storm/+bug/242813 this patch also should fix some of https://bugs.edge.launchpad.net/storm/+bug/309276 (which is a problem I'm having), however I don't think it addresses it fully. The proposed solution doesn't tackle heterogeneous nested set queries, which will still fail on MySQL. http://bugs.mysql.com/bug.php?id=3347 http://bugs.mysql.com/bug.php?id=2435 and http://bugs.mysql.com/bug.php?id=7360 seem to imply that there is a problem with MySQL that basically makes some such nested queries impossible to express, however simpler ones like (SELECT ...) UNION (SELECT ...) INTERSECT (SELECT ...) are supported in MySQL, but (I believe) can't be achieved through Storm itself.

Could/should there be a more graceful failure for these cases than a MySQL parse error? (Sorry if this is too far off track. Perhaps this discussion would be better continued on the appropriate bug? I've subscribed myself)

Cheers!