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

[SPARK-3813][SQL] Support "case when" conditional functions in Spark SQL. #2678

Closed
wants to merge 3 commits into from

Conversation

ravipesala
Copy link
Contributor

"case when" conditional function is already supported in Spark SQL but there is no support in SqlParser. So added parser support to it.

Author : ravipesala ravindra.pesala@huawei.com

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@@ -333,6 +338,24 @@ class SqlParser extends StandardTokenParsers with PackratParsers {
IF ~> "(" ~> expression ~ "," ~ expression ~ "," ~ expression <~ ")" ^^ {
case c ~ "," ~ t ~ "," ~ f => If(c,t,f)
} |
CASE ~> opt(expression) ~ (WHEN ~ expression ~ THEN ~ expression).* ~
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer expression.? to opt(expression) since you're already using postfix style like .*.

CASE ~> expression.? ~ (WHEN ~> expression ~ (THEN ~> expression)).* ~
(ELSE ~> expression).? <~ END ^^ {
case casePart ~ altPart ~ elsePart =>
val altExprs = altPart.flatMap{
Copy link
Contributor

Choose a reason for hiding this comment

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

Space missing before {.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Updated

@SparkQA
Copy link

SparkQA commented Oct 7, 2014

QA tests have started for PR 2678 at commit 709684f.

  • This patch merges cleanly.

case casePart ~ altPart ~ elsePart =>
val altExprs = altPart.flatMap{
case we ~ te =>
Seq(casePart.fold(we)(EqualTo(_, we)),te)
Copy link
Contributor

Choose a reason for hiding this comment

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

Space missing after ,.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Updated

@ravipesala
Copy link
Contributor Author

Thank you for reviewing it. I have updated the code as per your comments. Please review it

1 similar comment
@ravipesala
Copy link
Contributor Author

Thank you for reviewing it. I have updated the code as per your comments. Please review it

@SparkQA
Copy link

SparkQA commented Oct 7, 2014

QA tests have finished for PR 2678 at commit 709684f.

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

@liancheng
Copy link
Contributor

LGTM. Thanks for working on this!

@ravipesala
Copy link
Contributor Author

@marmbrus Can you also verify this PR. Thank you.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@marmbrus
Copy link
Contributor

marmbrus commented Oct 9, 2014

Thanks! I've merged to master.

@asfgit asfgit closed this in ac30205 Oct 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants