subject:"\[GitHub\] spark pull request\: \[SPARK\-7727\] \[SQL\] Avoid inner classes in Rule..."

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-28 Thread asfgit

Github user asfgit closed the pull request at:

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


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-28 Thread marmbrus

Github user marmbrus commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-167650817
  
Thanks, merging to master.


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166644466
  
**[Test build #48200 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48200/consoleFull)**
 for PR 10174 at commit 
[`8d36e38`](https://github.com/apache/spark/commit/8d36e386948defd17839323c697e62c2d1fb878c).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166670601
  
**[Test build #48200 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48200/consoleFull)**
 for PR 10174 at commit 
[`8d36e38`](https://github.com/apache/spark/commit/8d36e386948defd17839323c697e62c2d1fb878c).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`case class AssertNotNull(`\n  * `  * Abstract class all optimizers should 
inherit of, contains the standard batches (extending`\n  * `abstract class 
Optimizer extends RuleExecutor[LogicalPlan] `\n


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48229826
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -28,11 +28,13 @@ import org.apache.spark.sql.catalyst.plans.logical._
 import org.apache.spark.sql.catalyst.rules._
 import org.apache.spark.sql.types._
 
-abstract class Optimizer extends RuleExecutor[LogicalPlan]
-
-object DefaultOptimizer extends Optimizer {
-  val batches =
-// SubQueries are only needed for analysis and can be removed before 
execution.
+/**
+  * Abstract class all optimizers should inherit of, contains the standard 
batches (extending
--- End diff --

Done!


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48229820
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -28,11 +28,13 @@ import org.apache.spark.sql.catalyst.plans.logical._
 import org.apache.spark.sql.catalyst.rules._
 import org.apache.spark.sql.types._
 
-abstract class Optimizer extends RuleExecutor[LogicalPlan]
-
-object DefaultOptimizer extends Optimizer {
-  val batches =
-// SubQueries are only needed for analysis and can be removed before 
execution.
+/**
+  * Abstract class all optimizers should inherit of, contains the standard 
batches (extending
+  * Optimizers can override this.
+  */
+abstract class Optimizer extends RuleExecutor[LogicalPlan] {
+  def batches: Seq[Batch] =
+  // SubQueries are only needed for analysis and can be removed before 
execution.
--- End diff --

Good point! 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48229813
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -69,6 +71,13 @@ object DefaultOptimizer extends Optimizer {
 }
 
 /**
+  * Non-abstract representation of the standard Spark optimizing strategies
+  */
+object DefaultOptimizer extends Optimizer {
+  override def batches: Seq[Batch] = super.batches
--- End diff --

I changed it and modified the comment why we inherit!


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48229791
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/OptimizerExtendableSuite.scala
 ---
@@ -0,0 +1,54 @@
+/*
+ * 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
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.optimizer.Optimizer
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.rules.Rule
+
+/**
+  * This is a test for SPARK-7727 if the Optimizer is kept being extendable
+  */
+class OptimizerExtendableSuite extends SparkFunSuite {
+
+  /**
+* Dummy rule for test batches
+*/
+  object DummyRule extends Rule[LogicalPlan] {
+def apply(p: LogicalPlan): LogicalPlan = p
+  }
+
+  /**
+* This class represents a dummy extended optimizer that takes the 
batches of the
+* Optimizer and adds custom ones.
+*/
+  class ExtendedOptimizer extends Optimizer {
+
+// rules set to DummyRule, would not be executed anyways
+val myBatches: Seq[Batch] = Batch("once", Once, DummyRule) ::
+Batch("fixedPoint", FixedPoint(100), 
DummyRule) :: Nil
--- End diff --

Done!


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166560077
  
**[Test build #48175 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48175/consoleFull)**
 for PR 10174 at commit 
[`c7ea3b1`](https://github.com/apache/spark/commit/c7ea3b115540c8b1d428a56bb30434461e11f3e5).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * `  * 
Abstract class all optimizers should inherit of, contains the standard batches 
(extending`\n  * `abstract class Optimizer extends RuleExecutor[LogicalPlan] `\n


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166560115
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166560116
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48175/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166551520
  
**[Test build #48175 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48175/consoleFull)**
 for PR 10174 at commit 
[`c7ea3b1`](https://github.com/apache/spark/commit/c7ea3b115540c8b1d428a56bb30434461e11f3e5).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166670760
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48200/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-22 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166670758
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread marmbrus

Github user marmbrus commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48183908
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
 ---
@@ -59,7 +59,7 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] 
extends Logging {
   protected case class Batch(name: String, strategy: Strategy, rules: 
Rule[TreeType]*)
 
   /** Defines a sequence of rule batches, to be overridden by the 
implementation. */
-  protected val batches: Seq[Batch]
+  protected def batches: Seq[Batch]
--- End diff --

I don't think thats true.  You can't call `super` on a val.


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread marmbrus

Github user marmbrus commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48183993
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -69,6 +71,13 @@ object DefaultOptimizer extends Optimizer {
 }
 
 /**
+  * Non-abstract representation of the standard Spark optimizing strategies
+  */
+object DefaultOptimizer extends Optimizer {
+  override def batches: Seq[Batch] = super.batches
--- End diff --

I agree, this looks like a no-op.


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread marmbrus

Github user marmbrus commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48184082
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -28,11 +28,13 @@ import org.apache.spark.sql.catalyst.plans.logical._
 import org.apache.spark.sql.catalyst.rules._
 import org.apache.spark.sql.types._
 
-abstract class Optimizer extends RuleExecutor[LogicalPlan]
-
-object DefaultOptimizer extends Optimizer {
-  val batches =
-// SubQueries are only needed for analysis and can be removed before 
execution.
+/**
+  * Abstract class all optimizers should inherit of, contains the standard 
batches (extending
--- End diff --

closing parentheses? 


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166264736
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48105/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166264544
  
**[Test build #48105 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48105/consoleFull)**
 for PR 10174 at commit 
[`a7c9f5c`](https://github.com/apache/spark/commit/a7c9f5cc10234d68c84a59eb969e0e4cf3d84cf0).
 * 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166264734
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48212972
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/OptimizerExtendableSuite.scala
 ---
@@ -0,0 +1,54 @@
+/*
+ * 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
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.optimizer.Optimizer
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.rules.Rule
+
+/**
+  * This is a test for SPARK-7727 if the Optimizer is kept being extendable
+  */
+class OptimizerExtendableSuite extends SparkFunSuite {
+
+  /**
+* Dummy rule for test batches
+*/
+  object DummyRule extends Rule[LogicalPlan] {
+def apply(p: LogicalPlan): LogicalPlan = p
+  }
+
+  /**
+* This class represents a dummy extended optimizer that takes the 
batches of the
+* Optimizer and adds custom ones.
+*/
+  class ExtendedOptimizer extends Optimizer {
+
+// rules set to DummyRule, would not be executed anyways
+val myBatches: Seq[Batch] = Batch("once", Once, DummyRule) ::
+Batch("fixedPoint", FixedPoint(100), 
DummyRule) :: Nil
--- End diff --

You can follow `Optimizer` for how to indent these `Batch`es


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48212764
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
 ---
@@ -59,7 +59,7 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] 
extends Logging {
   protected case class Batch(name: String, strategy: Strategy, rules: 
Rule[TreeType]*)
 
   /** Defines a sequence of rule batches, to be overridden by the 
implementation. */
-  protected val batches: Seq[Batch]
+  protected def batches: Seq[Batch]
--- End diff --

Did a quick test, that's true, please ignore my previous comment.


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48212866
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -28,11 +28,13 @@ import org.apache.spark.sql.catalyst.plans.logical._
 import org.apache.spark.sql.catalyst.rules._
 import org.apache.spark.sql.types._
 
-abstract class Optimizer extends RuleExecutor[LogicalPlan]
-
-object DefaultOptimizer extends Optimizer {
-  val batches =
-// SubQueries are only needed for analysis and can be removed before 
execution.
+/**
+  * Abstract class all optimizers should inherit of, contains the standard 
batches (extending
--- End diff --

yea, parentheses `()`


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48127360
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
 ---
@@ -59,7 +59,7 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] 
extends Logging {
   protected case class Batch(name: String, strategy: Strategy, rules: 
Rule[TreeType]*)
 
   /** Defines a sequence of rule batches, to be overridden by the 
implementation. */
-  protected val batches: Seq[Batch]
+  protected def batches: Seq[Batch]
--- End diff --

we can still use `val` here


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-166240691
  
**[Test build #48105 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48105/consoleFull)**
 for PR 10174 at commit 
[`a7c9f5c`](https://github.com/apache/spark/commit/a7c9f5cc10234d68c84a59eb969e0e4cf3d84cf0).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48126327
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -28,44 +28,53 @@ import org.apache.spark.sql.catalyst.plans.logical._
 import org.apache.spark.sql.catalyst.rules._
 import org.apache.spark.sql.types._
 
-abstract class Optimizer extends RuleExecutor[LogicalPlan]
-
-object DefaultOptimizer extends Optimizer {
-  val batches =
-// SubQueries are only needed for analysis and can be removed before 
execution.
+/**
+  * Abstract class all optimizers should inherit of, contains the standard 
batches (extending
+  * Optimizers can override this.
+  */
+abstract class Optimizer extends RuleExecutor[LogicalPlan] {
+  def batches: Seq[Batch] =
+  // SubQueries are only needed for analysis and can be removed before 
execution.
 Batch("Remove SubQueries", FixedPoint(100),
   EliminateSubQueries) ::
-Batch("Aggregate", FixedPoint(100),
-  ReplaceDistinctWithAggregate,
-  RemoveLiteralFromGroupExpressions) ::
-Batch("Operator Optimizations", FixedPoint(100),
-  // Operator push down
-  SetOperationPushDown,
-  SamplePushDown,
-  ReorderJoin,
-  PushPredicateThroughJoin,
-  PushPredicateThroughProject,
-  PushPredicateThroughGenerate,
-  PushPredicateThroughAggregate,
-  ColumnPruning,
-  // Operator combine
-  ProjectCollapsing,
-  CombineFilters,
-  CombineLimits,
-  // Constant folding
-  NullPropagation,
-  OptimizeIn,
-  ConstantFolding,
-  LikeSimplification,
-  BooleanSimplification,
-  RemoveDispensableExpressions,
-  SimplifyFilters,
-  SimplifyCasts,
-  SimplifyCaseConversionExpressions) ::
-Batch("Decimal Optimizations", FixedPoint(100),
-  DecimalAggregates) ::
-Batch("LocalRelation", FixedPoint(100),
-  ConvertToLocalRelation) :: Nil
+  Batch("Aggregate", FixedPoint(100),
+ReplaceDistinctWithAggregate,
+RemoveLiteralFromGroupExpressions) ::
+  Batch("Operator Optimizations", FixedPoint(100),
+// Operator push down
+SetOperationPushDown,
+SamplePushDown,
+ReorderJoin,
+PushPredicateThroughJoin,
+PushPredicateThroughProject,
+PushPredicateThroughGenerate,
+PushPredicateThroughAggregate,
+ColumnPruning,
+// Operator combine
+ProjectCollapsing,
+CombineFilters,
+CombineLimits,
+// Constant folding
+NullPropagation,
+OptimizeIn,
+ConstantFolding,
+LikeSimplification,
+BooleanSimplification,
+RemoveDispensableExpressions,
+SimplifyFilters,
+SimplifyCasts,
+SimplifyCaseConversionExpressions) ::
+  Batch("Decimal Optimizations", FixedPoint(100),
+DecimalAggregates) ::
+  Batch("LocalRelation", FixedPoint(100),
+ConvertToLocalRelation) :: Nil
--- End diff --

Done!


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48127163
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -28,11 +28,13 @@ import org.apache.spark.sql.catalyst.plans.logical._
 import org.apache.spark.sql.catalyst.rules._
 import org.apache.spark.sql.types._
 
-abstract class Optimizer extends RuleExecutor[LogicalPlan]
-
-object DefaultOptimizer extends Optimizer {
-  val batches =
-// SubQueries are only needed for analysis and can be removed before 
execution.
+/**
+  * Abstract class all optimizers should inherit of, contains the standard 
batches (extending
--- End diff --

nit: add enclosing bracket


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48127180
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -28,11 +28,13 @@ import org.apache.spark.sql.catalyst.plans.logical._
 import org.apache.spark.sql.catalyst.rules._
 import org.apache.spark.sql.types._
 
-abstract class Optimizer extends RuleExecutor[LogicalPlan]
-
-object DefaultOptimizer extends Optimizer {
-  val batches =
-// SubQueries are only needed for analysis and can be removed before 
execution.
+/**
+  * Abstract class all optimizers should inherit of, contains the standard 
batches (extending
+  * Optimizers can override this.
+  */
+abstract class Optimizer extends RuleExecutor[LogicalPlan] {
+  def batches: Seq[Batch] =
--- End diff --

we can still use `val` here


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48127321
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -69,6 +71,13 @@ object DefaultOptimizer extends Optimizer {
 }
 
 /**
+  * Non-abstract representation of the standard Spark optimizing strategies
+  */
+object DefaultOptimizer extends Optimizer {
+  override def batches: Seq[Batch] = super.batches
--- End diff --

Do we need this override?


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-21 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48127272
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -28,11 +28,13 @@ import org.apache.spark.sql.catalyst.plans.logical._
 import org.apache.spark.sql.catalyst.rules._
 import org.apache.spark.sql.types._
 
-abstract class Optimizer extends RuleExecutor[LogicalPlan]
-
-object DefaultOptimizer extends Optimizer {
-  val batches =
-// SubQueries are only needed for analysis and can be removed before 
execution.
+/**
+  * Abstract class all optimizers should inherit of, contains the standard 
batches (extending
+  * Optimizers can override this.
+  */
+abstract class Optimizer extends RuleExecutor[LogicalPlan] {
+  def batches: Seq[Batch] =
+  // SubQueries are only needed for analysis and can be removed before 
execution.
--- End diff --

add 2 space before this comment, to align it to the "Remove SubQueries" 
batch it's talking about.


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-18 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-165764973
  
**[Test build #48002 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48002/consoleFull)**
 for PR 10174 at commit 
[`8b1a0f4`](https://github.com/apache/spark/commit/8b1a0f4f4500c51f45ae90882ae9e32495384bc3).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-18 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48018543
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/DefaultOptimizerExtendableSuite.scala
 ---
@@ -0,0 +1,45 @@
+/*
+ * 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
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.optimizer.{DefaultOptimizer, 
Optimizer}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.rules.Batch
+
+/**
+  * This is a test for SPARK-7727 if the Default Optimizer is kept being 
extendable
+  */
+class DefaultOptimizerExtendableSuite extends SparkFunSuite {
--- End diff --

That is true, i investigated it and proposed another change


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-18 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-165769694
  
**[Test build #48004 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48004/consoleFull)**
 for PR 10174 at commit 
[`78072af`](https://github.com/apache/spark/commit/78072af9026e5e91702c73faa85822b03df5ca95).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-18 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-165779961
  
**[Test build #48002 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48002/consoleFull)**
 for PR 10174 at commit 
[`8b1a0f4`](https://github.com/apache/spark/commit/8b1a0f4f4500c51f45ae90882ae9e32495384bc3).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * `  * 
Abstract class all optimizers should inherit of, contains the standard batches 
(extending`\n  * `abstract class Optimizer extends RuleExecutor[LogicalPlan] `\n


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-18 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-165788137
  
**[Test build #48004 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48004/consoleFull)**
 for PR 10174 at commit 
[`78072af`](https://github.com/apache/spark/commit/78072af9026e5e91702c73faa85822b03df5ca95).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * `  * 
Abstract class all optimizers should inherit of, contains the standard batches 
(extending`\n  * `abstract class Optimizer extends RuleExecutor[LogicalPlan] `\n


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-18 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-165780069
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48002/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-18 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-165780067
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-18 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-165788266
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48004/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-18 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r48047481
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 ---
@@ -28,44 +28,53 @@ import org.apache.spark.sql.catalyst.plans.logical._
 import org.apache.spark.sql.catalyst.rules._
 import org.apache.spark.sql.types._
 
-abstract class Optimizer extends RuleExecutor[LogicalPlan]
-
-object DefaultOptimizer extends Optimizer {
-  val batches =
-// SubQueries are only needed for analysis and can be removed before 
execution.
+/**
+  * Abstract class all optimizers should inherit of, contains the standard 
batches (extending
+  * Optimizers can override this.
+  */
+abstract class Optimizer extends RuleExecutor[LogicalPlan] {
+  def batches: Seq[Batch] =
+  // SubQueries are only needed for analysis and can be removed before 
execution.
 Batch("Remove SubQueries", FixedPoint(100),
   EliminateSubQueries) ::
-Batch("Aggregate", FixedPoint(100),
-  ReplaceDistinctWithAggregate,
-  RemoveLiteralFromGroupExpressions) ::
-Batch("Operator Optimizations", FixedPoint(100),
-  // Operator push down
-  SetOperationPushDown,
-  SamplePushDown,
-  ReorderJoin,
-  PushPredicateThroughJoin,
-  PushPredicateThroughProject,
-  PushPredicateThroughGenerate,
-  PushPredicateThroughAggregate,
-  ColumnPruning,
-  // Operator combine
-  ProjectCollapsing,
-  CombineFilters,
-  CombineLimits,
-  // Constant folding
-  NullPropagation,
-  OptimizeIn,
-  ConstantFolding,
-  LikeSimplification,
-  BooleanSimplification,
-  RemoveDispensableExpressions,
-  SimplifyFilters,
-  SimplifyCasts,
-  SimplifyCaseConversionExpressions) ::
-Batch("Decimal Optimizations", FixedPoint(100),
-  DecimalAggregates) ::
-Batch("LocalRelation", FixedPoint(100),
-  ConvertToLocalRelation) :: Nil
+  Batch("Aggregate", FixedPoint(100),
+ReplaceDistinctWithAggregate,
+RemoveLiteralFromGroupExpressions) ::
+  Batch("Operator Optimizations", FixedPoint(100),
+// Operator push down
+SetOperationPushDown,
+SamplePushDown,
+ReorderJoin,
+PushPredicateThroughJoin,
+PushPredicateThroughProject,
+PushPredicateThroughGenerate,
+PushPredicateThroughAggregate,
+ColumnPruning,
+// Operator combine
+ProjectCollapsing,
+CombineFilters,
+CombineLimits,
+// Constant folding
+NullPropagation,
+OptimizeIn,
+ConstantFolding,
+LikeSimplification,
+BooleanSimplification,
+RemoveDispensableExpressions,
+SimplifyFilters,
+SimplifyCasts,
+SimplifyCaseConversionExpressions) ::
+  Batch("Decimal Optimizations", FixedPoint(100),
+DecimalAggregates) ::
+  Batch("LocalRelation", FixedPoint(100),
+ConvertToLocalRelation) :: Nil
--- End diff --

can you undo these indent changes?


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-18 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-165788264
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-15 Thread marmbrus

Github user marmbrus commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47677958
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/DefaultOptimizerExtendableSuite.scala
 ---
@@ -0,0 +1,45 @@
+/*
+ * 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
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.optimizer.{DefaultOptimizer, 
Optimizer}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.rules.Batch
+
+/**
+  * This is a test for SPARK-7727 if the Default Optimizer is kept being 
extendable
+  */
+class DefaultOptimizerExtendableSuite extends SparkFunSuite {
--- End diff --

If this is the goal, I think this change could be a lot smaller (and thus a 
lot less disruptive to people who are already extending catalyst).

```
diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
index f608869..f52dab1 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
@@ -28,10 +28,8 @@ import org.apache.spark.sql.catalyst.plans.logical._
 import org.apache.spark.sql.catalyst.rules._
 import org.apache.spark.sql.types._
 
-abstract class Optimizer extends RuleExecutor[LogicalPlan]
-
-object DefaultOptimizer extends Optimizer {
-  val batches =
+abstract class Optimizer extends RuleExecutor[LogicalPlan] {
+  def batches =
 // SubQueries are only needed for analysis and can be removed before 
execution.
 Batch("Remove SubQueries", FixedPoint(100),
   EliminateSubQueries) ::
@@ -68,6 +66,11 @@ object DefaultOptimizer extends Optimizer {
   ConvertToLocalRelation) :: Nil
 }
 
+object DefaultOptimizer extends Optimizer {
+  val myBatches = Nil
+  override def batches = super.batches ++ myBatches
+}
+
 /**
  * Pushes operations down into a Sample.
  */
diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
index f80d2a9..62ea731 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
@@ -59,7 +59,7 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] 
extends Logging {
   protected case class Batch(name: String, strategy: Strategy, rules: 
Rule[TreeType]*)
 
   /** Defines a sequence of rule batches, to be overridden by the 
implementation. */
-  protected val batches: Seq[Batch]
+  protected def batches: Seq[Batch]
 
```


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-15 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164695506
  
**[Test build #47721 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47721/consoleFull)**
 for PR 10174 at commit 
[`fea0c58`](https://github.com/apache/spark/commit/fea0c58805f20eb4a44e58181e63e45e67bd5a23).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`case class Batch[TreeType <: TreeNode[_]] (name: String, strategy: Strategy, 
rules: Rule[TreeType]*)`\n  * `  case class FixedPoint(maxIterations: Int) 
extends Strategy`\n  * `abstract class Strategy `\n


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-15 Thread stephankessler

Github user stephankessler commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164682419
  
Just related, currently preparing appropriate documents to propose that to 
the community! Keep you posted!


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-15 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164695702
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47721/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-15 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164695698
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-15 Thread gatorsmile

Github user gatorsmile commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164785477
  
Thank you! Looking forward to reading your proposal! : )


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-14 Thread marmbrus

Github user marmbrus commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47581375
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/DefaultOptimizerExtendableSuite.scala
 ---
@@ -0,0 +1,45 @@
+/*
+ * 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
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.optimizer.{DefaultOptimizer, 
Optimizer}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.rules.Batch
+
+/**
+  * This is a test for SPARK-7727 if the Default Optimizer is kept being 
extendable
+  */
+class DefaultOptimizerExtendableSuite extends SparkFunSuite{
+
+  /**
+* This class represents a dummy extended optimizer that takes the 
rules of the
+* DefaultOptimizer and adds custom ones.
+*/
+  class ExtendedOptimizer extends Optimizer{
--- End diff --

BTW, we squash automatically when merging.


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-14 Thread marmbrus

Github user marmbrus commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47581335
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/DefaultOptimizerExtendableSuite.scala
 ---
@@ -0,0 +1,45 @@
+/*
+ * 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
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.optimizer.{DefaultOptimizer, 
Optimizer}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.rules.Batch
+
+/**
+  * This is a test for SPARK-7727 if the Default Optimizer is kept being 
extendable
+  */
+class DefaultOptimizerExtendableSuite extends SparkFunSuite{
+
+  /**
+* This class represents a dummy extended optimizer that takes the 
rules of the
+* DefaultOptimizer and adds custom ones.
+*/
+  class ExtendedOptimizer extends Optimizer{
--- End diff --

Hmm, weird.  Github is usually smarter than this.


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-14 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164402553
  
**[Test build #47648 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47648/consoleFull)**
 for PR 10174 at commit 
[`dfa642a`](https://github.com/apache/spark/commit/dfa642ae920ecf5f1a870397e15d282e001c05b7).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`case class Batch[TreeType <: TreeNode[_]] (name: String, strategy: Strategy, 
rules: Rule[TreeType]*)`\n  * `  case class FixedPoint(maxIterations: Int) 
extends Strategy`\n  * `abstract class Strategy `\n


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-14 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164402693
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-14 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164402695
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47648/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-14 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164675274
  
**[Test build #47721 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47721/consoleFull)**
 for PR 10174 at commit 
[`fea0c58`](https://github.com/apache/spark/commit/fea0c58805f20eb4a44e58181e63e45e67bd5a23).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-14 Thread 3ourroom

Github user 3ourroom commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164675412
  

NAVER - http://www.naver.com/


3ourr...@naver.com ëê» ë³´ë´ì  ë©ì¼  ì´ ë¤ìê³¼ ê°ì ì´ì ë¡ ì 
ì¡ ì¤í¨íìµëë¤.



ë°ë ì¬ëì´ íìëì ë©ì¼ì ìì ì°¨ë¨ íììµëë¤. 






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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-14 Thread gatorsmile

Github user gatorsmile commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164676681
  
Just curious. Is this for your project presented in "The Pushdown of 
Everything"? 


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-14 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47472268
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/DefaultOptimizerExtendableSuite.scala
 ---
@@ -0,0 +1,45 @@
+/*
+ * 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
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.optimizer.{DefaultOptimizer, 
Optimizer}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.rules.Batch
+
+/**
+  * This is a test for SPARK-7727 if the Default Optimizer is kept being 
extendable
+  */
+class DefaultOptimizerExtendableSuite extends SparkFunSuite{
+
+  /**
+* This class represents a dummy extended optimizer that takes the 
rules of the
+* DefaultOptimizer and adds custom ones.
+*/
+  class ExtendedOptimizer extends Optimizer{
--- End diff --

In my latest commit, there is one, in order to avoid confusing i (rebased) 
and squashed the commits to a single one!


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-14 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-164381751
  
**[Test build #47648 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47648/consoleFull)**
 for PR 10174 at commit 
[`dfa642a`](https://github.com/apache/spark/commit/dfa642ae920ecf5f1a870397e15d282e001c05b7).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-11 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-163889311
  
**[Test build #47577 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47577/consoleFull)**
 for PR 10174 at commit 
[`99968bc`](https://github.com/apache/spark/commit/99968bc91c2e70209b55ff17c49979f528c457bf).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`case class Batch[TreeType <: TreeNode[_]] (name: String, strategy: Strategy, 
rules: Rule[TreeType]*)`\n  * `  case class FixedPoint(maxIterations: Int) 
extends Strategy`\n  * `abstract class Strategy `\n


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-11 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-163889668
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47577/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-11 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-163889666
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-11 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47331692
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/DefaultOptimizerExtendableSuite.scala
 ---
@@ -0,0 +1,45 @@
+/*
+ * 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
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.optimizer.{DefaultOptimizer, 
Optimizer}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.rules.Batch
+
+/**
+  * This is a test for SPARK-7727 if the Default Optimizer is kept being 
extendable
+  */
+class DefaultOptimizerExtendableSuite extends SparkFunSuite{
+
+  /**
+* This class represents a dummy extended optimizer that takes the 
rules of the
+* DefaultOptimizer and adds custom ones.
+*/
+  class ExtendedOptimizer extends Optimizer{
--- End diff --

These is still no space before `{`?


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-11 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-163871707
  
**[Test build #47577 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47577/consoleFull)**
 for PR 10174 at commit 
[`99968bc`](https://github.com/apache/spark/commit/99968bc91c2e70209b55ff17c49979f528c457bf).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-11 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47329605
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/DefaultOptimizerExtendableSuite.scala
 ---
@@ -0,0 +1,45 @@
+/*
+ * 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
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.optimizer.{DefaultOptimizer, 
Optimizer}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.rules.Batch
+
+/**
+  * This is a test for SPARK-7727 if the Default Optimizer is kept being 
extendable
+  */
+class DefaultOptimizerExtendableSuite extends SparkFunSuite{
+
+  /**
+* This class represents a dummy extended optimizer that takes the 
rules of the
+* DefaultOptimizer and adds custom ones.
+*/
+  class ExtendedOptimizer extends Optimizer{
--- End diff --

Done


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-10 Thread marmbrus

Github user marmbrus commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47270183
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/DefaultOptimizerExtendableSuite.scala
 ---
@@ -0,0 +1,45 @@
+/*
+ * 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
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.optimizer.{DefaultOptimizer, 
Optimizer}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.rules.Batch
+
+/**
+  * This is a test for SPARK-7727 if the Default Optimizer is kept being 
extendable
+  */
+class DefaultOptimizerExtendableSuite extends SparkFunSuite{
+
+  /**
+* This class represents a dummy extended optimizer that takes the 
rules of the
+* DefaultOptimizer and adds custom ones.
+*/
+  class ExtendedOptimizer extends Optimizer{
--- End diff --

Nit: space before `{`


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-09 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-163179211
  
**[Test build #47425 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47425/consoleFull)**
 for PR 10174 at commit 
[`bc39e03`](https://github.com/apache/spark/commit/bc39e03d64d0ac338a12531e5cc4433acd18bdd0).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-09 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47071677
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
 ---
@@ -17,6 +17,8 @@
 
 package org.apache.spark.sql.catalyst.analysis
 
+import org.apache.spark.sql.catalyst.rules.RuleExecutor.{Batch, Once, 
FixedPoint}
--- End diff --

Done!


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-09 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47071770
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
 ---
@@ -56,12 +55,16 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] 
extends Logging {
   case class FixedPoint(maxIterations: Int) extends Strategy
 
   /** A batch of rules. */
-  protected case class Batch(name: String, strategy: Strategy, rules: 
Rule[TreeType]*)
+  case class Batch[TreeType <: TreeNode[_]]
--- End diff --

Got it, i moved Batch and Strategy to the rules package, and made Once and 
FixedPoint classes of the companion object of Strategy.


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-09 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-163199890
  
**[Test build #47425 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47425/consoleFull)**
 for PR 10174 at commit 
[`bc39e03`](https://github.com/apache/spark/commit/bc39e03d64d0ac338a12531e5cc4433acd18bdd0).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`case class Batch[TreeType <: TreeNode[_]] (name: String, strategy: Strategy, 
rules: Rule[TreeType]*)`\n  * `  case class FixedPoint(maxIterations: Int) 
extends Strategy`\n  * `abstract class Strategy `\n


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-09 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-163200019
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47425/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-09 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-163200018
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-09 Thread cloud-fan

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47090351
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/Strategy.scala 
---
@@ -0,0 +1,33 @@
+/*
+ * 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.rules
+
+object Strategy {
+  /** A strategy that only runs once. */
+  case object Once extends Strategy { val maxIterations = 1 }
--- End diff --

why do we put these 2 `case object` inside `object Strategy`?


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-09 Thread stephankessler

Github user stephankessler commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r47097859
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/Strategy.scala 
---
@@ -0,0 +1,33 @@
+/*
+ * 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.rules
+
+object Strategy {
+  /** A strategy that only runs once. */
+  case object Once extends Strategy { val maxIterations = 1 }
--- End diff --

First i thought about making both classes on the top level, however 
```org.apache.spark.sql.catalyst.rules.Once``` is not descriptive, while 
```org.apache.spark.sql.catalyst.rules.Strategy.Once``` is. That was my idea :)


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-08 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162888285
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47336/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-08 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162888281
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-08 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162890394
  
**[Test build #47337 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47337/consoleFull)**
 for PR 10174 at commit 
[`707ab3d`](https://github.com/apache/spark/commit/707ab3d30afcfc73f12ea049ff468d2b100fde52).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-08 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162921419
  
**[Test build #47337 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47337/consoleFull)**
 for PR 10174 at commit 
[`707ab3d`](https://github.com/apache/spark/commit/707ab3d30afcfc73f12ea049ff468d2b100fde52).
 * 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-08 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162921640
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47337/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-08 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162921638
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-08 Thread marmbrus

Github user marmbrus commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r46997515
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala
 ---
@@ -56,12 +55,16 @@ abstract class RuleExecutor[TreeType <: TreeNode[_]] 
extends Logging {
   case class FixedPoint(maxIterations: Int) extends Strategy
 
   /** A batch of rules. */
-  protected case class Batch(name: String, strategy: Strategy, rules: 
Rule[TreeType]*)
+  case class Batch[TreeType <: TreeNode[_]]
--- End diff --

I'd probably just make these top level classes if we are going to pull them 
out.  There is not a lot of stuff in the `rules` package.


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-08 Thread marmbrus

Github user marmbrus commented on a diff in the pull request:

https://github.com/apache/spark/pull/10174#discussion_r46997422
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
 ---
@@ -17,6 +17,8 @@
 
 package org.apache.spark.sql.catalyst.analysis
 
+import org.apache.spark.sql.catalyst.rules.RuleExecutor.{Batch, Once, 
FixedPoint}
--- End diff --

import ordering


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-07 Thread stephankessler

GitHub user stephankessler opened a pull request:

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

[SPARK-7727] [SQL] Avoid inner classes in RuleExecutor

Moved (case) classes Strategy, Once, FixedPoint and Batch to the companion 
object. This is necessary if we want to have the Optimizer easily extendable in 
the following sense: Usually a user wants to add additional rules, and just 
take the ones that are already there. However, inner classes made that 
impossible since the code did not compile

This allows easy extension of existing Optimizers see the 
DefaultOptimizerExtendableSuite for a corresponding test case.

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

$ git pull https://github.com/stephankessler/spark SPARK-7727

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

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


commit c48fd5f9efa970c856caf0fb52062597450965b0
Author: Stephan Kessler 
Date:   2015-12-07T07:38:31Z

* SPARK-7727 Avoid inner classes in RuleExecutor

Moved (case) classes Strategy, Once, FixedPoint and Batch to the companion 
object.

This allows easy extension of existing Optimizers see the 
DefaultOptimizerExtendableSuite for
a corresponding test case.

commit fb4988ab16b04753c8e8272de7227a325d22fc4c
Author: Stephan Kessler 
Date:   2015-12-07T13:56:25Z

Mod. SQLContext to access changed class locations




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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-07 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162534062
  
Can one of the admins verify this patch?


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-07 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162722926
  
**[Test build #47296 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47296/consoleFull)**
 for PR 10174 at commit 
[`fb4988a`](https://github.com/apache/spark/commit/fb4988ab16b04753c8e8272de7227a325d22fc4c).


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-07 Thread SparkQA

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162729095
  
**[Test build #47296 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47296/consoleFull)**
 for PR 10174 at commit 
[`fb4988a`](https://github.com/apache/spark/commit/fb4988ab16b04753c8e8272de7227a325d22fc4c).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`abstract class RuleExecutor[TreeType <: TreeNode[_]] extends Logging `\n


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

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-07 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162729172
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47296/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-07 Thread AmplabJenkins

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162729171
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request: [SPARK-7727] [SQL] Avoid inner classes in Rule...

2015-12-07 Thread cloud-fan

Github user cloud-fan commented on the pull request:

https://github.com/apache/spark/pull/10174#issuecomment-162719567
  
ok to test


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

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

86 matches

Mail list logo