Fix selections that have inner inputs. #1275
Conversation
llllllllll
added
bug
expression core
expression extension
and removed
expression core
labels
| return compute_up(expr, data, predicate, scope=scope, **kwargs) | ||
|
|
||
|
|
||
| @dispatch(Selection, sa.sql.ColumnElement, ClauseElement) |
There was a problem hiding this comment.
Should ClauseElement be sa.sql.expression.BinaryExpression?
There was a problem hiding this comment.
ClauseElement is very general and would let basically any sqlalchemy expression in.
There was a problem hiding this comment.
Is BinaryExpression too restrictive here? Are there any valid predicate functions that don't fall into this? Is there any ClauseElement that we would not treat as a valid predicate?
There was a problem hiding this comment.
Yes, Selectable is a subclass of ClauseElement and we don't support non-column boolean expressions.
There was a problem hiding this comment.
I tried making this BinaryExpression but it looks like there are some cases where we are getting a BooleanClauseList. The first common parrent is ColumnElement, does that seem restrictive enough?
|
looks like I broke everything. I will clean the other backends up and ping you when its passing. |
llllllllll
changed the title
llllllllll
force-pushed
the
select-inputs
branch
from
aef8bec
to
4d2707f
Compare
|
related #1230 I need to get back to this. I haven't had much time to work on blaze recently ;_; |
llllllllll
force-pushed
the
select-inputs
branch
3 times, most recently
from
e4e8914
to
f692fec
Compare
|
Hoping the tests pass. This should be ready to review so I am removing the wip tag. |
llllllllll
changed the title
|
@cpcloud any thoughts on this? Currently using this for our deploy so it would be good to get this merged. |
llllllllll
force-pushed
the
select-inputs
branch
from
5eec84c
to
573bcfb
Compare
Things like a[a.a == b.a] would fail because it wouldn't compute the b.a.
llllllllll
force-pushed
the
select-inputs
branch
from
573bcfb
to
afb3b18
Compare
This is a tighter restriction because we don't want selects for the predicate.
|
Not sure if the tests are still failing with a 500 from binstar, any idea what caused (or is still causing) this? edit: seems fine now |
|
+1 here |
good to merge? |
Fix selections that have inner inputs.
Things like
a[a.a == b.a]would fail because it wouldn't compute theb.a.