dongjoon-hyun commented on code in PR #57443:
URL: https://github.com/apache/spark/pull/57443#discussion_r3658919064
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala:
##########
@@ -257,10 +265,14 @@ case class CachedRDDBuilder(
storageLevel: StorageLevel,
@transient cachedPlan: SparkPlan,
tableName: Option[String],
- @transient logicalPlan: LogicalPlan) {
+ @transient logicalPlan: LogicalPlan,
+ isCachedLogicalPlanRepeatable: Boolean = false,
+ hasSelectivePredicate: Boolean = false,
+ fileSourceOptions: Seq[Map[String, String]] = Seq.empty) {
@transient @volatile private var _cachedColumnBuffers: RDD[CachedBatch] =
null
- @transient @volatile private var _cachedColumnBuffersAreLoaded: Boolean =
false
+ @volatile private var isCachedRDDRepeatable = false
+ private var hasStrictFileSourceReads = true
Review Comment:
`hasStrictFileSourceReads` seems never to recover once it becomes `false`.
This starts as `true`, is only
AND-ed down in `buildBuffers()`, and is not reset here, while the other
per-generation states
(`partitionStats`, `isCachedRDDRepeatable`) are. So if any earlier build of
this builder observed a
best-effort read (e.g. the session temporarily had
`spark.sql.files.ignoreMissingFiles=true`), every
later generation stays non-repeatable even when it is rebuilt with strict
settings.
This is safe (it only disables the optimization), but it seems inconsistent
with the
generation-isolation design of this PR — a rebuilt generation re-derives
everything else from the
configuration in effect at rebuild time. Was this intentional? If not, how
about resetting it
together with the other generation state:
```diff
isCachedRDDRepeatable = false
+ // Strictness is re-derived from the configuration in effect when the
next generation is
+ // built; a best-effort read observed by an earlier generation must not
taint rebuilds.
+ hasStrictFileSourceReads = true
}
```
If it is intentional (treating the cache as permanently tainted once any
generation used
best-effort reads), a short comment on the field explaining that would help.
--
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]