[GitHub] spark pull request #20944: [SPARK-23831][SQL] Add org.apache.derby to Isolat...

2018-07-13 Thread asfgit 
Github user asfgit closed the pull request at:

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


---

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

[GitHub] spark pull request #20944: [SPARK-23831][SQL] Add org.apache.derby to Isolat...

2018-07-13 Thread holdenk 
Github user holdenk commented on a diff in the pull request:

https://github.com/apache/spark/pull/20944#discussion_r202426713
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/IsolatedClientLoader.scala
 ---
@@ -182,6 +182,7 @@ private[hive] class IsolatedClientLoader(
 name.startsWith("org.slf4j") ||
 name.startsWith("org.apache.log4j") || // log4j1.x
 name.startsWith("org.apache.logging.log4j") || // log4j2
+name.startsWith("org.apache.derby.") ||
--- End diff --

So to clarify, this adds derby to the "shared" classes (we might want to 
mention this above on L139) and presumably this fix is for cases where we have 
an existing derby version on the class-path which may differ and if that 
happens we end up with strangeness on the metastore? cc @gatorsmile ?

Like I said, I've run into this issue so I'd like to see a fix :)


---

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

[GitHub] spark pull request #20944: [SPARK-23831][SQL] Add org.apache.derby to Isolat...

2018-04-07 Thread felixcheung 
Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/20944#discussion_r179937446
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/IsolatedClientLoader.scala
 ---
@@ -188,6 +188,9 @@ private[hive] class IsolatedClientLoader(
 (name.startsWith("com.google") && 
!name.startsWith("com.google.cloud")) ||
 name.startsWith("java.lang.") ||
 name.startsWith("java.net") ||
+name.startsWith("com.sun.") ||
+name.startsWith("sun.reflect.") ||
+name.startsWith("org.apache.derby.") ||
--- End diff --

I'd also move this line to other org.apache* above


---

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

[GitHub] spark pull request #20944: [SPARK-23831][SQL] Add org.apache.derby to Isolat...

2018-04-07 Thread gatorsmile 
Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/20944#discussion_r179935823
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/IsolatedClientLoader.scala
 ---
@@ -188,6 +188,9 @@ private[hive] class IsolatedClientLoader(
 (name.startsWith("com.google") && 
!name.startsWith("com.google.cloud")) ||
 name.startsWith("java.lang.") ||
 name.startsWith("java.net") ||
+name.startsWith("com.sun.") ||
+name.startsWith("sun.reflect.") ||
--- End diff --

Do not add them unless we have to do it.


---

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

[GitHub] spark pull request #20944: [SPARK-23831][SQL] Add org.apache.derby to Isolat...

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

https://github.com/apache/spark/pull/20944#discussion_r179158019
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/IsolatedClientLoader.scala
 ---
@@ -188,6 +188,9 @@ private[hive] class IsolatedClientLoader(
 (name.startsWith("com.google") && 
!name.startsWith("com.google.cloud")) ||
 name.startsWith("java.lang.") ||
 name.startsWith("java.net") ||
+name.startsWith("com.sun.") ||
+name.startsWith("sun.reflect.") ||
--- End diff --

Yes, it doesn't matter if add these two lines, but I think it's best to 
add. What do you think?


---

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

[GitHub] spark pull request #20944: [SPARK-23831][SQL] Add org.apache.derby to Isolat...

2018-04-02 Thread felixcheung 
Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/20944#discussion_r178720885
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/IsolatedClientLoader.scala
 ---
@@ -188,6 +188,9 @@ private[hive] class IsolatedClientLoader(
 (name.startsWith("com.google") && 
!name.startsWith("com.google.cloud")) ||
 name.startsWith("java.lang.") ||
 name.startsWith("java.net") ||
+name.startsWith("com.sun.") ||
+name.startsWith("sun.reflect.") ||
--- End diff --

these two lines above are really broad, is this intentional?


---

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

[GitHub] spark pull request #20944: [SPARK-23831][SQL] Add org.apache.derby to Isolat...

2018-03-30 Thread wangyum 
GitHub user wangyum opened a pull request:

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

[SPARK-23831][SQL] Add org.apache.derby to IsolatedClientLoader

## What changes were proposed in this pull request?

Add `org.apache.derby` to `IsolatedClientLoader`, otherwise it may throw an 
exception:
```
[info] Cause: java.sql.SQLException: Failed to start database 
'metastore_db' with class loader 
org.apache.spark.sql.hive.client.IsolatedClientLoader$$anon$1@2439ab23, see the 
next exception for details.
[info] at 
org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(Unknown Source)
[info] at 
org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(Unknown Source)
[info] at org.apache.derby.impl.jdbc.Util.seeNextException(Unknown Source)
[info] at org.apache.derby.impl.jdbc.EmbedConnection.bootDatabase(Unknown 
Source)
[info] at org.apache.derby.impl.jdbc.EmbedConnection.(Unknown Source)
[info] at org.apache.derby.jdbc.InternalDriver$1.run(Unknown Source)
```

How to reproduce:
```bash

sed 's/HiveExternalCatalogSuite/HiveExternalCatalog2Suite/g' 
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogSuite.scala
 > 
sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalog2Suite.scala

build/sbt -Phive "hive/test-only *.HiveExternalCatalogSuite 
*.HiveExternalCatalog2Suite"
```

## How was this patch tested?

manual tests

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

$ git pull https://github.com/wangyum/spark SPARK-23831

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

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


commit 7d5cc71e4753f26fed4563a0eef27aa9de173d57
Author: Yuming Wang 
Date:   2018-03-30T10:41:42Z

Add org.apache.derby to IsolatedClientLoader




---

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