[GitHub] spark pull request #20864: [SPARK-23745][SQL]Remove the directories of the �...

2018-09-18 Thread zuotingbing
GitHub user zuotingbing reopened a pull request:

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

[SPARK-23745][SQL]Remove the directories of the 
“hive.downloaded.resources.dir” when HiveThriftServer2 stopped

## What changes were proposed in this pull request?


![2018-03-20_164832](https://user-images.githubusercontent.com/24823338/37647937-871ff346-2c68-11e8-9ca4-1b3ebb291530.png)

when start the HiveThriftServer2(`./sbin/start-thriftserver.sh`), we create 
some  directories for hive.downloaded.resources.dir, but when stop the 
HiveThriftServer2(`./sbin/stop-thriftserver.sh`), we do not remove these 
directories.The directories could accumulate a lot.

## How was this patch tested?

manual tests



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

$ git pull https://github.com/zuotingbing/spark SPARK-23745

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

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


commit b177bf441a04c0a700e33d7e40c1b2408c3c0c3b
Author: zuotingbing 
Date:   2018-03-20T09:57:28Z

[SPARK-23745][SQL]Remove the directories of the 
“hive.downloaded.resources.dir” when HiveThriftServer2 stopped




---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread dilipbiswal
Github user dilipbiswal commented on the issue:

https://github.com/apache/spark/pull/22448
  
@ueshin 
> I was thinking whether or not we should handle the case like 
widenTest(DecimalType(3, 2), DecimalType(5, 1), Some(DecimalType(...))), which 
is currently None?

Thank you !! I think we should. Lets get the integral, decimal thing right 
first and then take on the (decimal, decimal). We never handled (decimal, 
decimal) in findTightestCommonType .. hopefully  there are no repercussions :-).


---

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



[GitHub] spark pull request #22448: [SPARK-25417][SQL] Improve findTightestCommonType...

2018-09-18 Thread dilipbiswal
Github user dilipbiswal commented on a diff in the pull request:

https://github.com/apache/spark/pull/22448#discussion_r218346135
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ---
@@ -31,6 +32,7 @@ import org.apache.spark.sql.internal.SQLConf
 import org.apache.spark.sql.types._
 
 
+
--- End diff --

Will do.


---

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



[GitHub] spark pull request #22448: [SPARK-25417][SQL] Improve findTightestCommonType...

2018-09-18 Thread ueshin
Github user ueshin commented on a diff in the pull request:

https://github.com/apache/spark/pull/22448#discussion_r218337303
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ---
@@ -31,6 +32,7 @@ import org.apache.spark.sql.internal.SQLConf
 import org.apache.spark.sql.types._
 
 
+
--- End diff --

nit: revert this?


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread dilipbiswal
Github user dilipbiswal commented on the issue:

https://github.com/apache/spark/pull/22448
  
@cloud-fan 
> I think it's a bug fix instead of an improvement. findTightestCommonType 
is used for binary operators and it should be easy to write some end-to-end 
test cases to verify the bug.

Please correct me on this one. I think for normal queries like `select * 
from foo where c1 = 1.23` (where c1 is a integral type) casts are setup 
correctly through TypeCoercion::DecimalPrecision.  Is that the kind of 
end-to-end test you had in mind ?


---

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



[GitHub] spark issue #22165: [SPARK-25017][Core] Add test suite for BarrierCoordinato...

2018-09-18 Thread SparkQA
Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/22165
  
