Skip to content

Commit

Permalink
[SPARK-12589][SQL] Fix UnsafeRowParquetRecordReader to properly set t…
Browse files Browse the repository at this point in the history
…he row length.

The reader was previously not setting the row length meaning it was wrong if there were variable
length columns. This problem does not manifest usually, since the value in the column is correct and
projecting the row fixes the issue.

Author: Nong Li <[email protected]>

Closes apache#10576 from nongli/spark-12589.
  • Loading branch information
nongli authored and yhuai committed Jan 4, 2016
1 parent d084a2d commit 34de24a
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ public void pointTo(byte[] buf, int sizeInBytes) {
pointTo(buf, Platform.BYTE_ARRAY_OFFSET, sizeInBytes);
}

public void setTotalSize(int sizeInBytes) {
this.sizeInBytes = sizeInBytes;
}

public void setNotNullAt(int i) {
assertIndexIsValid(i);
BitSetMethods.unset(baseObject, baseOffset, i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,15 @@ private boolean loadBatch() throws IOException {
numBatched = num;
batchIdx = 0;
}

// Update the total row lengths if the schema contained variable length. We did not maintain
// this as we populated the columns.
if (containsVarLenFields) {
for (int i = 0; i < numBatched; ++i) {
rows[i].setTotalSize(rowWriters[i].holder().totalSize());
}
}

return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import org.apache.parquet.schema.{MessageType, MessageTypeParser}
import org.apache.spark.SparkException
import org.apache.spark.sql._
import org.apache.spark.sql.catalyst.ScalaReflection
import org.apache.spark.sql.catalyst.expressions.UnsafeRow
import org.apache.spark.sql.catalyst.util.DateTimeUtils
import org.apache.spark.sql.test.SharedSQLContext
import org.apache.spark.sql.types._
Expand Down Expand Up @@ -618,6 +619,29 @@ class ParquetIOSuite extends QueryTest with ParquetTest with SharedSQLContext {
readResourceParquetFile("dec-in-fixed-len.parquet"),
sqlContext.range(1 << 4).select('id % 10 cast DecimalType(10, 2) as 'fixed_len_dec))
}

test("SPARK-12589 copy() on rows returned from reader works for strings") {
withTempPath { dir =>
val data = (1, "abc") ::(2, "helloabcde") :: Nil
data.toDF().write.parquet(dir.getCanonicalPath)
var hash1: Int = 0
var hash2: Int = 0
(false :: true :: Nil).foreach { v =>
withSQLConf(SQLConf.PARQUET_UNSAFE_ROW_RECORD_READER_ENABLED.key -> v.toString) {
val df = sqlContext.read.parquet(dir.getCanonicalPath)
val rows = df.queryExecution.toRdd.map(_.copy()).collect()
val unsafeRows = rows.map(_.asInstanceOf[UnsafeRow])
if (!v) {
hash1 = unsafeRows(0).hashCode()
hash2 = unsafeRows(1).hashCode()
} else {
assert(hash1 == unsafeRows(0).hashCode())
assert(hash2 == unsafeRows(1).hashCode())
}
}
}
}
}
}

class JobCommitFailureParquetOutputCommitter(outputPath: Path, context: TaskAttemptContext)
Expand Down

0 comments on commit 34de24a

Please sign in to comment.