dongjoon-hyun commented on code in PR #57443:
URL: https://github.com/apache/spark/pull/57443#discussion_r3658991403
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala:
##########
@@ -392,11 +448,106 @@ case class CachedRDDBuilder(
}
}.persist(storageLevel)
cached.setName(cachedName)
+ isCachedRDDRepeatable = hasStrictFileSourceReads &&
+ cached.outputDeterministicLevel != DeterministicLevel.INDETERMINATE &&
+ InMemoryRelation.hasRepeatablePhysicalPlan(cachedPlan)
cached
}
}
-object InMemoryRelation {
+object InMemoryRelation extends PredicateHelper {
+
+ private val trustedFileFormatClasses: Set[Class[_ <: FileFormat]] = Set(
+ classOf[BinaryFileFormat],
+ classOf[CSVFileFormat],
+ classOf[JsonFileFormat],
+ classOf[OrcFileFormat],
+ classOf[ParquetFileFormat],
+ classOf[TextFileFormat])
+
+ private val trustedExternalFileFormatNames = Set(
+ "org.apache.spark.sql.avro.AvroFileFormat",
+ "org.apache.spark.sql.hive.orc.OrcFileFormat")
+
+ private def hasSafeExpressions(plan: QueryPlan[_]): Boolean = {
+ plan.expressions.forall { expression =>
+ !expression.exists {
+ case _: AesEncrypt | _: NonSQLExpression | _: UserDefinedExpression =>
true
+ case value => !value.deterministic ||
value.containsPattern(CURRENT_LIKE) ||
+
!value.getClass.getName.startsWith("org.apache.spark.sql.catalyst.expressions.")
Review Comment:
The
`getClass.getName.startsWith("org.apache.spark.sql.catalyst.expressions.")`
check looks like a fragile allow-by-package heuristic. It rejects unknown
expressions (safe direction), but it silently
*accepts* every current and future expression under that package whose
`deterministic` flag is
`true` — including ones whose output is not actually repeatable across
evaluations.
`AesEncrypt` is exactly such a case, and the fact that it needs an explicit
carve-out in the first pattern shows the failure mode: nothing prevents the
next `AesEncrypt`-like expression (deterministic-looking flag, runtime
randomness or external state) from being added to the package without anyone
updating this denylist.
--
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]
