[SPARK-27266][SQL] Support ANALYZE TABLE to collect tables stats for cached catalog views #24200
Conversation
|
Test build #103895 has finished for PR 24200 at commit
|
|
retest this please |
|
Test build #103906 has finished for PR 24200 at commit
|
|
cc: @dongjoon-hyun |
|
Thank you for pinging me, @maropu . Yep. I'll take a look in a few hours. |
| statsOfPlanToCache.copy(sizeInBytes = cacheBuilder.sizeInBytesStats.value.longValue) | ||
| statsOfPlanToCache.copy( | ||
| sizeInBytes = cacheBuilder.sizeInBytesStats.value.longValue, | ||
| rowCount = Some(cacheBuilder.rowCountStats.value.longValue) |
There was a problem hiding this comment.
Is rowCount required additionally? If not, please remove this.
There was a problem hiding this comment.
Yea, we need it because this change passes rowCount into upper nodes;
scala> sql("CREATE VIEW v AS SELECT 1 c")
scala> sql("CACHE TABLE v")
scala> spark.table("v").explain(true)
...
== Optimized Logical Plan ==
InMemoryRelation [c#28], StorageLevel(disk, memory, deserialized, 1 replicas)
+- *(1) Project [1 AS c#1]
+- Scan OneRowRelation[]
...
> w/o this change
scala> val stats = spark.table("v").queryExecution.optimizedPlan.stats
.... Statistics(sizeInBytes=4.0 B)
> w/ this change
scala> val stats = spark.table("v").queryExecution.optimizedPlan.stats
.... Statistics(sizeInBytes=4.0 B, rowCount=1)
^^^^^^^^^^^
| val cacheManager = sparkSession.sharedState.cacheManager | ||
| if (cacheManager.lookupCachedData(table.logicalPlan).isDefined) { | ||
| // To collect table stats, materializes an underlying columnar RDD | ||
| table.collect() |
There was a problem hiding this comment.
Ur, @maropu , is this safe? Although ANALYZE TABLE is a heavy operation in general, Spark CBO collects statistics until now. For me, table.collect() looks too heavy.
Hi, @cloud-fan . Is this kind of heavy operations allowed?
There was a problem hiding this comment.
I wrote this code to do the same thing with the normal case:
|
Test build #103953 has finished for PR 24200 at commit
|
|
retest this please |
|
Test build #103959 has finished for PR 24200 at commit
|
| table.collect() | ||
| if (!noscan) { | ||
| // To collect table stats, materializes an underlying columnar RDD | ||
| table.collect() |
There was a problem hiding this comment.
According to the pointer you gave in the previous comment, table.count is enough for this?
There was a problem hiding this comment.
oh! I missed it... this is my silly mistake, sorry.
|
Test build #103999 has finished for PR 24200 at commit
|
|
Retest this please. |
|
Test build #104130 has finished for PR 24200 at commit
|
|
Retest this please. |
|
Test build #104144 has finished for PR 24200 at commit
|
| if (!noscan) { | ||
| // To collect table stats, materializes an underlying columnar RDD | ||
| table.count() | ||
| } |
There was a problem hiding this comment.
If noscan is true, this is no-op. We may want to show some warning or info here later. However, this is not a big deal. For now, this PR is much better than before because this prevents AnalysisException by supporting this. We can add doc later before 3.0.0 release (if needed.).
What changes were proposed in this pull request?
The current master doesn't support ANALYZE TABLE to collect tables stats for catalog views even if they are cached as follows;
Since SPARK-25196 has supported to an ANALYZE command to collect column statistics for cached catalog view, we could support table stats, too.
How was this patch tested?
Added tests in
StatisticsCollectionSuiteandInMemoryColumnarQuerySuite.