[GitHub] spark pull request #18148: [SPARK-20926][SQL] Removing exposures to guava li...

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

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


---
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 #18148: [SPARK-20926][SQL] Removing exposures to guava li...

2017-06-06 Thread rezasafi 
GitHub user rezasafi reopened a pull request:

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

[SPARK-20926][SQL] Removing exposures to guava library caused by directly 
accessing  SessionCatalog's tableRelationCache


There could be test failures because DataStorageStrategy, 
HiveMetastoreCatalog and also HiveSchemaInferenceSuite were exposed to guava 
library by directly accessing SessionCatalog's tableRelationCacheg. These 
failures occur when guava shading is in place. 

## What changes were proposed in this pull request?
This change removes those guava exposures by introducing new methods in 
SessionCatalog and also changing DataStorageStrategy, HiveMetastoreCatalog and 
HiveSchemaInferenceSuite so that they use those proxy methods.

## How was this patch tested?

Unit tests passed after applying these changes. 


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

$ git pull https://github.com/rezasafi/spark branch-2.2

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

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


commit 8253bbe36d551f11d8e48ab92444977ac5b0776a
Author: Reza Safi 
Date:   2017-05-30T21:58:39Z

[SPARK-20926][SQL] Removing exposures to guava library through directly 
accessing  SessionCatalog's tableRelationCache

There were test failures because DataStorageStrategy, HiveMetastoreCatalog 
and also HiveSchemaInferenceSuite were exposed to the shaded Guava library. 
This change removes those exposures by introducing new methods in 
SessionCatalog.

commit 9821ea191d63b327663f29adb04b48c856c550ff
Author: Reza Safi 
Date:   2017-06-02T01:36:05Z

Making tableRelationCache private and updating the comments.

commit 942137299dc03de53ce3e7120ac052f5764c14dc
Author: Reza Safi 
Date:   2017-06-02T03:44:57Z

Fixing scalastyle check errors

commit 2832253afe2a48daae3f78568315b19a5aeb045f
Author: Reza Safi 
Date:   2017-06-02T23:49:49Z

Changing the names for two of the methods.




---
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 #18148: [SPARK-20926][SQL] Removing exposures to guava li...

2017-06-06 Thread rezasafi 
Github user rezasafi closed the pull request at:

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


---
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 #18148: [SPARK-20926][SQL] Removing exposures to guava li...

2017-06-02 Thread vanzin 
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/18148#discussion_r119968393
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
 ---
@@ -125,14 +126,36 @@ class SessionCatalog(
 if (conf.caseSensitiveAnalysis) name else name.toLowerCase(Locale.ROOT)
   }
 
