You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mallman <gi...@git.apache.org> on 2016/10/28 17:49:49 UTC

[GitHub] spark pull request #15673: [SPARK-17992][SQL] Return all partitions from Hiv...

GitHub user mallman opened a pull request:

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

    [SPARK-17992][SQL] Return all partitions from HiveShim when Hive throws a metastore exception when attempting to fetch partitions by filter

    (Link to Jira issue: https://issues.apache.org/jira/browse/SPARK-17992)
    
    ## What changes were proposed in this pull request?
    
    We recently added table partition pruning for partitioned Hive tables converted to using `TableFileCatalog`. When the Hive configuration option `hive.metastore.try.direct.sql` is set to `false`, Hive will throw an exception for unsupported filter expressions. For example, attempting to filter on an integer partition column will throw a `org.apache.hadoop.hive.metastore.api.MetaException`.
    
    I discovered this behavior because VideoAmp uses the CDH version of Hive with a Postgresql metastore DB. In this configuration, CDH sets `hive.metastore.try.direct.sql` to `false` by default, and queries that filter on a non-string partition column will fail.
    
    Rather than throw an exception in query planning, this patch catches this exception, logs a warning and returns all table partitions instead. Clients of this method are already expected to handle the possibility that the filters will not be honored.
    
    ## How was this patch tested?
    
    A unit test was added.

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

    $ git pull https://github.com/VideoAmp/spark-public spark-17992-catch_hive_partition_filter_exception

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

    https://github.com/apache/spark/pull/15673.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 #15673
    
----
commit c62beda38fe16fb6fe391e125605cf6df90d1e57
Author: Michael Allman <mi...@videoamp.com>
Date:   2016-10-28T17:35:39Z

    [SPARK-17992][SQL] Return all partitions from HiveShim when Hive throws
    a metastore exception when attempting to fetch partitions by filter

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    @ericl I can do that, yes. I'm current tied down. I will push a new commit later today or tonight.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #67834 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67834/consoleFull)** for PR 15673 at commit [`4c438c8`](https://github.com/apache/spark/commit/4c438c8b2575880379e2a9a872fe07018cb62402).
     * This patch **fails Spark unit tests**.
     * This patch **does not merge cleanly**.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    @mallman can you bring this up-to-date?



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    Build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #67713 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67713/consoleFull)** for PR 15673 at commit [`c62beda`](https://github.com/apache/spark/commit/c62beda38fe16fb6fe391e125605cf6df90d1e57).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #67713 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67713/consoleFull)** for PR 15673 at commit [`c62beda`](https://github.com/apache/spark/commit/c62beda38fe16fb6fe391e125605cf6df90d1e57).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #3382 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3382/consoleFull)** for PR 15673 at commit [`4c438c8`](https://github.com/apache/spark/commit/4c438c8b2575880379e2a9a872fe07018cb62402).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    It looks like all the unit tests passed, however one of the forked test java processes exited with nonzero status for some unknown reason.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15673: [SPARK-17992][SQL] Return all partitions from Hiv...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #67859 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67859/consoleFull)** for PR 15673 at commit [`1ed3301`](https://github.com/apache/spark/commit/1ed3301ec4dcbcccde4cacd21909de4f97902e20).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    > Could we enable this fallback only when the conf is set to false? Otherwise, it might mask legitimate bugs.
    
    Certainly, but my intent with this PR is to prevent a (painful and confusing) regression for some Hive users of Spark 2.1 which can occur, because Spark 2.1 enables our new partition pruning implementation by default. I mentioned one case where this will happen, but we can't be sure this is the only case. If we make the conditions under which we use a fallback too narrow, we are assuming that other configurations of Hive are compatible with partition pruning outside of the specific conditions we check. I think that's a bit too risky. In fact, before submitting this PR I had written the catch block to catch and fall back for all types of `Exception`. What I ended up with here is a middle ground.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    @rxin I believe https://issues.apache.org/jira/browse/SPARK-18168 will need to be resolved before I can rebase this PR.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    LGTM


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    For large tables, the degraded performance should be considered a bug as well.
    
    How about this.
    1) If direct sql is disabled, log a warning about degraded performance with this flag and fall back to fetching all partitions.
    2) If direct sql is enabled, crash with a message suggesting to disable filesource partition management and report a bug.
    
    That way, we will know if there are cases where metastore pruning fails with direct sql enabled.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    This looks good to me. cc @cloud-fan 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    The current merge conflict is from d2d438d1d549628a0183e468ed11d6e85b5d6061, which touches the same code. I'll wait for that to be settled before rebasing.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #67772 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67772/consoleFull)** for PR 15673 at commit [`887e9b1`](https://github.com/apache/spark/commit/887e9b11899846405d4c61f028f422683ac89160).
     * This patch passes all tests.
     * This patch **does not merge cleanly**.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    Rebased.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    @mallman shall we go ahead and revert that in this PR? It didn't help with debugging the flaky test much.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15673: [SPARK-17992][SQL] Return all partitions from Hiv...

