You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Smith-Cruise (via GitHub)" <gi...@apache.org> on 2025/07/10 11:34:44 UTC

[PR] [Docs] Fix snapshot-id wrong type in Spec [iceberg]

Smith-Cruise opened a new pull request, #13513:
URL: https://github.com/apache/iceberg/pull/13513

   snapshot-id should use type `long`, I think here is just a typo.


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


Re: [PR] [Docs] Fix snapshot-id wrong type in Spec [iceberg]

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #13513:
URL: https://github.com/apache/iceberg/pull/13513#discussion_r2197709051


##########
format/spec.md:
##########
@@ -970,7 +970,7 @@ Statistics files metadata within `statistics` table metadata field is a struct w
 
 | v1 | v2 | Field name | Type | Description |
 |----|----|------------|------|-------------|
-| _required_ | _required_ | **`snapshot-id`** | `string` | ID of the Iceberg table's snapshot the statistics file is associated with. |
+| _required_ | _required_ | **`snapshot-id`** | `long` | ID of the Iceberg table's snapshot the statistics file is associated with. |

Review Comment:
   cc: @findepi since you worked on this spec



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


Re: [PR] [Docs] Fix snapshot-id wrong type in Spec [iceberg]

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #13513:
URL: https://github.com/apache/iceberg/pull/13513#discussion_r2197707885


##########
format/spec.md:
##########
@@ -970,7 +970,7 @@ Statistics files metadata within `statistics` table metadata field is a struct w
 
 | v1 | v2 | Field name | Type | Description |
 |----|----|------------|------|-------------|
-| _required_ | _required_ | **`snapshot-id`** | `string` | ID of the Iceberg table's snapshot the statistics file is associated with. |
+| _required_ | _required_ | **`snapshot-id`** | `long` | ID of the Iceberg table's snapshot the statistics file is associated with. |

Review Comment:
   Aaah. Good catch. 
   
   These puffin stats spec was approved 3 years ago. 
   https://github.com/apache/iceberg/commit/4687798af7913f25dea9a1d6badd252f429a6810
   
   I can see that [java](https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/StatisticsFile.java#L32) and [python](https://github.com/apache/iceberg-python/blob/479e6639103be367e218c16e83c22bc893400eb3/pyiceberg/table/statistics.py#L35) implementations use `long`.
   
   **But there can be some implementation out there that uses string (as per spec).** 
   
   Also, at Iceberg we need mailing list discussion and [vote](https://iceberg.apache.org/contribute/?h=contributing#merging-pull-requests) for notable spec change. 



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


Re: [PR] [Spec]: Fix wrong type for snapshot-id in table statistics [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #13513:
URL: https://github.com/apache/iceberg/pull/13513#discussion_r2210183773


##########
format/spec.md:
##########
@@ -970,7 +970,7 @@ Statistics files metadata within `statistics` table metadata field is a struct w
 
 | v1 | v2 | Field name | Type | Description |
 |----|----|------------|------|-------------|
-| _required_ | _required_ | **`snapshot-id`** | `string` | ID of the Iceberg table's snapshot the statistics file is associated with. |
+| _required_ | _required_ | **`snapshot-id`** | `long` | ID of the Iceberg table's snapshot the statistics file is associated with. |

Review Comment:
   This is clearly a mistake, still we need a formal discussion and vote for the spec change. Could one of you (@Smith-Cruise or @ajantha-bhat) start the threads please?



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


Re: [PR] [Spec]: Fix wrong type for snapshot-id in table statistics [iceberg]

Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #13513:
URL: https://github.com/apache/iceberg/pull/13513


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


Re: [PR] [Spec]: Fix wrong type for snapshot-id in table statistics [iceberg]

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #13513:
URL: https://github.com/apache/iceberg/pull/13513#discussion_r2212343376


##########
format/spec.md:
##########
@@ -970,7 +970,7 @@ Statistics files metadata within `statistics` table metadata field is a struct w
 
 | v1 | v2 | Field name | Type | Description |
 |----|----|------------|------|-------------|
-| _required_ | _required_ | **`snapshot-id`** | `string` | ID of the Iceberg table's snapshot the statistics file is associated with. |
+| _required_ | _required_ | **`snapshot-id`** | `long` | ID of the Iceberg table's snapshot the statistics file is associated with. |

Review Comment:
   @Smith-Cruise: Can you please start the mailing list discussion (since you found this issue) about further steps? If you doesn't have time for this, I can help.  



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


Re: [PR] [Spec]: Fix wrong type for snapshot-id in table statistics [iceberg]

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #13513:
URL: https://github.com/apache/iceberg/pull/13513#issuecomment-3138377611

   @pvary: Please help in merging this. The voting has passed on the mailing list.  


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


Re: [PR] [Docs] Fix snapshot-id wrong type in Spec [iceberg]

Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #13513:
URL: https://github.com/apache/iceberg/pull/13513#issuecomment-3057466447

   PR title can be
   
   Spec: Fix wrong type for snapshot-id in table statistics 


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