-  /**
-   * A cache of qualified table names to table relation plans.
-   */
-  val tableRelationCache: Cache[QualifiedTableName, LogicalPlan] = {
+  private val tableRelationCache: Cache[QualifiedTableName, LogicalPlan] = 
{
 val cacheSize = conf.tableRelationCacheSize
 
CacheBuilder.newBuilder().maximumSize(cacheSize).build[QualifiedTableName, 
LogicalPlan]()
   }
 
+  /** This method provides a way to get a cached plan. */
+  def getCachedPlan(t: QualifiedTableName, c: Callable[LogicalPlan]): 
LogicalPlan = {
+tableRelationCache.get(t, c)
+  }
+
+  /** This method provides a way to get a cached plan if the key exists. */
+  def getCachedTableIfPresent(key: QualifiedTableName): LogicalPlan = {
--- End diff --

`getCachedTable` instead. I'd even just use `cachedTable` but this class at 
least seems to use verbs more than the rest of the code.


---
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 #18148: [SPARK-20926][SQL] Removing exposures to guava li...

2017-06-02 Thread vanzin 
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/18148#discussion_r119968406
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
 ---
@@ -125,14 +126,36 @@ class SessionCatalog(
 if (conf.caseSensitiveAnalysis) name else name.toLowerCase(Locale.ROOT)
   }
 
-  /**
-   * A cache of qualified table names to table relation plans.
-   */
-  val tableRelationCache: Cache[QualifiedTableName, LogicalPlan] = {
+  private val tableRelationCache: Cache[QualifiedTableName, LogicalPlan] = 
{
 val cacheSize = conf.tableRelationCacheSize
 
CacheBuilder.newBuilder().maximumSize(cacheSize).build[QualifiedTableName, 
LogicalPlan]()
   }
 
+  /** This method provides a way to get a cached plan. */
+  def getCachedPlan(t: QualifiedTableName, c: Callable[LogicalPlan]): 
LogicalPlan = {
+tableRelationCache.get(t, c)
+  }
+
+  /** This method provides a way to get a cached plan if the key exists. */
+  def getCachedTableIfPresent(key: QualifiedTableName): LogicalPlan = {
+tableRelationCache.getIfPresent(key)
+  }
+
+  /** This method provides a way to cache a plan. */
+  def putTableInCache(t: QualifiedTableName, l: LogicalPlan): Unit = {
--- End diff --

`cacheTable`


---
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 #18148: [SPARK-20926][SQL] Removing exposures to guava li...

2017-06-01 Thread vanzin 
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/18148#discussion_r119753432
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
 ---
@@ -126,14 +127,56 @@ class SessionCatalog(
   }
 
   /**
-   * A cache of qualified table names to table relation plans.
-   */
+* A cache of qualified table names to table relation plans.
+* Accessing tableRelationCache directly is not recommended,
+* since it will introduce exposures to guava libraries.
+*/
   val tableRelationCache: Cache[QualifiedTableName, LogicalPlan] = {
 val cacheSize = conf.tableRelationCacheSize
 
CacheBuilder.newBuilder().maximumSize(cacheSize).build[QualifiedTableName, 
LogicalPlan]()
   }
 
   /**
+* This method provides a way to get a cached plan
--- End diff --

The "without exposing components to Guava" part of all these comments is 
unnecessary.


---
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 #18148: [SPARK-20926][SQL] Removing exposures to guava li...

2017-06-01 Thread vanzin 
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/18148#discussion_r119752212
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
 ---
@@ -126,14 +127,56 @@ class SessionCatalog(
   }
 
   /**
-   * A cache of qualified table names to table relation plans.
-   */
+* A cache of qualified table names to table relation plans.
+* Accessing tableRelationCache directly is not recommended,
+* since it will introduce exposures to guava libraries.
+*/
   val tableRelationCache: Cache[QualifiedTableName, LogicalPlan] = {
--- End diff --

This seems to have been introduced in 2.2, can you make it private instead, 
and change all call sites?


---
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 #18148: [SPARK-20926][SQL] Removing exposures to guava li...

2017-05-30 Thread rezasafi 
GitHub user rezasafi opened a pull request:

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

[SPARK-20926][SQL] Removing exposures to guava library caused by directly 
accessing  SessionCatalog's tableRelationCacheg


There could be test failures because DataStorageStrategy, 
HiveMetastoreCatalog and also HiveSchemaInferenceSuite were exposed to guava 
library by directly accessing SessionCatalog's tableRelationCacheg. These 
failures occur when guava shading is in place. 

## What changes were proposed in this pull request?
This change removes those guava exposures by introducing new methods in 
SessionCatalog and also changing DataStorageStrategy, HiveMetastoreCatalog and 
HiveSchemaInferenceSuite so that they use those proxy methods.

## How was this patch tested?

Unit tests passed after applying these changes. 


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

$ git pull https://github.com/rezasafi/spark branch-2.2

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

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


commit 8253bbe36d551f11d8e48ab92444977ac5b0776a
Author: Reza Safi 
Date:   2017-05-30T21:58:39Z

[SPARK-20926][SQL] Removing exposures to guava library through directly 
accessing  SessionCatalog's tableRelationCache

There were test failures because DataStorageStrategy, HiveMetastoreCatalog 
and also HiveSchemaInferenceSuite were exposed to the shaded Guava library. 
This change removes those exposures by introducing new methods in 
SessionCatalog.




---
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