dwsmith1983 commented on code in PR #6116:
URL: https://github.com/apache/datafusion-comet/pull/6116#discussion_r4100408936
##########
spark/src/test/scala/org/apache/comet/parquet/ParquetReadSuite.scala:
##########
@@ -2152,6 +2153,295 @@ abstract class ParquetReadSuite extends CometTestBase {
}
}
}
+
+ // Spark's `ParquetReadSupport` checks for missing file ids before it looks
at
+ // `fieldId.read.enabled`, so the error is raised with id matching off as
well. With
+ // `ignoreMissing` set, both engines fall back to matching by name and read
real values.
+ test("read schema with field ids raises on a file without ids when id
matching is off") {
+ withSQLConf(SQLConf.PARQUET_FIELD_ID_READ_ENABLED.key -> "false") {
+ withTempPath { dir =>
+ val readSchema = new StructType().add("a", IntegerType, true,
withId(1))
+ val writeSchema = new StructType().add("a", IntegerType, true)
+ val writeData = Seq(Row(100), Row(200))
+ spark
+ .createDataFrame(spark.sparkContext.parallelize(writeData),
writeSchema)
+ .write
+ .mode("overwrite")
+ .parquet(dir.getCanonicalPath)
+
+ def readCause(): Throwable = intercept[SparkException] {
+ spark.read.schema(readSchema).parquet(dir.getCanonicalPath).collect()
+ }.getCause
+ withClue("Spark with Comet disabled") {
+ withSQLConf(CometConf.COMET_ENABLED.key -> "false") {
+ assertMissingIdsException(readCause())
+ }
+ }
+ withClue("Comet") {
+ assertMissingIdsException(readCause())
+ }
+
+ withSQLConf(SQLConf.IGNORE_MISSING_PARQUET_FIELD_ID.key -> "true") {
+
checkSparkAnswerAndOperator(spark.read.schema(readSchema).parquet(dir.getCanonicalPath))
+ }
+ }
+ }
+ }
+
+ // Spark's `containsFieldIds` walks the whole file schema, so ids that sit
only on struct
+ // children count. The root field whose id the file lacks is null filled
rather than rejected.
+ test("a file whose field ids are only on nested fields reads without a
missing-id error") {
+ withSQLConf(SQLConf.PARQUET_FIELD_ID_READ_ENABLED.key -> "true") {
+ withTempPath { dir =>
+ val nested = StructType(Seq(StructField("a", IntegerType, nullable =
true, withId(11))))
+ val writeSchema = new StructType().add("s", nested, true)
+ val readSchema = new StructType()
+ .add("s", nested, true)
+ .add("missing", IntegerType, true, withId(7))
+ val writeData = Seq(Row(Row(1)), Row(Row(2)))
+ spark
+ .createDataFrame(spark.sparkContext.parallelize(writeData),
writeSchema)
+ .write
+ .mode("overwrite")
+ .parquet(dir.getCanonicalPath)
+
+
checkSparkAnswerAndOperator(spark.read.schema(readSchema).parquet(dir.getCanonicalPath))
+ }
+ }
+ }
+
+ // Second half of Spark `ParquetFieldIdIOSuite.test("global read/write flag
should work
+ // correctly")`: the file carries ids but the read flag is off, so columns
resolve by name
+ // only. None of the read names exist in the file, so every value is null
and nothing raises.
+ test("field ids in the file are ignored when id matching is off") {
+ withSQLConf(
+ SQLConf.PARQUET_FIELD_ID_WRITE_ENABLED.key -> "true",
+ SQLConf.PARQUET_FIELD_ID_READ_ENABLED.key -> "false") {
+ withTempPath { dir =>
+ val readSchema = new StructType()
+ .add("some", IntegerType, true, withId(1))
+ .add("other", StringType, true, withId(2))
+ .add("name", StringType, true, withId(3))
+ val writeSchema = new StructType()
+ .add("a", IntegerType, true, withId(1))
+ .add("rand1", StringType, true, withId(2))
+ .add("rand2", StringType, true, withId(3))
+ val writeData = Seq(Row(100, "text", "txt"), Row(200, "more", "mr"))
+ spark
+ .createDataFrame(spark.sparkContext.parallelize(writeData),
writeSchema)
+ .write
+ .mode("overwrite")
+ .parquet(dir.getCanonicalPath)
+
+ val df = spark.read.schema(readSchema).parquet(dir.getCanonicalPath)
+ checkSparkAnswerAndOperator(df)
+ checkAnswer(df, Row(null, null, null) :: Row(null, null, null) :: Nil)
+ }
+ }
+ }
+
+ // Spark checks for missing file ids against the pruned read schema, so an
id on a column
+ // the query never projects does not reject a file without ids, whatever the
read flag says.
+ // The same read with both columns projected still raises.
+ test("field ids on an unprojected column do not reject a file without ids") {
Review Comment:
Merged into one test over one file with `a, b, s struct<a, b>` and ids on
`a` and `s.a`. `select b`, `select s.b` and `count(*)` read and compare against
Spark, and `select a` and `select s.a` raise in both engines. The `readEnabled`
loops are gone. `checkAnswer` after `checkSparkAnswerAndOperator` only remains
for the null fill in the mixed directory test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
