[SPARK-4576][SQL] Add concatenation operator

[sarutak]

The standard SQL defines || as a concatenation operator.
The operator makes concatenated string from 2 operands.

[SparkQA]

Test build #23794 has finished for PR 3433 at commit f4bbec3.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #23795 has finished for PR 3433 at commit 0e5062c.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #23797 has finished for PR 3433 at commit 5437d49.

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

[SparkQA]

Test build #23960 has finished for PR 3433 at commit 66df576.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class Concat(left: Expression, right: Expression) extends BinaryExpression

[marmbrus]

Is this the right place to add this given the precedence of the operator?

[sarutak]

Actually, I found the precedence of the operator is different depends on the implementation of RDBMS. The precedence of this operator in this PR is similar to PostgreSQL's implementation.

[liancheng]

NIT: add a space after {.

[sarutak]

@liancheng Thanks for your comment. I've done.

[SparkQA]

Test build #24278 has finished for PR 3433 at commit 8b162ec.

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

[marmbrus]

This is automatically defined by BinaryExpression.

[marmbrus]

One minor style comment. Also, does Hive support this? If so we should add it to HiveQl.scala as well. Otherwise this LGTM

[sarutak]

Thanks @marmbrus .
In Hive, || is assigned as Logical OR operator and Hive has concat function instead of || so I don't think we need change for HiveQL.scala.

[SparkQA]

Test build #24565 has finished for PR 3433 at commit 372d665.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class Concat(left: Expression, right: Expression) extends BinaryExpression
  • class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener

[SparkQA]

Test build #24749 has finished for PR 3433 at commit 85469b4.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class Concat(left: Expression, right: Expression) extends BinaryExpression

[SparkQA]

Test build #24836 has finished for PR 3433 at commit 9b94d48.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class Concat(left: Expression, right: Expression) extends BinaryExpression

[SparkQA]

Test build #24896 has finished for PR 3433 at commit 6b47555.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • case class Concat(left: Expression, right: Expression) extends BinaryExpression

[marmbrus]

Hmm, okay. It might be a little confusing to users if in one version it does concatenation and in the other it is an OR. My other concern here is that the new concatenation operator is implicitly doing casting internally instead of actually casting its arguments to String. While this might be the same now, I'll break if we ever have special string casts. Perhaps this feature should be blocked on: SPARK-4867?

[sarutak]

Ah, exactly it can confusing users. So how about adding CONCAT function? If we do that, SPARK-4867 can blocks this PR.

[marmbrus]

Yeah, agreed. Lets block this on SPARK-4867, but once that is fixed I'd love to have a native concat function.

[marmbrus]

For now though I propose we close this issue.

[sarutak]

O.K, closing this PR.