You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/04/22 09:02:22 UTC
[GitHub] [spark] wangyum opened a new pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
wangyum opened a new pull request #32291:
URL: https://github.com/apache/spark/pull/32291
### What changes were proposed in this pull request?
This PR improves `Distinct` statistics estimation by rewrite it to `Aggregate`.
### Why are the changes needed?
The current implementation will missing column stat.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32291:
URL: https://github.com/apache/spark/pull/32291#discussion_r654727981
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/SizeInBytesOnlyStatsPlanVisitor.scala
##########
@@ -67,7 +67,7 @@ object SizeInBytesOnlyStatsPlanVisitor extends LogicalPlanVisitor[Statistics] {
}
}
- override def visitDistinct(p: Distinct): Statistics = default(p)
+ override def visitDistinct(p: Distinct): Statistics = visitUnaryNode(p)
Review comment:
Just to be consistent with the logic of `BasicStatsPlanVisitor`. The result will not change:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-824874932
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137796/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-824746735
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42325/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-824676219
**[Test build #137796 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137796/testReport)** for PR 32291 at commit [`00739ec`](https://github.com/apache/spark/commit/00739ecf716d7f4dae9458a43427f1c19a16e298).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-824862957
**[Test build #137796 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137796/testReport)** for PR 32291 at commit [`00739ec`](https://github.com/apache/spark/commit/00739ecf716d7f4dae9458a43427f1c19a16e298).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-824726073
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42325/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32291:
URL: https://github.com/apache/spark/pull/32291#discussion_r654666095
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/SizeInBytesOnlyStatsPlanVisitor.scala
##########
@@ -67,7 +67,7 @@ object SizeInBytesOnlyStatsPlanVisitor extends LogicalPlanVisitor[Statistics] {
}
}
- override def visitDistinct(p: Distinct): Statistics = default(p)
+ override def visitDistinct(p: Distinct): Statistics = visitUnaryNode(p)
Review comment:
Do we have a test coverage for this change? The newly added test case seems to pass without this change.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/SizeInBytesOnlyStatsPlanVisitor.scala
##########
@@ -67,7 +67,7 @@ object SizeInBytesOnlyStatsPlanVisitor extends LogicalPlanVisitor[Statistics] {
}
}
- override def visitDistinct(p: Distinct): Statistics = default(p)
+ override def visitDistinct(p: Distinct): Statistics = visitUnaryNode(p)
Review comment:
Do we have a test coverage for this change? The newly added test case seems to pass without this change. If this is required, please add a test coverage for this change.
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/statsEstimation/BasicStatsEstimationSuite.scala
##########
@@ -235,7 +235,6 @@ class BasicStatsEstimationSuite extends PlanTest with StatsEstimationTestBase {
test("SPARK-34121: Intersect operator missing rowCount when enable CBO") {
val intersect = Intersect(plan, plan, false)
- val childrenSize = intersect.children.size
Review comment:
super nit. irrelevant change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-824746735
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42325/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32291:
URL: https://github.com/apache/spark/pull/32291#discussion_r618498738
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/BasicStatsPlanVisitor.scala
##########
@@ -43,7 +43,10 @@ object BasicStatsPlanVisitor extends LogicalPlanVisitor[Statistics] {
AggregateEstimation.estimate(p).getOrElse(fallback(p))
}
- override def visitDistinct(p: Distinct): Statistics = default(p)
+ override def visitDistinct(p: Distinct): Statistics = {
+ val child = p.child
+ visitAggregate(Aggregate(child.output, child.output, child))
+ }
Review comment:
SPARK-34061 need this change:
https://github.com/apache/spark/pull/31113/files#diff-9973973f49899b41e043d219f8952275ec344237914e23c53c51a94b1eca9b52R46-R49
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32291:
URL: https://github.com/apache/spark/pull/32291#discussion_r654728179
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/statsEstimation/BasicStatsEstimationSuite.scala
##########
@@ -235,7 +235,6 @@ class BasicStatsEstimationSuite extends PlanTest with StatsEstimationTestBase {
test("SPARK-34121: Intersect operator missing rowCount when enable CBO") {
val intersect = Intersect(plan, plan, false)
- val childrenSize = intersect.children.size
Review comment:
`childrenSize` is useless. So remove it when touch this file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-824874932
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137796/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-864053745
Merged to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-864701873
the change looks good but we should merge PRs with review and approval. @wangyum please be more patient next time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-824676219
**[Test build #137796 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137796/testReport)** for PR 32291 at commit [`00739ec`](https://github.com/apache/spark/commit/00739ecf716d7f4dae9458a43427f1c19a16e298).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-861984801
cc @cloud-fan @maropu
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum closed pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
wangyum closed pull request #32291:
URL: https://github.com/apache/spark/pull/32291
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #32291: [SPARK-35185][SQL] Improve Distinct statistics estimation
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32291:
URL: https://github.com/apache/spark/pull/32291#issuecomment-864260218
Unfortunately, this seems to be merged with no review and approval.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org