Posted by rezasafi <gi...@git.apache.org>.
Github user rezasafi commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15673#discussion_r211370552
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ---
    @@ -586,17 +587,31 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
             getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
           } else {
             logDebug(s"Hive metastore filter is '$filter'.")
    +        val tryDirectSqlConfVar = HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL
    +        val tryDirectSql =
    +          hive.getConf.getBoolean(tryDirectSqlConfVar.varname, tryDirectSqlConfVar.defaultBoolVal)
             try {
    +          // Hive may throw an exception when calling this method in some circumstances, such as
    +          // when filtering on a non-string partition column when the hive config key
    +          // hive.metastore.try.direct.sql is false
               getPartitionsByFilterMethod.invoke(hive, table, filter)
                 .asInstanceOf[JArrayList[Partition]]
             } catch {
    -          case e: InvocationTargetException =>
    -            // SPARK-18167 retry to investigate the flaky test. This should be reverted before
    -            // the release is cut.
    -            val retry = Try(getPartitionsByFilterMethod.invoke(hive, table, filter))
    -            logError("getPartitionsByFilter failed, retry success = " + retry.isSuccess)
    -            logError("all partitions: " + getAllPartitions(hive, table))
    -            throw e
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              !tryDirectSql =>
    +            logWarning("Caught Hive MetaException attempting to get partition metadata by " +
    +              "filter from Hive. Falling back to fetching all partition metadata, which will " +
    +              "degrade performance. Modifying your Hive metastore configuration to set " +
    +              s"${tryDirectSqlConfVar.varname} to true may resolve this problem.", ex)
    +            // HiveShim clients are expected to handle a superset of the requested partitions
    +            getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              tryDirectSql =>
    +            throw new RuntimeException("Caught Hive MetaException attempting to get partition " +
    --- End diff --
    
    @mallman sorry to disturb you here, but what is the reason that when direct sql isn't set only a warning is logged?and why when direct sql is set a runtime exception is being raised instead of just a warning like no direct sql case?   


---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    Build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #67834 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67834/consoleFull)** for PR 15673 at commit [`4c438c8`](https://github.com/apache/spark/commit/4c438c8b2575880379e2a9a872fe07018cb62402).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    cc @ericl


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15673: [SPARK-17992][SQL] Return all partitions from Hiv...

Posted by kmanamcheri <gi...@git.apache.org>.
Github user kmanamcheri commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15673#discussion_r221107390
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ---
    @@ -586,17 +587,31 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
             getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
           } else {
             logDebug(s"Hive metastore filter is '$filter'.")
    +        val tryDirectSqlConfVar = HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL
    +        val tryDirectSql =
    +          hive.getConf.getBoolean(tryDirectSqlConfVar.varname, tryDirectSqlConfVar.defaultBoolVal)
             try {
    +          // Hive may throw an exception when calling this method in some circumstances, such as
    +          // when filtering on a non-string partition column when the hive config key
    +          // hive.metastore.try.direct.sql is false
               getPartitionsByFilterMethod.invoke(hive, table, filter)
                 .asInstanceOf[JArrayList[Partition]]
             } catch {
    -          case e: InvocationTargetException =>
    -            // SPARK-18167 retry to investigate the flaky test. This should be reverted before
    -            // the release is cut.
    -            val retry = Try(getPartitionsByFilterMethod.invoke(hive, table, filter))
    -            logError("getPartitionsByFilter failed, retry success = " + retry.isSuccess)
    -            logError("all partitions: " + getAllPartitions(hive, table))
    -            throw e
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              !tryDirectSql =>
    +            logWarning("Caught Hive MetaException attempting to get partition metadata by " +
    +              "filter from Hive. Falling back to fetching all partition metadata, which will " +
    +              "degrade performance. Modifying your Hive metastore configuration to set " +
    +              s"${tryDirectSqlConfVar.varname} to true may resolve this problem.", ex)
    +            // HiveShim clients are expected to handle a superset of the requested partitions
    +            getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              tryDirectSql =>
    +            throw new RuntimeException("Caught Hive MetaException attempting to get partition " +
    --- End diff --
    
    @mallman Your assumption is incorrect. If Hive on direct sql fails, it will retry with ORM. So in this case, I am able to reproduce a issue with postgres where direct sql fails and if it retries with ORM, spark fails! Hive has fallback behavior for direct sql. 
    
    Filed SPARK-25561


---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    Happy to help.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    Could we enable this fallback only when the conf is set to false? Otherwise, it might mask legitimate bugs.
    
    I also wonder if some of our flaky tests around this issue are due to the conf being leaked by some suites...


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15673: [SPARK-17992][SQL] Return all partitions from Hiv...

Posted by ericl <gi...@git.apache.org>.
Github user ericl commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15673#discussion_r85856641
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ---
    @@ -585,7 +586,31 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
             getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
           } else {
             logDebug(s"Hive metastore filter is '$filter'.")
    -        getPartitionsByFilterMethod.invoke(hive, table, filter).asInstanceOf[JArrayList[Partition]]
    +        val tryDirectSqlConfVar = HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL
    +        val tryDirectSql =
    +          hive.getConf.getBoolean(tryDirectSqlConfVar.varname, tryDirectSqlConfVar.defaultBoolVal)
    +        try {
    +          // Hive may throw an exception when calling this method in some circumstances, such as
    +          // when filtering on a non-string partition column when the hive config key
    +          // hive.metastore.try.direct.sql is false
    +          getPartitionsByFilterMethod.invoke(hive, table, filter)
    +            .asInstanceOf[JArrayList[Partition]]
    +        } catch {
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              !tryDirectSql =>
    +            logWarning("Caught Hive MetaException attempting to get partition metadata by " +
    +              "filter from Hive. Falling back to fetching all partition metadata, which will " +
    +              "degrade performance. Consider modifying your Hive metastore configuration to " +
    +              s"set ${tryDirectSqlConfVar.varname} to true.", ex)
    +            // HiveShim clients are expected to handle a superset of the requested partitions
    +            getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              tryDirectSql =>
    +            throw new RuntimeException("Caught Hive MetaException attempting to get partition " +
    +              "metadata by filter from Hive. Set the Spark configuration setting " +
    --- End diff --
    
    You probably want word it to suggest disabling partition management as a workaround only.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    Merging in master. Thanks.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15673: [SPARK-17992][SQL] Return all partitions from Hiv...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15673#discussion_r216037341
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ---
    @@ -586,17 +587,31 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
             getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
           } else {
             logDebug(s"Hive metastore filter is '$filter'.")
    +        val tryDirectSqlConfVar = HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL
    +        val tryDirectSql =
    +          hive.getConf.getBoolean(tryDirectSqlConfVar.varname, tryDirectSqlConfVar.defaultBoolVal)
             try {
    +          // Hive may throw an exception when calling this method in some circumstances, such as
    +          // when filtering on a non-string partition column when the hive config key
    +          // hive.metastore.try.direct.sql is false
               getPartitionsByFilterMethod.invoke(hive, table, filter)
                 .asInstanceOf[JArrayList[Partition]]
             } catch {
    -          case e: InvocationTargetException =>
    -            // SPARK-18167 retry to investigate the flaky test. This should be reverted before
    -            // the release is cut.
    -            val retry = Try(getPartitionsByFilterMethod.invoke(hive, table, filter))
    -            logError("getPartitionsByFilter failed, retry success = " + retry.isSuccess)
    -            logError("all partitions: " + getAllPartitions(hive, table))
    -            throw e
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              !tryDirectSql =>
    +            logWarning("Caught Hive MetaException attempting to get partition metadata by " +
    +              "filter from Hive. Falling back to fetching all partition metadata, which will " +
    +              "degrade performance. Modifying your Hive metastore configuration to set " +
    +              s"${tryDirectSqlConfVar.varname} to true may resolve this problem.", ex)
    +            // HiveShim clients are expected to handle a superset of the requested partitions
    +            getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              tryDirectSql =>
    +            throw new RuntimeException("Caught Hive MetaException attempting to get partition " +
    --- End diff --
    
    Hi @rezasafi
    
    I believe the reasoning is if the user has disabled direct sql, we will try to fetch the partitions for the requested partition predicate anyway. However, since we don't expect that call to succeed, we just log a warning and fallback to the legacy behavior.
    
    On the other hand, if the user has enabled direct sql, then we expect the call to Hive to succeed. If it fails, we consider that an error and throw an exception.
    
    I hope that helps clarify things.


---

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


[GitHub] spark pull request #15673: [SPARK-17992][SQL] Return all partitions from Hiv...

Posted by rezasafi <gi...@git.apache.org>.
Github user rezasafi commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15673#discussion_r216111838
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ---
    @@ -586,17 +587,31 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
             getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
           } else {
             logDebug(s"Hive metastore filter is '$filter'.")
    +        val tryDirectSqlConfVar = HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL
    +        val tryDirectSql =
    +          hive.getConf.getBoolean(tryDirectSqlConfVar.varname, tryDirectSqlConfVar.defaultBoolVal)
             try {
    +          // Hive may throw an exception when calling this method in some circumstances, such as
    +          // when filtering on a non-string partition column when the hive config key
    +          // hive.metastore.try.direct.sql is false
               getPartitionsByFilterMethod.invoke(hive, table, filter)
                 .asInstanceOf[JArrayList[Partition]]
             } catch {
    -          case e: InvocationTargetException =>
    -            // SPARK-18167 retry to investigate the flaky test. This should be reverted before
    -            // the release is cut.
    -            val retry = Try(getPartitionsByFilterMethod.invoke(hive, table, filter))
    -            logError("getPartitionsByFilter failed, retry success = " + retry.isSuccess)
    -            logError("all partitions: " + getAllPartitions(hive, table))
    -            throw e
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              !tryDirectSql =>
    +            logWarning("Caught Hive MetaException attempting to get partition metadata by " +
    +              "filter from Hive. Falling back to fetching all partition metadata, which will " +
    +              "degrade performance. Modifying your Hive metastore configuration to set " +
    +              s"${tryDirectSqlConfVar.varname} to true may resolve this problem.", ex)
    +            // HiveShim clients are expected to handle a superset of the requested partitions
    +            getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              tryDirectSql =>
    +            throw new RuntimeException("Caught Hive MetaException attempting to get partition " +
    --- End diff --
    
    Thank you very much for the explanation @mallman. I appreciate it. 


---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #67859 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67859/consoleFull)** for PR 15673 at commit [`1ed3301`](https://github.com/apache/spark/commit/1ed3301ec4dcbcccde4cacd21909de4f97902e20).
     * This patch passes all tests.
     * This patch **does not merge cleanly**.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #67772 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67772/consoleFull)** for PR 15673 at commit [`887e9b1`](https://github.com/apache/spark/commit/887e9b11899846405d4c61f028f422683ac89160).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    @ericl I've pushed a commit with the changes you recommended.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15673: [SPARK-17992][SQL] Return all partitions from Hiv...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15673#discussion_r85633937
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ---
    @@ -585,7 +585,18 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
             getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
           } else {
             logDebug(s"Hive metastore filter is '$filter'.")
    -        getPartitionsByFilterMethod.invoke(hive, table, filter).asInstanceOf[JArrayList[Partition]]
    +        try {
    +          // Hive may throw an exception when calling this method in some circumstances, such as
    +          // when filtering on a non-string partition column when the hive config key
    +          // hive.metastore.try.direct.sql is false
    +          getPartitionsByFilterMethod.invoke(hive, table, filter)
    +            .asInstanceOf[JArrayList[Partition]]
    +        } catch {
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] =>
    +            logWarning("Caught MetaException attempting to get partitions by filter from Hive", ex)
    --- End diff --
    
    change the msg to say we are falling back to fetch all partitions' medatadata?



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    Build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #67949 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67949/consoleFull)** for PR 15673 at commit [`8d468ac`](https://github.com/apache/spark/commit/8d468ac7097de56989deee124ce65a6583f8eaa8).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #3382 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3382/consoleFull)** for PR 15673 at commit [`4c438c8`](https://github.com/apache/spark/commit/4c438c8b2575880379e2a9a872fe07018cb62402).
     * This patch passes all tests.
     * This patch **does not merge cleanly**.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15673: [SPARK-17992][SQL] Return all partitions from Hiv...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15673#discussion_r85865327
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ---
    @@ -585,7 +586,31 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
             getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
           } else {
             logDebug(s"Hive metastore filter is '$filter'.")
    -        getPartitionsByFilterMethod.invoke(hive, table, filter).asInstanceOf[JArrayList[Partition]]
    +        val tryDirectSqlConfVar = HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL
    +        val tryDirectSql =
    +          hive.getConf.getBoolean(tryDirectSqlConfVar.varname, tryDirectSqlConfVar.defaultBoolVal)
    +        try {
    +          // Hive may throw an exception when calling this method in some circumstances, such as
    +          // when filtering on a non-string partition column when the hive config key
    +          // hive.metastore.try.direct.sql is false
    +          getPartitionsByFilterMethod.invoke(hive, table, filter)
    +            .asInstanceOf[JArrayList[Partition]]
    +        } catch {
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              !tryDirectSql =>
    +            logWarning("Caught Hive MetaException attempting to get partition metadata by " +
    +              "filter from Hive. Falling back to fetching all partition metadata, which will " +
    +              "degrade performance. Consider modifying your Hive metastore configuration to " +
    +              s"set ${tryDirectSqlConfVar.varname} to true.", ex)
    +            // HiveShim clients are expected to handle a superset of the requested partitions
    +            getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              tryDirectSql =>
    +            throw new RuntimeException("Caught Hive MetaException attempting to get partition " +
    +              "metadata by filter from Hive. Set the Spark configuration setting " +
    --- End diff --
    
    I made some revisions. LMK what you think.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #15673: [SPARK-17992][SQL] Return all partitions from HiveShim w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/15673
  
    **[Test build #67949 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/67949/consoleFull)** for PR 15673 at commit [`8d468ac`](https://github.com/apache/spark/commit/8d468ac7097de56989deee124ce65a6583f8eaa8).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #15673: [SPARK-17992][SQL] Return all partitions from Hiv...

Posted by mallman <gi...@git.apache.org>.
Github user mallman commented on a diff in the pull request:

    https://github.com/apache/spark/pull/15673#discussion_r85864458
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ---
    @@ -585,7 +586,31 @@ private[client] class Shim_v0_13 extends Shim_v0_12 {
             getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
           } else {
             logDebug(s"Hive metastore filter is '$filter'.")
    -        getPartitionsByFilterMethod.invoke(hive, table, filter).asInstanceOf[JArrayList[Partition]]
    +        val tryDirectSqlConfVar = HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL
    +        val tryDirectSql =
    +          hive.getConf.getBoolean(tryDirectSqlConfVar.varname, tryDirectSqlConfVar.defaultBoolVal)
    +        try {
    +          // Hive may throw an exception when calling this method in some circumstances, such as
    +          // when filtering on a non-string partition column when the hive config key
    +          // hive.metastore.try.direct.sql is false
    +          getPartitionsByFilterMethod.invoke(hive, table, filter)
    +            .asInstanceOf[JArrayList[Partition]]
    +        } catch {
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              !tryDirectSql =>
    +            logWarning("Caught Hive MetaException attempting to get partition metadata by " +
    +              "filter from Hive. Falling back to fetching all partition metadata, which will " +
    +              "degrade performance. Consider modifying your Hive metastore configuration to " +
    +              s"set ${tryDirectSqlConfVar.varname} to true.", ex)
    +            // HiveShim clients are expected to handle a superset of the requested partitions
    +            getAllPartitionsMethod.invoke(hive, table).asInstanceOf[JSet[Partition]]
    +          case ex: InvocationTargetException if ex.getCause.isInstanceOf[MetaException] &&
    +              tryDirectSql =>
    +            throw new RuntimeException("Caught Hive MetaException attempting to get partition " +
    +              "metadata by filter from Hive. Set the Spark configuration setting " +
    --- End diff --
    
    Good point.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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