Skip to content

Commit

Permalink
Prevent leaking state from internal/external row.
Browse files Browse the repository at this point in the history
  • Loading branch information
hvanhovell committed Jan 2, 2016
1 parent f6ecf14 commit 632b5dc
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
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 = Array(1, 2, "3", "IV", 6L)
val externalRow = Row(values.clone(): _*)
val internalRow = InternalRow(values.clone(): _*)

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
}
}
}

0 comments on commit 632b5dc

Please sign in to comment.