HyukjinKwon commented on code in PR #58808:
URL: https://github.com/apache/spark/pull/58808#discussion_r4014302248
##########
python/pyspark/sql/tests/test_dataframe.py:
##########
@@ -1228,6 +1228,37 @@ def test_to_json(self):
self.assertIsInstance(df, DataFrame)
self.assertEqual(df.select("value").count(), 10)
+ def test_rdd_conversion_propagates_sql_conf(self):
+ # Converting a DataFrame to an RDD must run under a tracked SQL
execution so
+ # that session SQLConfs are propagated to the executors, mirroring the
classic
+ # Scala Dataset.rdd behavior (SPARK-50994). Otherwise the
non-vectorized parquet
+ # reader on the executor side would not see spark.sql.caseSensitive
and would
+ # resolve the two same-named but differently-cased columns wrongly,
failing to
+ # read the file. Accessing `.rdd` eagerly materializes the shuffle map
stage
+ # (the parquet scan) under adaptive execution; disabling shuffle-file
cleanup
+ # (off by default outside of tests) lets `collect` reuse that
materialized
+ # output instead of recomputing the scan without the propagated conf.
+ with self.sql_conf(
+ {
+ "spark.sql.caseSensitive": True,
+ "spark.sql.parquet.enableVectorizedReader": False,
+ "spark.sql.classic.shuffleDependency.fileCleanup.enabled":
False,
Review Comment:
The `fileCleanup.enabled=false` override makes the test reflect the default
production config: the conf defaults to `Utils.isTesting`, i.e. off in
production and only on under tests. With it left on, the deferred
`rdd.collect()` recomputes the scan outside the tracked execution — the
documented tradeoff of that conf, and the same interaction the classic
SPARK-50994 test sidesteps by exercising the conversion via a re-wrapped
DataFrame + `checkAnswer` instead of acting on the RDD directly.
So exercising the cleanup-enabled path here would mean asserting the
documented recompute behavior (a wrong/failing read), which I do not think we
want to lock in. The intent is covered by the existing comment on the test,
which notes cleanup is "off by default outside of tests" and that disabling it
lets `collect` reuse the materialized output instead of recomputing without the
propagated conf. Happy to expand that comment further if it 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]
