srowen commented on a change in pull request #29434:
URL: https://github.com/apache/spark/pull/29434#discussion_r471125765
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/Row.scala
##########
@@ -314,7 +314,7 @@ trait Row extends Serializable {
*
* @throws ClassCastException when data type does not match.
*/
- def getSeq[T](i: Int): Seq[T] = getAs[Seq[T]](i)
+ def getSeq[T](i: Int): scala.collection.Seq[T] =
getAs[scala.collection.Seq[T]](i)
Review comment:
Yeah, there's a parallel problem though. If someone writes `val foo:
Seq[String] = getSeq[String](0)`, then in Scala 2.13, they are expecting an
immutable Seq. However this method returns `scala.collection.Seq` now, breaking
the code. Because all Scala apps would move to Seq meaning immutable Seq, I
think we actually want to follow that as it helps compatibility.
It means Scala 2.13 builds aren't necessarily source or binary compatible
with 2.12, but, that has never been true for any Scala app.
I think it's OK to encode arrays as (immutable) Seq even if it means
wrapping them, calling .toSeq or whatever, but that is perhaps the most
controversial part of a change like this.
Yes, the goal is to avoid parallel source trees if we can avoid it. It's
easier to manage and maybe allows fewer places where behavior might vary. That
means it won't be possible to use Scala 2.13-only classes, ideally.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
