[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

2017-09-04 Thread yaooqinn 
Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/19068#discussion_r136766581
  
--- Diff: 
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala
 ---
@@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
   System.exit(1)
 }
 
-val cliConf = new HiveConf(classOf[SessionState])
-// Override the location of the metastore since this is only used for 
local execution.
-HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
--- End diff --

@jiangxb1987 sorry for not @ you， please take a look again


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

2017-08-29 Thread yaooqinn 
Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/19068#discussion_r135940059
  
--- Diff: 
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala
 ---
@@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
   System.exit(1)
 }
 
-val cliConf = new HiveConf(classOf[SessionState])
-// Override the location of the metastore since this is only used for 
local execution.
-HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
--- End diff --

METASTORECONNECTURLKEY connects derby by default, we may set this key in 
hive-site.xml to talk with metastore. hiveclient will use the state init here, 
I don't think we should beinit hive conf as the old way.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

2017-08-29 Thread yaooqinn 
Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/19068#discussion_r135937401
  
--- Diff: 
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala
 ---
@@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
   System.exit(1)
 }
 
-val cliConf = new HiveConf(classOf[SessionState])
-// Override the location of the metastore since this is only used for 
local execution.
-HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
--- End diff --

Change this key to connect a dummy metastore instead of the real one？


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

2017-08-29 Thread jiangxb1987 
Github user jiangxb1987 commented on a diff in the pull request:

https://github.com/apache/spark/pull/19068#discussion_r135855867
  
--- Diff: 
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala
 ---
@@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
   System.exit(1)
 }
 
-val cliConf = new HiveConf(classOf[SessionState])
-// Override the location of the metastore since this is only used for 
local execution.
-HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
--- End diff --

I don't think you have answered the question here - for example, the 
original code will update the `HiveConf.ConfVars.METASTORECONNECTURLKEY` 
property here, but you don't touch that after the change. I'm not confident to 
make that behavior change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

2017-08-28 Thread yaooqinn 
Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/19068#discussion_r135676677
  
--- Diff: 
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala
 ---
@@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
   System.exit(1)
 }
 
-val cliConf = new HiveConf(classOf[SessionState])
-// Override the location of the metastore since this is only used for 
local execution.
-HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
--- End diff --

before #SPARK-21428, cliSessionState will be left behind; now we will reuse 
it. this func is used to organize execution hive configs, but now 
cliSessionState has to talk with metastore.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

2017-08-28 Thread jiangxb1987 
Github user jiangxb1987 commented on a diff in the pull request:

https://github.com/apache/spark/pull/19068#discussion_r135580204
  
--- Diff: 
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIDriver.scala
 ---
@@ -81,11 +81,7 @@ private[hive] object SparkSQLCLIDriver extends Logging {
   System.exit(1)
 }
 
-val cliConf = new HiveConf(classOf[SessionState])
-// Override the location of the metastore since this is only used for 
local execution.
-HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false).foreach {
--- End diff --

This function not only constructs the warehouse path, it also create many 
other configs, why is it safe to just ignore em? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

2017-08-28 Thread yaooqinn 
Github user yaooqinn commented on a diff in the pull request:

https://github.com/apache/spark/pull/19068#discussion_r135536738
  
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala 
---
@@ -395,7 +395,7 @@ private[spark] object HiveUtils extends Logging {
 propMap.put(confvar.varname, confvar.getDefaultExpr())
   }
 }
-propMap.put(WAREHOUSE_PATH.key, localMetastore.toURI.toString)
+if (useInMemoryDerby) propMap.put(WAREHOUSE_PATH.key, 
localMetastore.toURI.toString)
--- End diff --

@cloud-fan updated. i notice that hiveConf initialized by executionHive way 
does not load hive-site.xml, so i changed it to meta Hive way


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

2017-08-28 Thread cloud-fan 
Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/19068#discussion_r135494418
  
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala 
---
@@ -395,7 +395,7 @@ private[spark] object HiveUtils extends Logging {
 propMap.put(confvar.varname, confvar.getDefaultExpr())
   }
 }
-propMap.put(WAREHOUSE_PATH.key, localMetastore.toURI.toString)
+if (useInMemoryDerby) propMap.put(WAREHOUSE_PATH.key, 
localMetastore.toURI.toString)
--- End diff --

when is `useInMemoryDerby` false?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #19068: [SPARK-21428][SQL][FOLLOWUP] Reused state should ...

2017-08-28 Thread yaooqinn 
GitHub user yaooqinn opened a pull request:

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

[SPARK-21428][SQL][FOLLOWUP] Reused state should respect warehouse dir 
determined in SharedState

## What changes were proposed in this pull request?

While running spark-sql, we will reuse cliSessionState, but the warehouse 
is determined later in SharedState. HiveClient should respect this config 
changing in this case.

## How was this patch tested?
existing ut

cc @cloud-fan @jiangxb1987 

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

$ git pull https://github.com/yaooqinn/spark SPARK-21428-FOLLOWUP

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

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


commit b9e9a19fdc36a2b65e46406f78e5d50d211a3e45
Author: Kent Yao 
Date:   2017-08-28T09:25:17Z

SPARK-21428-FOLLOWUP: CliSessionState should respect warehouse dir 
determined in SharedState




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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