[GitHub] spark pull request #19801: [SPARK-22592][SQL] cleanup filter converting for ...

2017-11-23 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/19801


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #19801: [SPARK-22592][SQL] cleanup filter converting for ...

2017-11-23 Thread gatorsmile
Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/19801#discussion_r152880052
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ---
@@ -660,40 +626,68 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
   }
 
   def unapply(values: Set[Any]): Option[Seq[String]] = {
-values.toSeq.foldLeft(Option(Seq.empty[String])) {
-  case (Some(accum), value) if 
valueToLiteralString.isDefinedAt(value) =>
-Some(accum :+ valueToLiteralString(value))
-  case _ => None
+val extractables = values.toSeq.map(valueToLiteralString.lift)
+if (extractables.nonEmpty && extractables.forall(_.isDefined)) {
--- End diff --

The same here.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #19801: [SPARK-22592][SQL] cleanup filter converting for ...

2017-11-23 Thread gatorsmile
Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/19801#discussion_r152880027
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ---
@@ -643,9 +607,11 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
 
 object ExtractableLiterals {
   def unapply(exprs: Seq[Expression]): Option[Seq[String]] = {
-
exprs.map(ExtractableLiteral.unapply).foldLeft(Option(Seq.empty[String])) {
-  case (Some(accum), Some(value)) => Some(accum :+ value)
-  case _ => None
+val extractables = exprs.map(ExtractableLiteral.unapply)
+if (extractables.nonEmpty && extractables.forall(_.isDefined)) {
--- End diff --

-> `if (extractables.exists(_.isDefined))`


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #19801: [SPARK-22592][SQL] cleanup filter converting for ...

2017-11-23 Thread cloud-fan
GitHub user cloud-fan opened a pull request:

https://github.com/apache/spark/pull/19801

[SPARK-22592][SQL] cleanup filter converting for hive

## What changes were proposed in this pull request?

We have 2 different methods to convert filters for hive, regarding a 
config. This introduces duplicated and inconsistent code(e.g. one use helper 
objects for pattern match and one doesn't).

## How was this patch tested?

existing tests

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/cloud-fan/spark cleanup

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/19801.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 #19801


commit e6571a6a756391f64cf4f925626bf358777b969d
Author: Wenchen Fan 
Date:   2017-11-23T12:20:50Z

cleanup filter converting for hive




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org