Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-12421][SQL] Prevent Internal/External row from exposing state. #10553

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -199,9 +199,9 @@ class GenericRow(protected[sql] val values: Array[Any]) extends Row {

override def get(i: Int): Any = values(i)

override def toSeq: Seq[Any] = values.toSeq
override def toSeq: Seq[Any] = values.clone()

override def copy(): Row = this
override def copy(): GenericRow = this
}

class GenericRowWithSchema(values: Array[Any], override val schema: StructType)
Expand All @@ -226,11 +226,11 @@ class GenericInternalRow(private[sql] val values: Array[Any]) extends BaseGeneri

override protected def genericGet(ordinal: Int) = values(ordinal)

override def toSeq(fieldTypes: Seq[DataType]): Seq[Any] = values
override def toSeq(fieldTypes: Seq[DataType]): Seq[Any] = values.clone()

override def numFields: Int = values.length

override def copy(): InternalRow = new GenericInternalRow(values.clone())
override def copy(): GenericInternalRow = this
}

/**
Expand Down
30 changes: 30 additions & 0 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/RowTest.scala
Expand Up @@ -104,4 +104,34 @@ class RowTest extends FunSpec with Matchers {
internalRow shouldEqual internalRow2
}
}

describe("row immutability") {
val values = Seq(1, 2, "3", "IV", 6L)
val externalRow = Row.fromSeq(values)
val internalRow = InternalRow.fromSeq(values)

def modifyValues(values: Seq[Any]): Seq[Any] = {
val array = values.toArray
array(2) = "42"
array
}

it("copy should return same ref for external rows") {
externalRow should be theSameInstanceAs externalRow.copy()
}

it("copy should return same ref for interal rows") {
internalRow should be theSameInstanceAs internalRow.copy()
}

it("toSeq should not expose internal state for external rows") {
val modifiedValues = modifyValues(externalRow.toSeq)
externalRow.toSeq should not equal modifiedValues
}

it("toSeq should not expose internal state for internal rows") {
val modifiedValues = modifyValues(internalRow.toSeq(Seq.empty))
internalRow.toSeq(Seq.empty) should not equal modifiedValues
}
}
}