[GitHub] spark pull request #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[watermen]

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

https://github.com/apache/spark/pull/14988#discussion_r79970830
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala
 ---
@@ -164,4 +164,19 @@ case class HiveTableScanExec(
   }
 
   override def output: Seq[Attribute] = attributes
+
+  override def sameResult(plan: SparkPlan): Boolean = plan match {
+case other: HiveTableScanExec =>
+  val thisPredicates = partitionPruningPred.map(cleanExpression)
+  val otherPredicates = other.partitionPruningPred.map(cleanExpression)
+
+  val result = relation.sameResult(other.relation) &&
+output.length == other.output.length &&
+  output.zip(other.output)
+.forall(p => p._1.name == p._2.name && p._1.dataType == 
p._2.dataType) &&
--- End diff --

@cloud-fan 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[watermen]

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

https://github.com/apache/spark/pull/14988#discussion_r79587529
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala
 ---
@@ -164,4 +164,19 @@ case class HiveTableScanExec(
   }
 
   override def output: Seq[Attribute] = attributes
+
+  override def sameResult(plan: SparkPlan): Boolean = plan match {
+case other: HiveTableScanExec =>
+  val thisPredicates = partitionPruningPred.map(cleanExpression)
+  val otherPredicates = other.partitionPruningPred.map(cleanExpression)
+
+  val result = relation.sameResult(other.relation) &&
+output.length == other.output.length &&
+  output.zip(other.output)
+.forall(p => p._1.name == p._2.name && p._1.dataType == 
p._2.dataType) &&
--- End diff --

For example, the full output of table is (a: Int, b: Int), and output1 is 
(a: Int), output2 is (b: Int), their dataType are same, but they are diff.


---
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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[cloud-fan]

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

https://github.com/apache/spark/pull/14988#discussion_r79568346
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala
 ---
@@ -164,4 +164,19 @@ case class HiveTableScanExec(
   }
 
   override def output: Seq[Attribute] = attributes
+
+  override def sameResult(plan: SparkPlan): Boolean = plan match {
+case other: HiveTableScanExec =>
+  val thisPredicates = partitionPruningPred.map(cleanExpression)
+  val otherPredicates = other.partitionPruningPred.map(cleanExpression)
+
+  val result = relation.sameResult(other.relation) &&
+output.length == other.output.length &&
+  output.zip(other.output)
+.forall(p => p._1.name == p._2.name && p._1.dataType == 
p._2.dataType) &&
--- End diff --

does the name matter? I'm not quite sure, but `LogicalRelation` only checks 
data type 


---
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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[watermen]

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

https://github.com/apache/spark/pull/14988#discussion_r79562122
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala
 ---
@@ -164,4 +164,28 @@ case class HiveTableScanExec(
   }
 
   override def output: Seq[Attribute] = attributes
+
+  override def sameResult(plan: SparkPlan): Boolean = plan match {
+case other: HiveTableScanExec =>
+  val thisRequestedAttributes = 
requestedAttributes.map(cleanExpression)
+  val otherRequestedAttributes = 
other.requestedAttributes.map(cleanExpression)
--- End diff --

I only override the `sameReult` like `FileSourceScanExec `. I think 
`HiveTableScanExec` is used to text format and `FileSourceScanExec ` is used to 
parquet/orc format.


---
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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[cloud-fan]

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

https://github.com/apache/spark/pull/14988#discussion_r79557679
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala
 ---
@@ -164,4 +164,28 @@ case class HiveTableScanExec(
   }
 
   override def output: Seq[Attribute] = attributes
+
+  override def sameResult(plan: SparkPlan): Boolean = plan match {
+case other: HiveTableScanExec =>
+  val thisRequestedAttributes = 
requestedAttributes.map(cleanExpression)
+  val otherRequestedAttributes = 
other.requestedAttributes.map(cleanExpression)
--- End diff --

can you follow the existing workaround?
```
override def sameResult(plan: LogicalPlan): Boolean = {
plan.canonicalized match {
  case LocalRelation(otherOutput, otherData) =>
otherOutput.map(_.dataType) == output.map(_.dataType) && otherData 
== data
  case _ => false
}
  }
```

then we don't need to override `cleanExpression`


---
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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[cloud-fan]

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

https://github.com/apache/spark/pull/14988#discussion_r78330099
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala
 ---
@@ -164,4 +164,28 @@ case class HiveTableScanExec(
   }
 
   override def output: Seq[Attribute] = attributes
+
+  override def sameResult(plan: SparkPlan): Boolean = plan match {
+case other: HiveTableScanExec =>
+  val thisRequestedAttributes = 
requestedAttributes.map(cleanExpression)
+  val otherRequestedAttributes = 
other.requestedAttributes.map(cleanExpression)
+
+  val result = partitionPruningPred == other.partitionPruningPred &&
+relation.sameResult(other.relation) &&
+  thisRequestedAttributes.zip(otherRequestedAttributes)
+.forall(p => p._1.semanticEquals(p._2))
+  result
+case _ => false
+  }
+
+  private def cleanExpression(e: Attribute): Expression = e match {
+case a: AttributeReference =>
+  // As the root of the expression, Alias will always take an 
arbitrary exprId, we need
--- End diff --

this comment doesn't match the code. Can you explain more about why the 
default `cleanExpression` doesn't work?


---
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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[watermen]

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

https://github.com/apache/spark/pull/14988#discussion_r78309372
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala
 ---
@@ -164,4 +164,11 @@ case class HiveTableScanExec(
   }
 
   override def output: Seq[Attribute] = attributes
+
+  override def sameResult(plan: SparkPlan): Boolean = plan match {
--- End diff --

`ReuseExchange` work in parquet/orc format, because `FileSourceScanExec` 
has override the `sameResult`.
if `left.cleanArgs == right.cleanArgs` return false, we can't run the next 
`(left.children, right.children).zipped.forall(_ sameResult _)`



---
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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[cloud-fan]

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

https://github.com/apache/spark/pull/14988#discussion_r77812716
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala
 ---
@@ -164,4 +164,11 @@ case class HiveTableScanExec(
   }
 
   override def output: Seq[Attribute] = attributes
+
+  override def sameResult(plan: SparkPlan): Boolean = plan match {
--- End diff --

ah, I think all leaf nodes suffer this problem, can you follow the way how 
they fix it? e.g.
```
  override def sameResult(plan: LogicalPlan): Boolean = {
plan.canonicalized match {
  case LocalRelation(otherOutput, otherData) =>
otherOutput.map(_.dataType) == output.map(_.dataType) && otherData 
== data
  case _ => false
}
  }
```


---
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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[watermen]

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

https://github.com/apache/spark/pull/14988#discussion_r77754923
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala
 ---
@@ -164,4 +164,11 @@ case class HiveTableScanExec(
   }
 
   override def output: Seq[Attribute] = attributes
+
+  override def sameResult(plan: SparkPlan): Boolean = plan match {
--- End diff --

`left.cleanArgs == right.cleanArgs` in defalut `sameResult` return false, 
because `equals` in `MetastoreRelation` compare the 
output(`AttributeReference`) and `exprId`s are diff. We need to erase the 
exprId.


---
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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[cloud-fan]

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

https://github.com/apache/spark/pull/14988#discussion_r77750354
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveTableScanExec.scala
 ---
@@ -164,4 +164,11 @@ case class HiveTableScanExec(
   }
 
   override def output: Seq[Attribute] = attributes
+
+  override def sameResult(plan: SparkPlan): Boolean = plan match {
--- End diff --

why the default one doesn't work?


---
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 #14988: [SPARK-17425][SQL] Override sameResult in HiveTab...

[watermen]

GitHub user watermen opened a pull request:

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

[SPARK-17425][SQL] Override sameResult in HiveTableScanExec to make 
ReusedExchange work in text format table

## What changes were proposed in this pull request?
The PR will override the `sameResult` in `HiveTableScanExec` to make 
`ReusedExchange` work in text format table.

## How was this patch tested?
# SQL
```sql
SELECT * FROM src t1
JOIN src t2 ON t1.key = t2.key
JOIN src t3 ON t1.key = t3.key;
```

# Before
```
== Physical Plan ==
*BroadcastHashJoin [key#30], [key#34], Inner, BuildRight
:- *BroadcastHashJoin [key#30], [key#32], Inner, BuildRight
:  :- *Filter isnotnull(key#30)
:  :  +- HiveTableScan [key#30, value#31], MetastoreRelation default, src
:  +- BroadcastExchange HashedRelationBroadcastMode(List(cast(input[0, int, 
false] as bigint)))
: +- *Filter isnotnull(key#32)
:+- HiveTableScan [key#32, value#33], MetastoreRelation default, src
+- BroadcastExchange HashedRelationBroadcastMode(List(cast(input[0, int, 
false] as bigint)))
   +- *Filter isnotnull(key#34)
  +- HiveTableScan [key#34, value#35], MetastoreRelation default, src
```

# After
```
== Physical Plan ==
*BroadcastHashJoin [key#2], [key#6], Inner, BuildRight
:- *BroadcastHashJoin [key#2], [key#4], Inner, BuildRight
:  :- *Filter isnotnull(key#2)
:  :  +- HiveTableScan [key#2, value#3], MetastoreRelation default, src
:  +- BroadcastExchange HashedRelationBroadcastMode(List(cast(input[0, int, 
false] as bigint)))
: +- *Filter isnotnull(key#4)
:+- HiveTableScan [key#4, value#5], MetastoreRelation default, src
+- ReusedExchange [key#6, value#7], BroadcastExchange 
HashedRelationBroadcastMode(List(cast(input[0, int, false] as bigint)))
```

cc: @davies @cloud-fan


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

$ git pull https://github.com/watermen/spark SPARK-17425

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

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


commit 8e537a161560a6d717a40d8aae44b1973dda9695
Author: Yadong Qi 
Date:   2016-09-07T01:26:46Z

Override sameResult in HiveTableScanExec.




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