You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by viirya <gi...@git.apache.org> on 2017/08/16 05:02:09 UTC

[GitHub] spark pull request #18956: [SPARK-21726][SQL] Check for structural integrity...

GitHub user viirya opened a pull request:

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

    [SPARK-21726][SQL] Check for structural integrity of the plan in Optimzer in test mode.

    ## What changes were proposed in this pull request?
    
    We have many optimization rules now in `Optimzer`. Right now we don't have any checks in the optimizer to check for the structural integrity of the plan (e.g. resolved). When debugging, it is difficult to identify which rules return invalid plans.
    
    It would be great if in test mode, we can check whether a plan is still resolved after the execution of each rule, so we can catch rules that return invalid plans.
    
    ## How was this patch tested?
    
    Added tests.


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

    $ git pull https://github.com/viirya/spark-1 SPARK-21726

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

    https://github.com/apache/spark/pull/18956.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 #18956
    
----
commit 21d86bac80790d0b994df79b5e27a7d2d354e90f
Author: Liang-Chi Hsieh <vi...@gmail.com>
Date:   2017-08-16T04:53:49Z

    Check for structural integrity of the plan in Optimzer in test mode.

----


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity...

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

    https://github.com/apache/spark/pull/18956#discussion_r133360995
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -37,6 +37,12 @@ import org.apache.spark.sql.types._
     abstract class Optimizer(sessionCatalog: SessionCatalog)
       extends RuleExecutor[LogicalPlan] {
     
    +  // Check for structural integrity of the plan in test mode. Currently we only check if a plan is
    +  // still resolved after the execution of each rule.
    +  override protected def planChecker: Option[LogicalPlan => Boolean] = Some(
    --- End diff --
    
    Thanks. I will update it.


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #80717 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80717/testReport)** for PR 18956 at commit [`9170ceb`](https://github.com/apache/spark/commit/9170ceb69fda3ae6a064b1941cd380ee7a2a13ed).


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark pull request #18956: [SPARK-21726][SQL] Check for structural integrity...

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

    https://github.com/apache/spark/pull/18956#discussion_r137679007
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala ---
    @@ -64,6 +64,14 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging {
       protected def batches: Seq[Batch]
     
       /**
    +   * Defines a check function which checks for structural integrity of the plan after the execution
    +   * of each rule. For example, we can check whether a plan is still resolved after each rule in
    +   * `Optimizer`, so we can catch rules that return invalid plans. The check function will returns
    +   * `false` if the given plan doesn't pass the structural integrity check.
    +   */
    +  protected def planChecker(plan: TreeType): Boolean = true
    --- End diff --
    
    Looks good.


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark pull request #18956: [SPARK-21726][SQL] Check for structural integrity...

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

    https://github.com/apache/spark/pull/18956#discussion_r137613115
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala ---
    @@ -64,6 +64,14 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging {
       protected def batches: Seq[Batch]
     
       /**
    +   * Defines a check function which checks for structural integrity of the plan after the execution
    +   * of each rule. For example, we can check whether a plan is still resolved after each rule in
    +   * `Optimizer`, so we can catch rules that return invalid plans. The check function will returns
    +   * `false` if the given plan doesn't pass the structural integrity check.
    +   */
    +  protected def planChecker(plan: TreeType): Boolean = true
    --- End diff --
    
    `planChecker ` -> `isPlanIntegral`?


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Merged 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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #80715 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80715/testReport)** for PR 18956 at commit [`21d86ba`](https://github.com/apache/spark/commit/21d86bac80790d0b994df79b5e27a7d2d354e90f).


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity...

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

    https://github.com/apache/spark/pull/18956#discussion_r133360047
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala ---
    @@ -37,6 +37,12 @@ import org.apache.spark.sql.types._
     abstract class Optimizer(sessionCatalog: SessionCatalog)
       extends RuleExecutor[LogicalPlan] {
     
    +  // Check for structural integrity of the plan in test mode. Currently we only check if a plan is
    +  // still resolved after the execution of each rule.
    +  override protected def planChecker: Option[LogicalPlan => Boolean] = Some(
    --- End diff --
    
    can we move the checking of whether this is a test in here, then this method simply returns boolean.


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #81518 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81518/testReport)** for PR 18956 at commit [`959e315`](https://github.com/apache/spark/commit/959e31539f7e26d919617bb9969edec6e4daf2fd).


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #81518 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81518/testReport)** for PR 18956 at commit [`959e315`](https://github.com/apache/spark/commit/959e31539f7e26d919617bb9969edec6e4daf2fd).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #81504 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81504/testReport)** for PR 18956 at commit [`e1e4aa1`](https://github.com/apache/spark/commit/e1e4aa1e80f0c0f549d74f34d3ae8a1bca5b372d).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class NettyMemoryMetrics implements MetricSet `
      * `  class ByteArrayConstructor extends net.razorvine.pickle.objects.ByteArrayConstructor `
      * `   *        (4) the main class for the child`
      * `public class JavaFeatureHasherExample `
      * `sealed trait LogisticRegressionTrainingSummary extends LogisticRegressionSummary `
      * `sealed trait BinaryLogisticRegressionSummary extends LogisticRegressionSummary `


---

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


[GitHub] spark pull request #18956: [SPARK-21726][SQL] Check for structural integrity...

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

    https://github.com/apache/spark/pull/18956#discussion_r137609405
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala ---
    @@ -93,6 +101,13 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging {
                     """.stripMargin)
                 }
     
    +            // Run the structural integrity checker against the plan after each rule.
    +            if (!planChecker(result)) {
    +              val message = s"After applying rule ${rule.ruleName} in batch ${batch.name}, " +
    +                "the structural integrity of the plan is broken."
    +              throw new TreeNodeException(result, message, null)
    --- End diff --
    
    move the exception throwing logics into the `planChecker `?


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #80715 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80715/testReport)** for PR 18956 at commit [`21d86ba`](https://github.com/apache/spark/commit/21d86bac80790d0b994df79b5e27a7d2d354e90f).
     * This patch **fails Spark unit 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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    The PR going to fix the issue in `PullupCorrelatedPredicates` is submitted at #18968.



---
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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #81079 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81079/testReport)** for PR 18956 at commit [`c99011d`](https://github.com/apache/spark/commit/c99011ddbf60ae104cb91c578d56c971e6b87c86).
     * This patch **fails Spark unit 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 #18956: [SPARK-21726][SQL] Check for structural integrity...

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

    https://github.com/apache/spark/pull/18956#discussion_r137680999
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala ---
    @@ -64,6 +64,14 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging {
       protected def batches: Seq[Batch]
     
       /**
    +   * Defines a check function which checks for structural integrity of the plan after the execution
    +   * of each rule. For example, we can check whether a plan is still resolved after each rule in
    +   * `Optimizer`, so we can catch rules that return invalid plans. The check function will returns
    --- End diff --
    
    `will returns` -> `returns`


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    `RewritePredicateSubquery` fails structural integrity check because it can produce `Join` with conflicting attributes in its left and right plans.
    
    I submitted #19050 to fix it.


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Merged 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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark pull request #18956: [SPARK-21726][SQL] Check for structural integrity...

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

    https://github.com/apache/spark/pull/18956#discussion_r137610235
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/OptimizerSICheckerSuite.scala ---
    @@ -0,0 +1,60 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.catalyst.optimizer
    +
    +import org.apache.spark.sql.catalyst.analysis.{EmptyFunctionRegistry, UnresolvedAttribute}
    +import org.apache.spark.sql.catalyst.catalog.{InMemoryCatalog, SessionCatalog}
    +import org.apache.spark.sql.catalyst.dsl.plans._
    +import org.apache.spark.sql.catalyst.errors.TreeNodeException
    +import org.apache.spark.sql.catalyst.expressions.{Alias, Literal}
    +import org.apache.spark.sql.catalyst.plans.PlanTest
    +import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, OneRowRelation, Project}
    +import org.apache.spark.sql.catalyst.rules._
    +import org.apache.spark.sql.internal.SQLConf
    +
    +
    +class OptimizerSICheckerkSuite extends PlanTest {
    --- End diff --
    
    -> OptimizerStructuralIntegrityCheckerkSuite


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    #19050 is merged now. Let's see if there is any rule can fail this structural integrity check.


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Seems there are other issues caused by `RewritePredicateSubquery` rule. I'll investigate and fix it.


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Interesting, existing `PullupCorrelatedPredicates` produces unresolved plan. I'll figure out the 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 issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80717/
    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 #18956: [SPARK-21726][SQL] Check for structural integrity...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80715/
    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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Thanks @rxin @gatorsmile 


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #81460 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81460/testReport)** for PR 18956 at commit [`c99011d`](https://github.com/apache/spark/commit/c99011ddbf60ae104cb91c578d56c971e6b87c86).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Merged 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 pull request #18956: [SPARK-21726][SQL] Check for structural integrity...

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

    https://github.com/apache/spark/pull/18956#discussion_r137681045
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala ---
    @@ -64,6 +64,14 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging {
       protected def batches: Seq[Batch]
     
       /**
    +   * Defines a check function which checks for structural integrity of the plan after the execution
    --- End diff --
    
    `which` -> `that`


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81079/
    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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #80718 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80718/testReport)** for PR 18956 at commit [`c99011d`](https://github.com/apache/spark/commit/c99011ddbf60ae104cb91c578d56c971e6b87c86).
     * This patch **fails Spark unit 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 #18956: [SPARK-21726][SQL] Check for structural integrity...

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

    https://github.com/apache/spark/pull/18956#discussion_r137678987
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/OptimizerSICheckerSuite.scala ---
    @@ -0,0 +1,60 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.catalyst.optimizer
    +
    +import org.apache.spark.sql.catalyst.analysis.{EmptyFunctionRegistry, UnresolvedAttribute}
    +import org.apache.spark.sql.catalyst.catalog.{InMemoryCatalog, SessionCatalog}
    +import org.apache.spark.sql.catalyst.dsl.plans._
    +import org.apache.spark.sql.catalyst.errors.TreeNodeException
    +import org.apache.spark.sql.catalyst.expressions.{Alias, Literal}
    +import org.apache.spark.sql.catalyst.plans.PlanTest
    +import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, OneRowRelation, Project}
    +import org.apache.spark.sql.catalyst.rules._
    +import org.apache.spark.sql.internal.SQLConf
    +
    +
    +class OptimizerSICheckerkSuite extends PlanTest {
    --- End diff --
    
    Ok.


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #80717 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80717/testReport)** for PR 18956 at commit [`9170ceb`](https://github.com/apache/spark/commit/9170ceb69fda3ae6a064b1941cd380ee7a2a13ed).
     * This patch **fails Spark unit 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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    **[Test build #81496 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81496/testReport)** for PR 18956 at commit [`e1e4aa1`](https://github.com/apache/spark/commit/e1e4aa1e80f0c0f549d74f34d3ae8a1bca5b372d).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class NettyMemoryMetrics implements MetricSet `
      * `  class ByteArrayConstructor extends net.razorvine.pickle.objects.ByteArrayConstructor `
      * `   *        (4) the main class for the child`
      * `public class JavaFeatureHasherExample `
      * `sealed trait LogisticRegressionTrainingSummary extends LogisticRegressionSummary `
      * `sealed trait BinaryLogisticRegressionSummary extends LogisticRegressionSummary `


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Merged 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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    LGTM except two minor comments


---

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


[GitHub] spark pull request #18956: [SPARK-21726][SQL] Check for structural integrity...

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

    https://github.com/apache/spark/pull/18956#discussion_r137611075
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala ---
    @@ -93,6 +101,13 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging {
                     """.stripMargin)
                 }
     
    +            // Run the structural integrity checker against the plan after each rule.
    +            if (!planChecker(result)) {
    +              val message = s"After applying rule ${rule.ruleName} in batch ${batch.name}, " +
    +                "the structural integrity of the plan is broken."
    +              throw new TreeNodeException(result, message, null)
    --- End diff --
    
    nvm. The message also has rule and batch names.


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    The reason `PullupCorrelatedPredicates` produces unresolved plan:
    
    The query causing the problem in `subquery_in_having.q` looks like:
    
        select b.key, min(b.value)
        from src b
        group by b.key
        having b.key in ( select a.key
                        from src a
                        where a.value > 'val_9' and a.value = min(b.value)
                        )
        order by b.key
        ;
    
    The optimized plan looks like:
    
        'Sort [key#201 ASC NULLS FIRST], true
        +- 'Project [key#201, min(value)#204]
           +- 'Filter key#201 IN (list#200 [(value#207 = min(value)#204)])
              :  +- Project [key#206, value#207]
              :     +- Filter (value#207 > val_9)
              :        +- InMemoryRelation [key#206, value#207], true, 5, StorageLevel(disk, memory, deserialized, 1 replicas), src
              :              +- HiveTableScan [key#0, value#1], HiveTableRelation `default`.`src`, org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe, [key#0, value#1]
           +- Aggregate [key#201], [key#201, min(value#202) AS min(value)#204, min(value#202) AS min(value#202)#209]
           +- InMemoryRelation [key#201, value#202], true, 5, StorageLevel(disk, memory, deserialized, 1 replicas), src
           +- HiveTableScan [key#0, value#1], HiveTableRelation `default`.`src`, org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe, [key#0, value#1]
    
    Before `PullupCorrelatedPredicates` rule, the subquery in `ListQuery` looks like:
    
        Project [key#206]
        +- Filter ((value#207 > val_9) && (value#207 = outer(min(value)#204)))
           +- InMemoryRelation [key#206, value#207], true, 5, StorageLevel(disk, memory, deserialized, 1 replicas), src
                 +- HiveTableScan [key#0, value#1], HiveTableRelation `default`.`src`, org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe, [key#0, value#1]
    
    Currently the `In` predicate in `Filter` is resolved.
    
    After the rule, the subquery looks like:
    
        Project [key#206, value#207]
        +- Filter (value#207 > val_9)
           +- InMemoryRelation [key#206, value#207], true, 5, StorageLevel(disk, memory, deserialized, 1 replicas), src
                 +- HiveTableScan [key#0, value#1], HiveTableRelation `default`.`src`, org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe, [key#0, value#1]
    
    Notice `key#206` has been added into `Project` and the condition `value#207 = outer(min(value)#204)` has been pulled out to top `Filter`:
    
         'Filter key#201 IN (list#200 [(value#207 = min(value)#204)])
    
    Because `In.checkInputDataTypes` checks if the size of left columns (key#201) matches the size of subquery output (key#206, value#207), so it fails and return `false` for `In.resolved`.
    
    The unresolved `Filter` doesn't cause problem before because it will be converted to `Join` by `RewritePredicateSubquery` rule later. But it has been detected by this structural integrity check.
    
    By modifying `In.checkInputDataTypes`, we can solve this issue. I'd submit another PR for it.


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80718/
    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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


---

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


[GitHub] spark issue #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

    https://github.com/apache/spark/pull/18956
  
    #18968 is merged. This should pass the tests now.


---
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 #18956: [SPARK-21726][SQL] Check for structural integrity of the...

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

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


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