-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix #2268: fix expression parsing bug #2288
Conversation
Can you explain the |
@benbjohnson sometimes it needs to replace the entire expression tree and other times in descents into the tree and modifies a sub node. I hate this change...I'm sure there's a cleaner way that I'm just not seeing. This was what I wanted to pair on yesterday. I have an idea that'll I'll try. If that doesn't clean it up then maybe we can pair for 15 - 20 min later today. |
Parsing: WHERE time > now() - 2d AND time < now() + 10d generated an expression tree that evaluated as: ... AND (time < now()) + 10d instead of: ... AND time < (now() + 10d)
@dgnorton sure, sounds good. |
👍 if you can make ben happy. 💃 |
works for me |
} | ||
} else { | ||
expr = &BinaryExpr{LHS: expr, RHS: rhs, Op: op} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: looks like a superfluous blank line here.
Some super-picky nits, test case looks good. +1 on green build. |
fix #2268: fix expression parsing bug
@benbjohnson
Before the fix...
After the fix...