Repository navigation
Conversation
The test was skipped until parquet-rs accepted a NullType column's BOOLEAN + UNKNOWN encoding. apache/arrow-rs#9855 made it accept UNKNOWN on any physical type, and the parquet 59 crate includes it (apache#4199). Gate the test on Spark 4.1, the first version that writes NullType columns to Parquet, like the struct test beside it.
The 4.1.3 and 4.2.0 diffs ignored "SPARK-54220: vectorized reader: missing all struct fields, struct with NullType only" for apache#4199, which no longer reproduces. Both diffs were regenerated from Spark checkouts at their tags, after a round trip that reproduced the old diffs byte for byte. With the ignore gone, the diffs no longer touch ParquetIOSuite.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Which issue does this PR close?
Follow-up to #4199, which is closed because it no longer reproduces on
main.Rationale for this change
Two tests were skipped until parquet-rs accepted Spark's encoding of a
NullTypeParquet column,BOOLEANannotatedUNKNOWN. One is Comet's own SPARK-54220 reproducer inCometNativeReaderSuite. The other is Spark'sSPARK-54220: vectorized reader: missing all struct fields, struct with NullType onlyinParquetIOSuite, which the 4.1.3 and 4.2.0 diffs ignored. apache/arrow-rs#9855 made parquet-rs acceptUNKNOWNon any physical type, and the parquet 59 crate Comet uses includes it.What changes are included in this PR?
CometNativeReaderSuite: the reproducer runs on Spark 4.1 and later, the versions that writeNullTypecolumns to Parquet, behind the sameisSpark41Plusgate as the struct test beside it. Its comment no longer says the read fails.dev/diffs/4.1.3.diffanddev/diffs/4.2.0.diff: theIgnoreCometon the SPARK-54220 test is gone, and with it the diffs' only change toParquetIOSuite, including theIgnoreCometimport. Both diffs were regenerated from Spark checkouts at their tags, asspark-sql-tests.mddescribes. Before the change, applying and regenerating each diff reproduced it byte for byte. Both new diffs apply cleanly to fresh checkouts.How are these changes tested?
The re-enabled
CometNativeReaderSuitetest passes locally on Spark 4.1 and 4.2. Spark's own SPARK-54220 test runs in the Spark SQL jobs, so this PR carriesrun-spark-4.1-testsandrun-spark-4.2-tests.