andygrove opened a new issue, #6794:
URL: https://github.com/apache/datafusion-comet/issues/6794

   ### Describe the bug
   
   When an Iceberg data file has no field ids and the table has no name mapping 
(`schema.name-mapping.default`), Spark reads every nested field as NULL, but 
the native Iceberg scan returns the values stored in the file. That covers 
struct fields, list elements and map values, so query results, filters and 
aggregates differ. `count(*) WHERE s.a IS NULL` is 50 in Spark and 0 in Comet. 
When the file's struct field names differ from the table's, Comet still returns 
the values, matched by position.
   
   Iceberg Java's fallback for such a file, `ParquetSchemaUtil.addFallbackIds`, 
assigns ids by position to the top-level fields only. Its readers then match 
nested fields by id, find none, and return NULL. A list of structs even comes 
back empty. iceberg-rust's fallback assigns the same top-level ids 
(`add_fallback_field_ids_to_arrow_schema`), but 
`get_arrow_projection_mask_fallback` reads each top-level column whole, and the 
reader then matches the file's nested fields to the table's by name, or by 
position when the names differ. The spec's column projection rules give null 
for a field id the file doesn't contain when there is no name mapping, which is 
what Spark returns for the nested fields.
   
   The iceberg-rust code is the same at Comet's current pin and at bb1e4a4 (the 
1.1 pin), so the 1.1.0 release behaves the same way.
   
   ### Steps to reproduce
   
   ```scala
   // Parquet files written without Iceberg field ids
   spark.sql("""SELECT CAST(id AS INT) AS id,
       named_struct('a', CAST(id AS INT), 'b', CAST(-id AS INT)) AS s,
       array(named_struct('x', CAST(id AS INT))) AS items,
       map('k', named_struct('v', CAST(id AS INT))) AS m
     FROM range(50)""").coalesce(1).write.parquet(dataPath)
   
   spark.sql("""CREATE TABLE cat.db.t (id INT, s STRUCT<a: INT, b: INT>,
     items ARRAY<STRUCT<x: INT>>, m MAP<STRING, STRUCT<v: INT>>) USING 
iceberg""")
   spark.sql(s"CREATE TABLE src USING parquet LOCATION '$dataPath'")
   // Adds the files without setting a name mapping.
   SparkTableUtil.importSparkTable(spark, TableIdentifier("src"), table, 
stagingDir)
   ```
   
   On main at 7d294535e8 (Spark 4.1, Iceberg 1.11):
   
   | Query | Spark | Comet |
   | --- | --- | --- |
   | `SELECT id, s FROM t WHERE id < 3 ORDER BY id` | `[0,[null,null]] 
[1,[null,null]] [2,[null,null]]` | `[0,[0,0]] [1,[1,-1]] [2,[2,-2]]` |
   | `SELECT id, items, m FROM t WHERE id < 3 ORDER BY id` | `[0,[],{k -> 
[null]}] …` | `[0,[[0]],{k -> [0]}] …` |
   | `SELECT s.a, s.b FROM t ORDER BY s.a LIMIT 3` | `[null,null]` three times 
| `[0,0] [1,-1] [2,-2]` |
   | `SELECT count(*) FROM t WHERE s.a IS NULL` | `[50]` | `[0]` |
   
   With the same data written under struct field names `p` and `q`, and the 
table still declaring `s STRUCT<a: INT, b: INT>`, `SELECT id, s.a, s.b` gives 
`[0,null,null] …` in Spark and `[0,0,0] [1,1,-1] [2,2,-2]` in Comet.
   
   ### Expected behavior
   
   The native scan returns what Spark returns for these files, which is NULL 
for the nested fields. iceberg-rust would have to stop reading the nested 
content of such a file, or null it out, to match Iceberg Java. A fallback in 
`CometScanRule` can't target just these files, because whether a file has field 
ids is only known once the native reader opens it.
   
   Two existing tests encode the current behavior and would change with a fix. 
`migration - INT96 timestamp` checks the native `ts_struct` against the raw 
Parquet file because Spark returns NULL for it, which the test puts down to 
INT96. Spark returns NULL for every nested field of such a file, so that isn't 
specific to INT96. `filter with nested types in migrated table` reads only flat 
columns, calling Spark's NULLs a separate Spark bug. #6725 also adds a test 
that asserts the native read returns non-NULL nested values for such a file.
   
   ### Additional context
   
   Found while reviewing #6725. Tables migrated with `migrate`, `snapshot` or 
`add_files` get a name mapping, so this needs files added without one, as 
`SparkTableUtil.importSparkTable` does. With a name mapping, the native scan 
has the opposite problem: #6790.
   


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

Reply via email to