pan3793 commented on PR #58314: URL: https://github.com/apache/spark/pull/58314#issuecomment-5440876273
Thanks for the review! **1.** Adopted. `cachedName` is now a `lazy val`. Both consumers are driver-side, and `cacheBuilder` is `@transient` in `InMemoryRelation`, so the builder is never deserialized on executors. **2.** Kept `getAndIncrement()` -- the first name is `CachedRDD 0` -- and fixed the doc and PR description instead. Also renamed the generator to follow the `SparkPlan.newPlanId()` precedent (`nextCachedRDDId` / `newCachedRDDId()`). **3.** Made the conf internal, renamed it to `spark.sql.dataframeCache.sequentialName.enabled`, and added `.withBindingPolicy(ConfigBindingPolicy.NOT_APPLICABLE)` -- the name only affects display. This was also behind the CI failure in `SparkConfigBindingPolicySuite`. **4.** Adopted: `cachedPlan.conf.getConf(...)`. **5.** Added a short comment noting the sequential id is consumed when the lazy val is first forced. **6.** Added the `SPARK-59024:` prefix; the disabled case now asserts equality against `Utils.abbreviate(cachedPlan.toString, 1024)`. -- 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]
