Skip to content
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

Potential solve for #200 #396

Open
wants to merge 3 commits into
base: master
from
Open

Potential solve for #200 #396

wants to merge 3 commits into from

Conversation

@JeroennC
Copy link

@JeroennC JeroennC commented Feb 17, 2017

Hi,

I've changed around the grammar a bit in order to support CASE WHENs being used as Conditions on their own (see #200). This means that the THENs and ELSEs have to be able to contain full expressions. One of the problems that made this not work was that Condition() would be okay with turning into a Column or a 0 or 1. I've removed this and added Conditions becoming SimpleExpressions. This does however mean that it will accept any SimpleExpression in place of a condition, for instance:

SELECT * FROM t1 WHERE 5

or

SELECT * FROM t1 WHERE a + b

Which is also why I removed one of the tests what ensure this is not parse-able. I would argue that while it may indeed not be valid SQL (depending on the DB's implementation), it is no harm if it is actually parse-able. What do you think of this?

JeroennC added 3 commits Feb 17, 2017
…onditions besides SimpleExpressions. Swap Regular and SQL Condition around in Condition().
… become Expressions. Add Columns to SimpleExpression.
@coveralls
Copy link

@coveralls coveralls commented Feb 17, 2017

Coverage Status

Coverage remained the same at 84.714% when pulling a990675 on JeroennC:master into 320f64a on JSQLParser:master.

@tomershay
Copy link
Contributor

@tomershay tomershay commented Jan 28, 2018

@JeroennC, Thanks for providing this fix. I merged it to my own custom branch to see if it's working and it worked perfectly.
@wumpz - did you consider pulling this request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.