Re: [PR] [SPARK-58170][SQL][TESTS] Provide sql shorthand in SparkSessionProvider [spark]

["uros-b (via GitHub)" <gi...@apache.org>]

uros-b commented on code in PR #57300:
URL: https://github.com/apache/spark/pull/57300#discussion_r3700485976


##########
sql/core/src/test/scala/org/apache/spark/sql/classic/SparkSessionBinder.scala:
##########
@@ -24,4 +24,6 @@ import org.apache.spark.{sql, SparkFunSuite}
  */
 trait SparkSessionBinder extends sql.SparkSessionBinder { self: SparkFunSuite =>
   override protected def spark: SparkSession = super.spark.asInstanceOf[SparkSession]
+
+  override protected def sql(query: String): DataFrame = spark.sql(query)

Review Comment:
   The narrowing works, but I think it belongs in classic.SparkSessionProvider rather than here. That trait exists purely to narrow types (override protected def spark: SparkSession), and classic.QueryTest extends sql.QueryTest with classic.SparkSessionProvider without the binder; so classic-only suites built on classic.QueryTest still get an org.apache.spark.sql.DataFrame from sql(...) and still need testImplicits.castToImpl, which is exactly the annoyance this PR removes elsewhere.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org