**[Test build #96173 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96173/testReport)**
 for PR 22165 at commit 
[`ec8466a`](https://github.com/apache/spark/commit/ec8466a8272b93e12fa651e65db65f12148576bb).


---

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



[GitHub] spark issue #22165: [SPARK-25017][Core] Add test suite for BarrierCoordinato...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22165
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 

https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3186/
Test PASSed.


---

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



[GitHub] spark issue #22165: [SPARK-25017][Core] Add test suite for BarrierCoordinato...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22165
  
Merged build finished. Test PASSed.


---

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



[GitHub] spark pull request #22165: [SPARK-25017][Core] Add test suite for BarrierCoo...

2018-09-18 Thread xuanyuanking
Github user xuanyuanking commented on a diff in the pull request:

https://github.com/apache/spark/pull/22165#discussion_r218328998
  
--- Diff: core/src/main/scala/org/apache/spark/BarrierCoordinator.scala ---
@@ -84,20 +88,20 @@ private[spark] class BarrierCoordinator(
 
   /**
* Provide the current state of a barrier() call. A state is created 
when a new stage attempt
-   * sends out a barrier() call, and recycled on stage completed.
+   * sends out a barrier() call, and recycled on stage completed. Visible 
for testing.
*
* @param barrierId Identifier of the barrier stage that make a 
barrier() call.
* @param numTasks Number of tasks of the barrier stage, all barrier() 
calls from the stage shall
* collect `numTasks` requests to succeed.
*/
-  private class ContextBarrierState(
+  private[spark] class ContextBarrierState(
   val barrierId: ContextBarrierId,
   val numTasks: Int) {
 
 // There may be multiple barrier() calls from a barrier stage attempt, 
`barrierEpoch` is used
 // to identify each barrier() call. It shall get increased when a 
barrier() call succeeds, or
 // reset when a barrier() call fails due to timeout.
-private var barrierEpoch: Int = 0
+private[spark] var barrierEpoch: Int = 0
--- End diff --

Make sense, done in ec8466a。


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread dilipbiswal
Github user dilipbiswal commented on the issue:

https://github.com/apache/spark/pull/22448
  
@ueshin Can you please explain a bit ? 


---

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



[GitHub] spark pull request #19773: [SPARK-22546][SQL] Supporting for changing column...

2018-09-18 Thread xuanyuanking
Github user xuanyuanking commented on a diff in the pull request:

https://github.com/apache/spark/pull/19773#discussion_r218324420
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -350,16 +366,11 @@ case class AlterTableChangeColumnCommand(
 s"${schema.fieldNames.mkString("[`", "`, `", "`]")}"))
   }
 
-  // Add the comment to a column, if comment is empty, return the original 
column.
-  private def addComment(column: StructField, comment: Option[String]): 
StructField = {
-comment.map(column.withComment(_)).getOrElse(column)
-  }
-
--- End diff --

Thanks for your advise!
I look into this in these days. With currently implement, all behavior 
comply with Hive(Support type change/Work well in non binary format/Exception 
in binary format like orc and parquet). Is it ok to add a config for constraint 
this?

The work of adding logic to cast input data into changed type in catalog 
may need modifying 4 parts logic including vectorized reader and row reader in 
parquet and orc. If we don't agree the currently behavior, I'll keep following 
these.

Item | Behavior
 | -
Parquet Row Reader | ClassCastException in SpecificInternalRow.set${Type}
Parquet Vectorized Reader | SchemaColumnConvertNotSupportedException in 
VectorizedColumnReader.read${Type}Batch
Orc Row Reader | ClassCastException in OrcDeserializer.newWriter
Orc Vectorized Reader | NullPointerException in OrcColumnVector get value 
by type method


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread ueshin
Github user ueshin commented on the issue:

https://github.com/apache/spark/pull/22448
  
I'm just wondering we should care about the case like `decimal(3, 2)` vs. 
`decimal(5, 1)`?


---

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



[GitHub] spark issue #22402: [SPARK-25414][SS][TEST] make it clear that the numRows m...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22402
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 

https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3185/
Test PASSed.


---

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



[GitHub] spark issue #22402: [SPARK-25414][SS][TEST] make it clear that the numRows m...

2018-09-18 Thread SparkQA
Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/22402
  
**[Test build #96172 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96172/testReport)**
 for PR 22402 at commit 
[`0c661a0`](https://github.com/apache/spark/commit/0c661a08e74fea90b025ad21fb9da6113ef70d4c).


---

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



[GitHub] spark issue #22402: [SPARK-25414][SS][TEST] make it clear that the numRows m...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22402
  
Merged build finished. Test PASSed.


---

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



[GitHub] spark issue #22411: [SPARK-25421][SQL] Abstract an output path field in trai...

2018-09-18 Thread cloud-fan
Github user cloud-fan commented on the issue:

https://github.com/apache/spark/pull/22411
  
We can't merge new features to maintenance branches(2.4 as well), so we 
don't need to rush here, as this feature can only be available in the next 
release.


---

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



[GitHub] spark issue #22402: [SPARK-25414][SS][TEST] make it clear that the numRows m...

2018-09-18 Thread cloud-fan
Github user cloud-fan commented on the issue:

https://github.com/apache/spark/pull/22402
  
retest this please


---

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



[GitHub] spark pull request #22448: [SPARK-25417][SQL] Improve findTightestCommonType...

2018-09-18 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/22448#discussion_r218322027
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ---
@@ -106,6 +108,22 @@ object TypeCoercion {
 case (t1, t2) => findTypeForComplex(t1, t2, findTightestCommonType)
   }
 
+  /**
+   * Finds a wider decimal type between the two supplied decimal types 
without
+   * any loss of precision.
+   */
+  def findWiderDecimalType(d1: DecimalType, d2: DecimalType): 
Option[DecimalType] = {
--- End diff --

ok. Then can we add some more tests with negative scale?


---

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



[GitHub] spark issue #22443: [SPARK-25339][TEST] Refactor FilterPushdownBenchmark

2018-09-18 Thread cloud-fan
Github user cloud-fan commented on the issue:

https://github.com/apache/spark/pull/22443
  
@dongjoon-hyun we are trying to avoid the overhead of scalatest, not sbt. 
So this LGTM


---

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



[GitHub] spark pull request #22448: [SPARK-25417][SQL] Improve findTightestCommonType...

2018-09-18 Thread dilipbiswal
Github user dilipbiswal commented on a diff in the pull request:

https://github.com/apache/spark/pull/22448#discussion_r218321366
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ---
@@ -89,10 +91,10 @@ object TypeCoercion {
 case (NullType, t1) => Some(t1)
 case (t1, NullType) => Some(t1)
 
-case (t1: IntegralType, t2: DecimalType) if t2.isWiderThan(t1) =>
-  Some(t2)
-case (t1: DecimalType, t2: IntegralType) if t1.isWiderThan(t2) =>
-  Some(t1)
+case (t1: IntegralType, t2: DecimalType) =>
--- End diff --

@cloud-fan ok... thanks !!


---

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



[GitHub] spark pull request #22448: [SPARK-25417][SQL] Improve findTightestCommonType...

2018-09-18 Thread dilipbiswal
Github user dilipbiswal commented on a diff in the pull request:

https://github.com/apache/spark/pull/22448#discussion_r218321144
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ---
@@ -106,6 +108,22 @@ object TypeCoercion {
 case (t1, t2) => findTypeForComplex(t1, t2, findTightestCommonType)
   }
 
+  /**
+   * Finds a wider decimal type between the two supplied decimal types 
without
+   * any loss of precision.
+   */
+  def findWiderDecimalType(d1: DecimalType, d2: DecimalType): 
Option[DecimalType] = {
--- End diff --

@cloud-fan Actually the bounded version is in 
`DecimalPrecision::widerDecimalType`. Thats the function i looked at as 
reference.


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread cloud-fan
Github user cloud-fan commented on the issue:

https://github.com/apache/spark/pull/22448
  
I think it's a bug fix instead of an improvement. `findTightestCommonType` 
is used for binary operators and it should be easy to write some end-to-end 
test cases to verify the bug.


---

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



[GitHub] spark issue #22441: [SPARK-25445][BUILD] the release script should be able t...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22441
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 

https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3184/
Test PASSed.


---

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



[GitHub] spark pull request #22448: [SPARK-25417][SQL] Improve findTightestCommonType...

2018-09-18 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/22448#discussion_r218320357
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ---
@@ -106,6 +108,22 @@ object TypeCoercion {
 case (t1, t2) => findTypeForComplex(t1, t2, findTightestCommonType)
   }
 
+  /**
+   * Finds a wider decimal type between the two supplied decimal types 
without
+   * any loss of precision.
+   */
+  def findWiderDecimalType(d1: DecimalType, d2: DecimalType): 
Option[DecimalType] = {
--- End diff --

Is there a reference for this implementation? I'm worried about corner 
cases like negative scale.


---

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



[GitHub] spark issue #22441: [SPARK-25445][BUILD] the release script should be able t...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22441
  
Merged build finished. Test PASSed.


---

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



[GitHub] spark issue #22441: [SPARK-25445][BUILD] the release script should be able t...

2018-09-18 Thread SparkQA
Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/22441
  
**[Test build #96171 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96171/testReport)**
 for PR 22441 at commit 
[`110913a`](https://github.com/apache/spark/commit/110913a6d4a10087ee31247d5357a0432ffbd8b7).


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22448
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 

https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3183/
Test PASSed.


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread SparkQA
Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/22448
  
**[Test build #96170 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96170/testReport)**
 for PR 22448 at commit 
[`8946034`](https://github.com/apache/spark/commit/8946034e1b25fcf2f4595d27943c9df87c12e096).


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22448
  
Merged build finished. Test PASSed.


---

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



[GitHub] spark pull request #22448: [SPARK-25417][SQL] Improve findTightestCommonType...

2018-09-18 Thread cloud-fan
Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/22448#discussion_r218319943
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ---
@@ -89,10 +91,10 @@ object TypeCoercion {
 case (NullType, t1) => Some(t1)
 case (t1, NullType) => Some(t1)
 
-case (t1: IntegralType, t2: DecimalType) if t2.isWiderThan(t1) =>
-  Some(t2)
-case (t1: DecimalType, t2: IntegralType) if t1.isWiderThan(t2) =>
-  Some(t1)
+case (t1: IntegralType, t2: DecimalType) =>
--- End diff --

I don't think so. We don't need to handle integral and decimal again after 
it.


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread dilipbiswal
Github user dilipbiswal commented on the issue:

https://github.com/apache/spark/pull/22448
  
retest this please.


---

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



[GitHub] spark issue #22441: [SPARK-25445][BUILD] the release script should be able t...

2018-09-18 Thread cloud-fan
Github user cloud-fan commented on the issue:

https://github.com/apache/spark/pull/22441
  
retest this please


---

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



[GitHub] spark issue #22441: [SPARK-25445][BUILD] the release script should be able t...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22441
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96168/
Test FAILed.


---

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



[GitHub] spark issue #22441: [SPARK-25445][BUILD] the release script should be able t...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22441
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96167/
Test FAILed.


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22448
  
Merged build finished. Test FAILed.


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread SparkQA
Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/22448
  
**[Test build #96169 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96169/testReport)**
 for PR 22448 at commit 
[`8946034`](https://github.com/apache/spark/commit/8946034e1b25fcf2f4595d27943c9df87c12e096).
 * 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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22448
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96169/
Test FAILed.


---

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



[GitHub] spark issue #22441: [SPARK-25445][BUILD] the release script should be able t...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22441
  
Merged build finished. Test FAILed.


---

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



[GitHub] spark issue #22441: [SPARK-25445][BUILD] the release script should be able t...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22441
  
Merged build finished. Test FAILed.


---

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



[GitHub] spark issue #22441: [SPARK-25445][BUILD] the release script should be able t...

2018-09-18 Thread SparkQA
Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/22441
  
**[Test build #96167 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96167/testReport)**
 for PR 22441 at commit 
[`b5237ec`](https://github.com/apache/spark/commit/b5237ecfed1831305cec482ea982c0050e0e3970).
 * 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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #22441: [SPARK-25445][BUILD] the release script should be able t...

2018-09-18 Thread SparkQA
Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/22441
  
**[Test build #96168 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96168/testReport)**
 for PR 22441 at commit 
[`110913a`](https://github.com/apache/spark/commit/110913a6d4a10087ee31247d5357a0432ffbd8b7).
 * 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-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #21217: [SPARK-24151][SQL] Fix CURRENT_DATE, CURRENT_TIME...

2018-09-18 Thread asfgit
Github user asfgit closed the pull request at:

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


---

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



[GitHub] spark pull request #22448: [SPARK-25417][SQL] Improve findTightestCommonType...

2018-09-18 Thread dilipbiswal
Github user dilipbiswal commented on a diff in the pull request:

https://github.com/apache/spark/pull/22448#discussion_r218310175
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ---
@@ -89,10 +91,10 @@ object TypeCoercion {
 case (NullType, t1) => Some(t1)
 case (t1, NullType) => Some(t1)
 
-case (t1: IntegralType, t2: DecimalType) if t2.isWiderThan(t1) =>
-  Some(t2)
-case (t1: DecimalType, t2: IntegralType) if t1.isWiderThan(t2) =>
-  Some(t1)
+case (t1: IntegralType, t2: DecimalType) =>
--- End diff --

@cloud-fan Would we need a guard here to be safe ? like 
``` case (t1: IntegralType, t2: DecimalType) if 
findWiderDecimalType(DecimalType.forType(t1), t2)).isdefined => ```


---

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



[GitHub] spark pull request #22440: [SPARK-24151][SQL] Case insensitive resolution of...

2018-09-18 Thread asfgit
Github user asfgit closed the pull request at:

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


---

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



[GitHub] spark pull request #22295: [SPARK-25255][PYTHON]Add getActiveSession to Spar...

2018-09-18 Thread gatorsmile
Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/22295#discussion_r218309924
  
--- Diff: python/pyspark/sql/session.py ---
@@ -252,6 +253,22 @@ def newSession(self):
 """
 return self.__class__(self._sc, self._jsparkSession.newSession())
 
+@since(3.0)
+def getActiveSession(self):
--- End diff --

cc @ueshin 


---

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



[GitHub] spark issue #22440: [SPARK-24151][SQL] Case insensitive resolution of CURREN...

2018-09-18 Thread gatorsmile
Github user gatorsmile commented on the issue:

https://github.com/apache/spark/pull/22440
  
Thanks! Merged to master/2.4


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22448
  
Merged build finished. Test PASSed.


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/22448
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 

https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3182/
Test PASSed.


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread SparkQA
Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/22448
  
**[Test build #96169 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96169/testReport)**
 for PR 22448 at commit 
[`8946034`](https://github.com/apache/spark/commit/8946034e1b25fcf2f4595d27943c9df87c12e096).


---

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



[GitHub] spark issue #22448: [SPARK-25417][SQL] Improve findTightestCommonType to coe...

2018-09-18 Thread dilipbiswal
Github user dilipbiswal commented on the issue:

https://github.com/apache/spark/pull/22448
  
cc @cloud-fan 


---

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



[GitHub] spark pull request #22448: [SPARK-25417][SQL] Improve findTightestCommonType...

2018-09-18 Thread dilipbiswal
GitHub user dilipbiswal opened a pull request:

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

[SPARK-25417][SQL] Improve findTightestCommonType to coerce Integral and 
decimal types

## What changes were proposed in this pull request?
Currently `findTightestCommonType` is not able to coerce between integral 
and decimal types properly. For example, while trying to find a common type 
between (int, decimal) , it is able to find a common type only when the number 
of digits to the left of decimal point of the decimal number is >= 10. This PR 
enhances the logic to to correctly find a wider decimal type between the 
integral and decimal types.

Here are some examples of the result of `findTightestCommonType`
```
int, decimal(3, 2) => decimal(12, 2)
int, decimal(4, 3) => decimal(13, 3)
int, decimal(11, 3) => decimal(14, 3)
int, decimal(38, 18) => decimal(38, 18)
int, decimal(38, 29) => None 
```

## How was this patch tested?
Added tests to TypeCoercionSuite.

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

$ git pull https://github.com/dilipbiswal/spark find_tightest

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

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


commit ea0707bea4744a5217baeb82219bed414458afeb
Author: Dilip Biswal 
Date:   2018-09-18T05:28:54Z

Improve findTightestCommonType to coerce Integral and decimal types

commit 8946034e1b25fcf2f4595d27943c9df87c12e096
Author: Dilip Biswal 
Date:   2018-09-18T05:52:14Z

remove space




---

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



[GitHub] spark issue #22420: [SPARK-25429][SQL]Use Set improve SparkListenerBus effic...

2018-09-18 Thread gengliangwang
Github user gengliangwang commented on the issue:

https://github.com/apache/spark/pull/22420
  
The change LGTM.
Please change the PR against `master` branch as per @dongjoon-hyun 's 
suggestion.
It would be nice to have a better PR description :)


---

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



[GitHub] spark pull request #22443: [SPARK-25339][TEST] Refactor FilterPushdownBenchm...

2018-09-18 Thread wangyum
Github user wangyum commented on a diff in the pull request:

https://github.com/apache/spark/pull/22443#discussion_r218308258
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/FilterPushdownBenchmark.scala
 ---
@@ -17,29 +17,28 @@
 
 package org.apache.spark.sql.execution.benchmark
 
-import java.io.{File, FileOutputStream, OutputStream}
+import java.io.File
 
 import scala.util.{Random, Try}
 
-import org.scalatest.{BeforeAndAfterEachTestData, Suite, TestData}
-
 import org.apache.spark.SparkConf
-import org.apache.spark.SparkFunSuite
 import org.apache.spark.sql.{DataFrame, SparkSession}
 import org.apache.spark.sql.functions.monotonically_increasing_id
 import org.apache.spark.sql.internal.SQLConf
 import org.apache.spark.sql.internal.SQLConf.ParquetOutputTimestampType
 import org.apache.spark.sql.types.{ByteType, Decimal, DecimalType, 
TimestampType}
-import org.apache.spark.util.{Benchmark, Utils}
+import org.apache.spark.util.{Benchmark, BenchmarkBase => 
FileBenchmarkBase, Utils}
 
 /**
  * Benchmark to measure read performance with Filter pushdown.
- * To run this:
- *  build/sbt "sql/test-only *FilterPushdownBenchmark"
- *
- * Results will be written to 
"benchmarks/FilterPushdownBenchmark-results.txt".
+ * To run this benchmark:
+ *  1. without sbt: bin/spark-submit --class  
+ *  2. build/sbt "sql/test:runMain "
+ *  3. generate result: SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt 
"sql/test:runMain "
--- End diff --

Yes, It can print the output to console if `SPARK_GENERATE_BENCHMARK_FILES` 
not set.

https://github.com/apache/spark/blob/4e8ac6edd5808ca8245b39d804c6d4f5ea9d0d36/core/src/main/scala/org/apache/spark/util/Benchmark.scala#L59-L63


---

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



[GitHub] spark pull request #22429: [SPARK-25440][SQL] Dumping query execution info t...

2018-09-18 Thread gatorsmile
Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/22429#discussion_r218307450
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
 ---
@@ -16,11 +16,33 @@
  */
 package org.apache.spark.sql.execution
 
+import scala.io.Source
+
 import org.apache.spark.sql.AnalysisException
 import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, 
OneRowRelation}
 import org.apache.spark.sql.test.SharedSQLContext
 
 class QueryExecutionSuite extends SharedSQLContext {
+  test("dumping query execution info to a file") {
+withTempDir { dir =>
+  val path = dir.getCanonicalPath + "/plans.txt"
--- End diff --

Also add a few cases. For example, the path already exists? the path is not 
legal? and so on.


---

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



<    1   2   3   4   5   6