You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/05/05 15:13:42 UTC

[GitHub] [iceberg] Fokko commented on pull request #4685: First version of the Accessor

Fokko commented on PR #4685:
URL: https://github.com/apache/iceberg/pull/4685#issuecomment-1118679163

   Hey @samredai It took me a while, but I noticed that we're doing post-order traversal over the schema. This made it challenging to implement the Accessors because it would first create the object that should be set to `inner`, and then the parent objects. Instead of visiting the leaves first, I've changed the code to pre-order traversal. This way we can just recursively construct the accessors.
   
   Let's consider the following schema:
   ```
   Schema: table {
     2: id: required int
     1: data: optional string
     3: location: optional struct<5: latitude: required float, 6: longitude: required float>
   }
   ```
   
   Before it would visit:
   ```
   Field: 2: id: required int
   Field: 1: data: optional string
   Field: 5: latitude: required float
   Field: 6: longitude: required float
   Struct: struct<5: latitude: required float, 6: longitude: required float>
   Field: 3: location: optional struct<5: latitude: required float, 6: longitude: required float>
   Struct: struct<2: id: required int, 1: data: optional string, 3: location: optional struct<5: latitude: required float, 6: longitude: required float>>
   ```
   We first visit all the fields, and then the objects. I've changed this to pre-order:
   ```
   Struct: struct<2: id: required int, 1: data: optional string, 3: location: optional struct<5: latitude: required float, 6: longitude: required float>>
   Field: 2: id: required int
   Field: 1: data: optional string
   Field: 3: location: optional struct<5: latitude: required float, 6: longitude: required float>
   Struct: struct<5: latitude: required float, 6: longitude: required float>
   Field: 5: latitude: required float
   Field: 6: longitude: required float
   ```
   Which makes more sense to me and it also allows me to simplify the code. Please let me know if I'm missing anything here.


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