Skip to content

Conversation

@hvanhovell
Copy link
Contributor

What changes were proposed in this pull request?

The SQL parser can mistake a WHEN (...) used in CASE for a function call. This happens in cases like the following:

select case when (1) + case when 1 > 0 then 1 else 0 end = 2 then 1 else 0 end
from tb

This PR fixes this by re-organizing the case related parsing rules.

How was this patch tested?

Added a regression test to the ExpressionParserSuite.

@hvanhovell hvanhovell changed the title [SPARK-19472][SQL] Parser should mistake CASE WHEN(...) for a function call [SPARK-19472][SQL] Parser should not mistake CASE WHEN(...) for a function call Feb 6, 2017
@dongjoon-hyun
Copy link
Member

+1

@SparkQA
Copy link

SparkQA commented Feb 6, 2017

Test build #72454 has finished for PR 16821 at commit 870f298.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile
Copy link
Member

LGTM

assertEqual("case when a = 1 then b when a = 2 then c else d end",
CaseWhen(Seq(('a === 1, 'b.expr), ('a === 2, 'c.expr)), 'd))
assertEqual("case when (1) + case when a > b then c else d end then f else g end",
CaseWhen(Seq((Literal(1) + CaseWhen(Seq(('a > 'b, 'c.expr)), 'd.expr), 'f.expr)), 'g))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To other reviewers: before the fix, if users do not put round brackets ( ), it works well. For example, case when 1 + case when a > b then c else d end then f else g end

asfgit pushed a commit that referenced this pull request Feb 6, 2017
…ction call

## What changes were proposed in this pull request?
The SQL parser can mistake a `WHEN (...)` used in `CASE` for a function call. This happens in cases like the following:
```sql
select case when (1) + case when 1 > 0 then 1 else 0 end = 2 then 1 else 0 end
from tb
```
This PR fixes this by re-organizing the case related parsing rules.

## How was this patch tested?
Added a regression test to the `ExpressionParserSuite`.

Author: Herman van Hovell <[email protected]>

Closes #16821 from hvanhovell/SPARK-19472.

(cherry picked from commit cb2677b)
Signed-off-by: gatorsmile <[email protected]>
asfgit pushed a commit that referenced this pull request Feb 6, 2017
…ction call

## What changes were proposed in this pull request?
The SQL parser can mistake a `WHEN (...)` used in `CASE` for a function call. This happens in cases like the following:
```sql
select case when (1) + case when 1 > 0 then 1 else 0 end = 2 then 1 else 0 end
from tb
```
This PR fixes this by re-organizing the case related parsing rules.

## How was this patch tested?
Added a regression test to the `ExpressionParserSuite`.

Author: Herman van Hovell <[email protected]>

Closes #16821 from hvanhovell/SPARK-19472.

(cherry picked from commit cb2677b)
Signed-off-by: gatorsmile <[email protected]>
@gatorsmile
Copy link
Member

Thanks! Merging to master/2.1/2.0

@asfgit asfgit closed this in cb2677b Feb 6, 2017
cmonkey pushed a commit to cmonkey/spark that referenced this pull request Feb 15, 2017
…ction call

## What changes were proposed in this pull request?
The SQL parser can mistake a `WHEN (...)` used in `CASE` for a function call. This happens in cases like the following:
```sql
select case when (1) + case when 1 > 0 then 1 else 0 end = 2 then 1 else 0 end
from tb
```
This PR fixes this by re-organizing the case related parsing rules.

## How was this patch tested?
Added a regression test to the `ExpressionParserSuite`.

Author: Herman van Hovell <[email protected]>

Closes apache#16821 from hvanhovell/SPARK-19472.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants