You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/12/08 19:30:18 UTC

[GitHub] [spark] shardulm94 opened a new pull request #34839: [SPARK-37569][SQL] Don't mark nested view fields as nullable

shardulm94 opened a new pull request #34839:
URL: https://github.com/apache/spark/pull/34839

### What changes were proposed in this pull request?

When analyzing a view, we should not unnecessarily mark nested fields as nullable. If the columns projected by the view define themselves as non-nullable, their nullability should be preserved.

### Why are the changes needed?

Consider a view as follows with all fields non-nullable (required)
```
spark.sql("""
CREATE OR REPLACE VIEW v AS
SELECT id, named_struct('a', id) AS nested
FROM RANGE(10)
""")
```

When trying to read this view, it incorrectly marks nested column a as nullable
```
scala> spark.table("v2").printSchema
root
|-- id: long (nullable = false)
|-- nested: struct (nullable = false)
| |-- a: long (nullable = true)
```

However, we can see that the view schema has been correctly stored as non-nullable
```
scala> System.out.println(spark.sessionState.catalog.externalCatalog.getTable("default", "v2"))
CatalogTable(
Database: default
Table: v2
.
.
.
Schema: root
|-- id: long (nullable = false)
|-- nested: struct (nullable = false)
| |-- a: long (nullable = false)
)
```

This is caused by [this line](https://github.com/apache/spark/blob/fb40c0e19f84f2de9a3d69d809e9e4031f76ef90/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L3546) in Analyzer.scala. Going through the history of changes for this block of code, it seems like `asNullable` is a remnant of a time before we added [checks](https://github.com/apache/spark/blob/fb40c0e19f84f2de9a3d69d809e9e4031f76ef90/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L3543) to ensure that the from and to types of the cast were compatible. As nullability is already checked, it should be safe to add a cast without converting the target datatype to nullable.

### Does this PR introduce _any_ user-facing change?
Yes. View analysis will preserve nullability of nested fields instead of marking all nested fields as nullable.

### How was this patch tested?
Added unit 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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org