spark-test commented on code in PR #59890:
URL: https://github.com/apache/spark/pull/59890#discussion_r4406801453
##########
sql/core/src/test/scala/org/apache/spark/sql/CachedTableSuite.scala:
##########
@@ -855,6 +856,115 @@ class CachedTableSuite extends SharedSparkSession
}
}
+ private def appendParquetWithoutRefresh(path: String, value: Long): Unit = {
+ withTempDir { staging =>
+ val output = new File(staging, "data")
+ spark.range(value, value + 1).write.parquet(output.getCanonicalPath)
+ // Move files externally so Spark does not refresh the destination
before refreshByPath.
+ output.listFiles().filter(_.getName.endsWith(".parquet")).foreach { file
=>
+ Files.move(file.toPath, new File(path, file.getName).toPath)
+ }
+ }
+ }
+
+ for (relationCount <- Seq(1, 2, 3)) {
+ test(s"refreshByPath refreshes all $relationCount file indexes in one
cached plan") {
+ withTempDir { dir =>
+ val paths = (0 until relationCount).map(i => new File(dir,
s"part$i").getCanonicalPath)
+ paths.foreach(path => spark.range(1).write.parquet(path))
+ val cached =
paths.map(spark.read.parquet(_)).reduce(_.union(_)).cache()
Review Comment:
**Nit (P3):** These reads all resolve to V1
`LogicalRelation(HadoopFsRelation)` nodes, because the default
`spark.sql.sources.useV1SourceList` includes `parquet`, and the
refresh-counting tests below build V1 relations by hand. So only the V1 arm of
`lookupAndRefresh` is exercised. This PR also changes the V2
`ExtractV2Table(FileTable)` arm (CacheManager.scala:678-685), which now
refreshes every matching `FileTable` index and shares the per-entry identity
set, but no test builds a cached plan with two or more matching V2 reads. The
existing V2 `recacheByPath` tests in `V1TimeTravelCacheSuite` use a single
relation and only check that the cache was invalidated, not that the rebuilt
data is fresh. If the V2 arm regressed, this suite would still pass, while
users who opt into V2 file sources would get stale cached results after
`refreshByPath`. Running this union case again under
`withSQLConf(SQLConf.USE_V1_SOURCE_LIST.key -> "")`, as other file-source
suites do, would cover the V2 a
rm.
**Verification:**
- **Regression:** With V2 file sources enabled for parquet, build a cached
union of two or more FileTable reads under one parent directory and add files
outside Spark's writer. After refreshByPath on the parent, the cached query
must return complete fresh results, and the same case must fail when only the
first matching V2 index is refreshed.
- **Inspection:** The reads in the V2 variant must resolve to
DataSourceV2Relation nodes over a FileTable rather than to LogicalRelation
nodes over a HadoopFsRelation, so that the variant actually exercises the V2
refresh arm and not the already-covered V1 arm.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
