[SPARK-31703][SQL] Parquet RLE float/double are read incorrectly on big endian platforms #29419
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d in partition filters We support partially push partition filters since SPARK-28169. We can also support partially push down data filters if it mixed in partition filters and data filters. For example: ``` spark.sql( s""" |CREATE TABLE t(i INT, p STRING) |USING parquet |PARTITIONED BY (p)""".stripMargin) spark.range(0, 1000).selectExpr("id as col").createOrReplaceTempView("temp") for (part <- Seq(1, 2, 3, 4)) { sql(s""" |INSERT OVERWRITE TABLE t PARTITION (p='$part') |SELECT col FROM temp""".stripMargin) } spark.sql("SELECT * FROM t WHERE WHERE (p = '1' AND i = 1) OR (p = '2' and i = 2)").explain() ``` We can also push down ```i = 1 or i = 2 ``` Extract more data filter to FileSourceScanExec NO Added UT Closes #29406 from AngersZhuuuu/SPARK-32352. Authored-by: angerszhu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
…ig endian platforms ### What changes were proposed in this pull request? This PR fixes the issue introduced during SPARK-26985. SPARK-26985 changes the `putDoubles()` and `putFloats()` methods to respect the platform's endian-ness. However, that causes the RLE paths in VectorizedRleValuesReader.java to read the RLE entries in parquet as BIG_ENDIAN on big endian platforms (i.e., as is), even though parquet data is always in little endian format. The comments in `WriteableColumnVector.java` say those methods are used for "ieee formatted doubles in platform native endian" (or floats), but since the data in parquet is always in little endian format, use of those methods appears to be inappropriate. To demonstrate the problem with spark-shell: ```scala import org.apache.spark._ import org.apache.spark.sql._ import org.apache.spark.sql.types._ var data = Seq( (1.0, 0.1), (2.0, 0.2), (0.3, 3.0), (4.0, 4.0), (5.0, 5.0)) var df = spark.createDataFrame(data).write.mode(SaveMode.Overwrite).parquet("/tmp/data.parquet2") var df2 = spark.read.parquet("/tmp/data.parquet2") df2.show() ``` result: ```scala +--------------------+--------------------+ | _1| _2| +--------------------+--------------------+ | 3.16E-322|-1.54234871366845...| | 2.0553E-320| 2.0553E-320| | 2.561E-320| 2.561E-320| |4.66726145843124E-62| 1.0435E-320| | 3.03865E-319|-1.54234871366757...| +--------------------+--------------------+ ``` Also tests in ParquetIOSuite that involve float/double data would fail, e.g., - basic data types (without binary) - read raw Parquet file /examples/src/main/python/mllib/isotonic_regression_example.py would fail as well. Purposed code change is to add `putDoublesLittleEndian()` and `putFloatsLittleEndian()` methods for parquet to invoke, just like the existing `putIntsLittleEndian()` and `putLongsLittleEndian()`. On little endian platforms they would call `putDoubles()` and `putFloats()`, on big endian they would read the entries as little endian like pre-SPARK-26985. No new unit-test is introduced as the existing ones are actually sufficient. ### Why are the changes needed? RLE float/double data in parquet files will not be read back correctly on big endian platforms. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? All unit tests (mvn test) were ran and OK. Closes #29383 from tinhto-000/SPARK-31703. Authored-by: Tin Hang To <tinto@us.ibm.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit a418548) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
|
ok to test |
|
Test build #127387 has finished for PR 29419 at commit
|
|
thanks, merging to 2.4! |
cloud-fan
pushed a commit
that referenced
this pull request
Aug 13, 2020
…ig endian platforms ### What changes were proposed in this pull request? (back-porting from 9a3811d) This PR fixes the issue introduced during SPARK-26985. SPARK-26985 changes the `putDoubles()` and `putFloats()` methods to respect the platform's endian-ness. However, that causes the RLE paths in VectorizedRleValuesReader.java to read the RLE entries in parquet as BIG_ENDIAN on big endian platforms (i.e., as is), even though parquet data is always in little endian format. The comments in `WriteableColumnVector.java` say those methods are used for "ieee formatted doubles in platform native endian" (or floats), but since the data in parquet is always in little endian format, use of those methods appears to be inappropriate. To demonstrate the problem with spark-shell: ```scala import org.apache.spark._ import org.apache.spark.sql._ import org.apache.spark.sql.types._ var data = Seq( (1.0, 0.1), (2.0, 0.2), (0.3, 3.0), (4.0, 4.0), (5.0, 5.0)) var df = spark.createDataFrame(data).write.mode(SaveMode.Overwrite).parquet("/tmp/data.parquet2") var df2 = spark.read.parquet("/tmp/data.parquet2") df2.show() ``` result: ```scala +--------------------+--------------------+ | _1| _2| +--------------------+--------------------+ | 3.16E-322|-1.54234871366845...| | 2.0553E-320| 2.0553E-320| | 2.561E-320| 2.561E-320| |4.66726145843124E-62| 1.0435E-320| | 3.03865E-319|-1.54234871366757...| +--------------------+--------------------+ ``` Also tests in ParquetIOSuite that involve float/double data would fail, e.g., - basic data types (without binary) - read raw Parquet file /examples/src/main/python/mllib/isotonic_regression_example.py would fail as well. Purposed code change is to add `putDoublesLittleEndian()` and `putFloatsLittleEndian()` methods for parquet to invoke, just like the existing `putIntsLittleEndian()` and `putLongsLittleEndian()`. On little endian platforms they would call `putDoubles()` and `putFloats()`, on big endian they would read the entries as little endian like pre-SPARK-26985. No new unit-test is introduced as the existing ones are actually sufficient. ### Why are the changes needed? RLE float/double data in parquet files will not be read back correctly on big endian platforms. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? All unit tests (mvn test) were ran and OK. Closes #29419 from tinhto-000/SPARK-31703-2.4. Lead-authored-by: Tin Hang To <tinto@us.ibm.com> Co-authored-by: angerszhu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
(back-porting from 9a3811d)
This PR fixes the issue introduced during SPARK-26985.
SPARK-26985 changes the
putDoubles()andputFloats()methods to respect the platform's endian-ness. However, that causes the RLE paths in VectorizedRleValuesReader.java to read the RLE entries in parquet as BIG_ENDIAN on big endian platforms (i.e., as is), even though parquet data is always in little endian format.The comments in
WriteableColumnVector.javasay those methods are used for "ieee formatted doubles in platform native endian" (or floats), but since the data in parquet is always in little endian format, use of those methods appears to be inappropriate.To demonstrate the problem with spark-shell:
result:
Also tests in ParquetIOSuite that involve float/double data would fail, e.g.,
/examples/src/main/python/mllib/isotonic_regression_example.py would fail as well.
Purposed code change is to add
putDoublesLittleEndian()andputFloatsLittleEndian()methods for parquet to invoke, just like the existingputIntsLittleEndian()andputLongsLittleEndian(). On little endian platforms they would callputDoubles()andputFloats(), on big endian they would read the entries as little endian like pre-SPARK-26985.No new unit-test is introduced as the existing ones are actually sufficient.
Why are the changes needed?
RLE float/double data in parquet files will not be read back correctly on big endian platforms.
Does this PR introduce any user-facing change?
No
How was this patch tested?
All unit tests (mvn test) were ran and OK.