You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by maropu <gi...@git.apache.org> on 2018/01/02 13:40:20 UTC
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
GitHub user maropu opened a pull request:
https://github.com/apache/spark/pull/20135
[SPARK-22937][SQL] SQL elt output binary for binary inputs
## What changes were proposed in this pull request?
This pr modified `elt` to output binary for binary inputs.
`elt` in the current master always output data as a string. But, in some databases (e.g., MySQL), if all inputs are binary, `elt` also outputs binary (Also, this might be a small surprise).
This pr is related to #19977.
## How was this patch tested?
Added tests in `SQLQueryTestSuite` and `TypeCoercionSuite`.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/maropu/spark SPARK-22937
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20135.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #20135
----
commit 733043853c5f60eba042af9a9cd05a6e11a199e9
Author: Takeshi Yamamuro <ya...@...>
Date: 2018-01-02T13:13:09Z
SQL elt output binary for binary inputs
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20135
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/20135#discussion_r159567565
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -58,7 +58,7 @@ case class Concat(children: Seq[Expression]) extends Expression {
} else {
val childTypes = children.map(_.dataType)
if (childTypes.exists(tpe => !Seq(StringType, BinaryType).contains(tpe))) {
- TypeCheckResult.TypeCheckFailure(
+ return TypeCheckResult.TypeCheckFailure(
s"input to function $prettyName should have StringType or BinaryType, but it's " +
--- End diff --
ok
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
Posted by gczsjdy <gi...@git.apache.org>.
Github user gczsjdy commented on a diff in the pull request:
https://github.com/apache/spark/pull/20135#discussion_r159235432
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
@@ -684,6 +685,34 @@ object TypeCoercion {
}
}
+ /**
+ * Coerces the types of [[Elt]] children to expected ones.
+ *
+ * If `spark.sql.function.eltOutputAsString` is false and all children types are binary,
+ * the expected types are binary. Otherwise, the expected ones are strings.
+ */
+ case class EltCoercion(conf: SQLConf) extends TypeCoercionRule {
+
+ override protected def coerceTypes(plan: LogicalPlan): LogicalPlan = plan transform { case p =>
+ p transformExpressionsUp {
+ // Skip nodes if unresolved or not enough children
+ case c @ Elt(children) if !c.childrenResolved || children.size < 2 => c
+ case c @ Elt(children) if conf.eltOutputAsString ||
+ !children.tail.map(_.dataType).forall(_ == BinaryType) =>
+ val index = children.head
+ val newIndex = ImplicitTypeCasts.implicitCast(index, IntegerType).getOrElse(index)
+ val newInputs = children.tail.map { e =>
+ ImplicitTypeCasts.implicitCast(e, StringType).getOrElse(e)
+ }
+ c.copy(children = newIndex +: newInputs)
+ case c @ Elt(children) =>
+ val index = children.head
+ val newIndex = ImplicitTypeCasts.implicitCast(index, IntegerType).getOrElse(index)
+ c.copy(children = newIndex +: children.tail)
--- End diff --
Is is better to merge the common parts of the last 2 cases? We can add one more nested pattern match, but I'm not sure this is a better way.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/20135#discussion_r159243492
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
@@ -684,6 +685,34 @@ object TypeCoercion {
}
}
+ /**
+ * Coerces the types of [[Elt]] children to expected ones.
+ *
+ * If `spark.sql.function.eltOutputAsString` is false and all children types are binary,
+ * the expected types are binary. Otherwise, the expected ones are strings.
+ */
+ case class EltCoercion(conf: SQLConf) extends TypeCoercionRule {
+
+ override protected def coerceTypes(plan: LogicalPlan): LogicalPlan = plan transform { case p =>
+ p transformExpressionsUp {
+ // Skip nodes if unresolved or not enough children
+ case c @ Elt(children) if !c.childrenResolved || children.size < 2 => c
+ case c @ Elt(children) if conf.eltOutputAsString ||
+ !children.tail.map(_.dataType).forall(_ == BinaryType) =>
+ val index = children.head
+ val newIndex = ImplicitTypeCasts.implicitCast(index, IntegerType).getOrElse(index)
+ val newInputs = children.tail.map { e =>
+ ImplicitTypeCasts.implicitCast(e, StringType).getOrElse(e)
+ }
+ c.copy(children = newIndex +: newInputs)
+ case c @ Elt(children) =>
+ val index = children.head
+ val newIndex = ImplicitTypeCasts.implicitCast(index, IntegerType).getOrElse(index)
+ c.copy(children = newIndex +: children.tail)
--- End diff --
I'll update soon.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20135
**[Test build #85661 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85661/testReport)** for PR 20135 at commit [`1ac512b`](https://github.com/apache/spark/commit/1ac512b26c2726904dbffdb447f538edfd686535).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
Posted by gczsjdy <gi...@git.apache.org>.
Github user gczsjdy commented on a diff in the pull request:
https://github.com/apache/spark/pull/20135#discussion_r159234455
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -271,33 +271,45 @@ case class ConcatWs(children: Seq[Expression])
}
}
+/**
+ * An expression that returns the `n`-th input in given inputs.
+ * If all inputs are binary, `elt` returns an output as binary. Otherwise, it returns as string.
+ * If any input is null, `elt` returns null.
+ */
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(n, str1, str2, ...) - Returns the `n`-th string, e.g., returns `str2` when `n` is 2.",
+ usage = "_FUNC_(n, input1, input2, ...) - Returns the `n`-th input, e.g., returns `input2` when `n` is 2.",
examples = """
Examples:
> SELECT _FUNC_(1, 'scala', 'java');
scala
""")
// scalastyle:on line.size.limit
-case class Elt(children: Seq[Expression])
- extends Expression with ImplicitCastInputTypes {
+case class Elt(children: Seq[Expression]) extends Expression {
private lazy val indexExpr = children.head
- private lazy val stringExprs = children.tail.toArray
+ private lazy val inputExprs = children.tail.toArray
/** This expression is always nullable because it returns null if index is out of range. */
override def nullable: Boolean = true
- override def dataType: DataType = StringType
-
- override def inputTypes: Seq[DataType] = IntegerType +: Seq.fill(children.size - 1)(StringType)
+ override def dataType: DataType = inputExprs.map(_.dataType).headOption.getOrElse(StringType)
--- End diff --
Should we return null of `BinaryType` when `eltOutputAsString` is `false` and there's only 1 parameter for `Elt`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20135
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85603/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20135
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20135
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20135
**[Test build #85597 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85597/testReport)** for PR 20135 at commit [`7330438`](https://github.com/apache/spark/commit/733043853c5f60eba042af9a9cd05a6e11a199e9).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` case class EltCoercion(conf: SQLConf) extends TypeCoercionRule `
* `case class Elt(children: Seq[Expression]) extends Expression `
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20135
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85661/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/20135
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20135
**[Test build #85603 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85603/testReport)** for PR 20135 at commit [`2aeb56d`](https://github.com/apache/spark/commit/2aeb56d97cd2d65fcccace4ad18439e9daf2805b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/20135#discussion_r159227959
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -58,7 +58,7 @@ case class Concat(children: Seq[Expression]) extends Expression {
} else {
val childTypes = children.map(_.dataType)
if (childTypes.exists(tpe => !Seq(StringType, BinaryType).contains(tpe))) {
- TypeCheckResult.TypeCheckFailure(
+ return TypeCheckResult.TypeCheckFailure(
s"input to function $prettyName should have StringType or BinaryType, but it's " +
--- End diff --
This is not related to this pr though, this might be a bug I left in #19977.
(Since branch-2.3 has already been cut out, is it better to split this from this pr and backport into branch-2.3? @gatorsmile).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/20135#discussion_r159243470
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -271,33 +271,45 @@ case class ConcatWs(children: Seq[Expression])
}
}
+/**
+ * An expression that returns the `n`-th input in given inputs.
+ * If all inputs are binary, `elt` returns an output as binary. Otherwise, it returns as string.
+ * If any input is null, `elt` returns null.
+ */
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(n, str1, str2, ...) - Returns the `n`-th string, e.g., returns `str2` when `n` is 2.",
+ usage = "_FUNC_(n, input1, input2, ...) - Returns the `n`-th input, e.g., returns `input2` when `n` is 2.",
examples = """
Examples:
> SELECT _FUNC_(1, 'scala', 'java');
scala
""")
// scalastyle:on line.size.limit
-case class Elt(children: Seq[Expression])
- extends Expression with ImplicitCastInputTypes {
+case class Elt(children: Seq[Expression]) extends Expression {
private lazy val indexExpr = children.head
- private lazy val stringExprs = children.tail.toArray
+ private lazy val inputExprs = children.tail.toArray
/** This expression is always nullable because it returns null if index is out of range. */
override def nullable: Boolean = true
- override def dataType: DataType = StringType
-
- override def inputTypes: Seq[DataType] = IntegerType +: Seq.fill(children.size - 1)(StringType)
+ override def dataType: DataType = inputExprs.map(_.dataType).headOption.getOrElse(StringType)
--- End diff --
sorry, but I miss your point. What's the type of the only 1 parameter?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20135
**[Test build #85661 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85661/testReport)** for PR 20135 at commit [`1ac512b`](https://github.com/apache/spark/commit/1ac512b26c2726904dbffdb447f538edfd686535).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20135
**[Test build #85597 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85597/testReport)** for PR 20135 at commit [`7330438`](https://github.com/apache/spark/commit/733043853c5f60eba042af9a9cd05a6e11a199e9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20135
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85597/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20135
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
Posted by gczsjdy <gi...@git.apache.org>.
Github user gczsjdy commented on a diff in the pull request:
https://github.com/apache/spark/pull/20135#discussion_r159353652
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -271,33 +271,45 @@ case class ConcatWs(children: Seq[Expression])
}
}
+/**
+ * An expression that returns the `n`-th input in given inputs.
+ * If all inputs are binary, `elt` returns an output as binary. Otherwise, it returns as string.
+ * If any input is null, `elt` returns null.
+ */
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(n, str1, str2, ...) - Returns the `n`-th string, e.g., returns `str2` when `n` is 2.",
+ usage = "_FUNC_(n, input1, input2, ...) - Returns the `n`-th input, e.g., returns `input2` when `n` is 2.",
examples = """
Examples:
> SELECT _FUNC_(1, 'scala', 'java');
scala
""")
// scalastyle:on line.size.limit
-case class Elt(children: Seq[Expression])
- extends Expression with ImplicitCastInputTypes {
+case class Elt(children: Seq[Expression]) extends Expression {
private lazy val indexExpr = children.head
- private lazy val stringExprs = children.tail.toArray
+ private lazy val inputExprs = children.tail.toArray
/** This expression is always nullable because it returns null if index is out of range. */
override def nullable: Boolean = true
- override def dataType: DataType = StringType
-
- override def inputTypes: Seq[DataType] = IntegerType +: Seq.fill(children.size - 1)(StringType)
+ override def dataType: DataType = inputExprs.map(_.dataType).headOption.getOrElse(StringType)
--- End diff --
I meant for expression `elt(1)` with `eltOutputAsString` as `false`, since the index is out of range. Is it better to make the result `null` of `BinaryType`? Now I think your solution makes more sense.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20135
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20135#discussion_r160012524
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -271,33 +271,45 @@ case class ConcatWs(children: Seq[Expression])
}
}
+/**
+ * An expression that returns the `n`-th input in given inputs.
+ * If all inputs are binary, `elt` returns an output as binary. Otherwise, it returns as string.
+ * If any input is null, `elt` returns null.
+ */
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(n, str1, str2, ...) - Returns the `n`-th string, e.g., returns `str2` when `n` is 2.",
+ usage = "_FUNC_(n, input1, input2, ...) - Returns the `n`-th input, e.g., returns `input2` when `n` is 2.",
examples = """
Examples:
> SELECT _FUNC_(1, 'scala', 'java');
scala
""")
// scalastyle:on line.size.limit
-case class Elt(children: Seq[Expression])
- extends Expression with ImplicitCastInputTypes {
+case class Elt(children: Seq[Expression]) extends Expression {
private lazy val indexExpr = children.head
- private lazy val stringExprs = children.tail.toArray
+ private lazy val inputExprs = children.tail.toArray
/** This expression is always nullable because it returns null if index is out of range. */
override def nullable: Boolean = true
- override def dataType: DataType = StringType
-
- override def inputTypes: Seq[DataType] = IntegerType +: Seq.fill(children.size - 1)(StringType)
+ override def dataType: DataType = inputExprs.map(_.dataType).headOption.getOrElse(StringType)
--- End diff --
We issue an exception when the input argument is 1.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20135#discussion_r159566662
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala ---
@@ -58,7 +58,7 @@ case class Concat(children: Seq[Expression]) extends Expression {
} else {
val childTypes = children.map(_.dataType)
if (childTypes.exists(tpe => !Seq(StringType, BinaryType).contains(tpe))) {
- TypeCheckResult.TypeCheckFailure(
+ return TypeCheckResult.TypeCheckFailure(
s"input to function $prettyName should have StringType or BinaryType, but it's " +
--- End diff --
Yes. Also create a test case
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20135
Thanks! Merged to master/2.3
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20135
**[Test build #85603 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85603/testReport)** for PR 20135 at commit [`2aeb56d`](https://github.com/apache/spark/commit/2aeb56d97cd2d65fcccace4ad18439e9daf2805b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/20135
I'll update the migration guide, too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20135
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85673/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20135
**[Test build #85673 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85673/testReport)** for PR 20135 at commit [`1ac512b`](https://github.com/apache/spark/commit/1ac512b26c2726904dbffdb447f538edfd686535).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20135: [SPARK-22937][SQL] SQL elt output binary for bina...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20135
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20135
**[Test build #85673 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85673/testReport)** for PR 20135 at commit [`1ac512b`](https://github.com/apache/spark/commit/1ac512b26c2726904dbffdb447f538edfd686535).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20135: [SPARK-22937][SQL] SQL elt output binary for binary inpu...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/20135
@gatorsmile
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org