You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by heary-cao <gi...@git.apache.org> on 2017/09/16 10:49:53 UTC
[GitHub] spark pull request #19251: [SPARK-22035][SQL]the value of statistical logica...
GitHub user heary-cao opened a pull request:
https://github.com/apache/spark/pull/19251
[SPARK-22035][SQL]the value of statistical logicalPlan.stats.sizeInBytes which is not expected
## What changes were proposed in this pull request?
Currently, assume there will be the same number of rows as child has. statistics `logicalPlan.stats.SizeInBytes` is calculated based on the percentage of the size of the child's data type and the size of the current data type size. But there is a problem. Statistics are not very accurate. for example:
```
val N = 1 << 3
val df = spark.range(N).selectExpr("id as k1",
s"cast(id % 3 as string) as idString1",
s"cast((id + 1) % 5 as string) as idString3")
val sizeInBytes = df.logicalPlan.stats.sizeInBytes
println("sizeInBytes : " + sizeInBytes)
```
before modify：
sizeInBytes is 224(8 * 8 * ( (8 + 20 + 20 +8) / (8 + 8))).
debug information in ` SizeInBytesOnlyStatsPlanVisitor.visitUnaryNode `
```
p.child.dataType: LongType defaultSize: 8
p.dataType: LongType defaultSize: 8
p.dataType: StringType defaultSize: 20
p.dataType: StringType defaultSize: 20
childRowSize: 16 outputRowSize: 56
p.child.stats.sizeInBytes : 64
p.stats.sizeInBytes : 224
sizeInBytes: 224
```
after modify:
sizeInBytes is 384( 8 * 8 ((8 + 20 + 20) / 8) ).
debug information in ` SizeInBytesOnlyStatsPlanVisitor.visitUnaryNode `
```
p.child.dataType: LongType defaultSize: 8
p.dataType: LongType defaultSize: 8
p.dataType: StringType defaultSize: 20
p.dataType: StringType defaultSize: 20
childRowSize: 8 outputRowSize: 48
p.child.stats.sizeInBytes : 64
p.stats.sizeInBytes : 384
sizeInBytes: 384
```
this PR fix it.
## How was this patch tested?
add new test cases.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/heary-cao/spark sizeInBytes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19251.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 #19251
----
commit 99d48e5045775853ed7f487c94522e43f98a867a
Author: caoxuewen <ca...@zte.com.cn>
Date: 2017-09-16T10:11:46Z
improved statistics logicalPlan.stats.sizeInBytes
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19251: [SPARK-22035][SQL]the value of statistical logica...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19251#discussion_r143385665
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/SizeInBytesOnlyStatsPlanVisitor.scala ---
@@ -32,12 +31,14 @@ object SizeInBytesOnlyStatsPlanVisitor extends LogicalPlanVisitor[Statistics] {
* same as the output row number, and compute sizes based on the column types.
*/
private def visitUnaryNode(p: UnaryNode): Statistics = {
- // There should be some overhead in Row object, the size should not be zero when there is
- // no columns, this help to prevent divide-by-zero error.
- val childRowSize = p.child.output.map(_.dataType.defaultSize).sum + 8
- val outputRowSize = p.output.map(_.dataType.defaultSize).sum + 8
--- End diff --
yes
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19251: [SPARK-22035][SQL]the value of statistical logica...
Posted by heary-cao <gi...@git.apache.org>.
Github user heary-cao commented on a diff in the pull request:
https://github.com/apache/spark/pull/19251#discussion_r143377744
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/SizeInBytesOnlyStatsPlanVisitor.scala ---
@@ -32,12 +31,14 @@ object SizeInBytesOnlyStatsPlanVisitor extends LogicalPlanVisitor[Statistics] {
* same as the output row number, and compute sizes based on the column types.
*/
private def visitUnaryNode(p: UnaryNode): Statistics = {
- // There should be some overhead in Row object, the size should not be zero when there is
- // no columns, this help to prevent divide-by-zero error.
- val childRowSize = p.child.output.map(_.dataType.defaultSize).sum + 8
- val outputRowSize = p.output.map(_.dataType.defaultSize).sum + 8
--- End diff --
Does 8 represent the overhead of each row of data?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19251: [SPARK-22035][SQL]the value of statistical logica...
Posted by heary-cao <gi...@git.apache.org>.
Github user heary-cao closed the pull request at:
https://github.com/apache/spark/pull/19251
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19251: [SPARK-22035][SQL]the value of statistical logica...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19251#discussion_r142722330
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/SizeInBytesOnlyStatsPlanVisitor.scala ---
@@ -32,12 +31,14 @@ object SizeInBytesOnlyStatsPlanVisitor extends LogicalPlanVisitor[Statistics] {
* same as the output row number, and compute sizes based on the column types.
*/
private def visitUnaryNode(p: UnaryNode): Statistics = {
- // There should be some overhead in Row object, the size should not be zero when there is
- // no columns, this help to prevent divide-by-zero error.
- val childRowSize = p.child.output.map(_.dataType.defaultSize).sum + 8
- val outputRowSize = p.output.map(_.dataType.defaultSize).sum + 8
--- End diff --
The original codes look right.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19251: [SPARK-22035][SQL]the value of statistical logicalPlan.s...
Posted by heary-cao <gi...@git.apache.org>.
Github user heary-cao commented on the issue:
https://github.com/apache/spark/pull/19251
Leave a comment
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19251: [SPARK-22035][SQL]the value of statistical logicalPlan.s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19251
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org