[GitHub] [iceberg] kbendick commented on a change in pull request #3683: Spark: clarify alter column of complex types

[GitBox <gi...@apache.org>]

kbendick commented on a change in pull request #3683:
URL: https://github.com/apache/iceberg/pull/3683#discussion_r764338728



##########
File path: site/docs/spark-ddl.md
##########
@@ -182,6 +182,28 @@ ALTER TABLE prod.db.sample
 ADD COLUMN point.z double
 ```
 
+```sql
+-- create a nested array column of struct
+ALTER TABLE prod.db.sample
+ADD COLUMN points array<struct<x: double, y: double>>;
+
+-- add a field to the struct with in array

Review comment:
       Nit: within not with in.

##########
File path: site/docs/spark-ddl.md
##########
@@ -182,6 +182,28 @@ ALTER TABLE prod.db.sample
 ADD COLUMN point.z double
 ```
 
+```sql
+-- create a nested array column of struct
+ALTER TABLE prod.db.sample
+ADD COLUMN points array<struct<x: double, y: double>>;
+
+-- add a field to the struct with in array
+ALTER TABLE prod.db.sample
+ADD COLUMN points.element.z double
+```
+
+```sql
+-- create a map column of struct key and struct value
+ALTER TABLE prod.db.sample
+ADD COLUMN points map<struct<x: int>, struct<a: int>>;
+
+-- add a field to the value struct in a map
+ALTER TABLE prod.db.sample
+ADD COLUMN points.value.b int

Review comment:
       Nit: The comment is potentially ambiguous to me.
   
   I would try to be more clear about the fact that `value` is a keyword for accessing a map element. (vs it just being possibly part of the existing table's definition). Again, the comment in the added test is great!

##########
File path: site/docs/spark-ddl.md
##########
@@ -182,6 +182,28 @@ ALTER TABLE prod.db.sample
 ADD COLUMN point.z double
 ```
 
+```sql
+-- create a nested array column of struct
+ALTER TABLE prod.db.sample
+ADD COLUMN points array<struct<x: double, y: double>>;
+
+-- add a field to the struct with in array
+ALTER TABLE prod.db.sample
+ADD COLUMN points.element.z double

Review comment:
       Nit / non-blocking: You might consider documenting that `element` is a keyword for referring to the elements of an array.
   
   It's not necessarily clear to me as a reader that element is a keyword, and when it should be used. Possibly others feel differently though. The comment in the test is great in my opinion.




-- 
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: issues-unsubscribe@iceberg.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org