Iceberg and Delta target: sync schema field comments #575
Conversation
It fixed the issue that the comments are not synced correctly when the target table is Iceberg or Delta. For Delta target, extract the comment from internal schema and add to Spark schema representative. For Iceberg target, make sure any difference of comments is captured during syncSchema() and the schema is updated with new comments.
| if (schema.getComment() != null) { | ||
| metadataBuilder.putString(COMMENT, schema.getComment()); | ||
| } | ||
| return metadataBuilder.build(); | ||
| } | ||
|
|
||
| public InternalSchema toInternalSchema(StructType structType) { |
There was a problem hiding this comment.
Can the Map keys/values or the array values have comments on the field? if so, it does not look like they are being extracted read into the InternalSchema representation currently
There was a problem hiding this comment.
Good question, after checking the APIs, there is no comment can be added to array value or map key/values level:
https://spark.apache.org/docs/latest/api/java/org/apache/spark/sql/types/StructType.html
The name of the test is confusing, actually we tested the nested struct instead of array field: https://github.com/apache/incubator-xtable/pull/575/files#diff-9328d59be408d684281f3d5b2b7a9ca9d9b030e8e7075cd741796eb85eb2642cR328
There was a problem hiding this comment.
Thanks for looking into this, everything else looks good to me
|
Thank you for fixing this @emilie-wang! |
Important Read
Aim to address #525
What is the purpose of the pull request
It is expected to capture the comments and sync the comments of field when syncing schema. The PR makes the comments of schema fields are captured and synced correctly.
Brief change log
Verify this pull request
This change added tests and can be verified as follows: