alkis commented on code in PR #603:
URL: https://github.com/apache/parquet-format/pull/603#discussion_r3814768246


##########
LogicalTypes.md:
##########
@@ -735,41 +732,35 @@ only.
 A value resolves to bytes based on which of `inline`, `uri`, `offset`, and 
`size` are
 set:
 
-| `inline` | `uri` | `offset` | `size` | Resolves to                           
                |
-|----------|-------|----------|--------|-------------------------------------------------------|
-| set      | -     | -        | -      | the inline bytes                      
                |
-| -        | set   | -        | -      | whole external file at `uri`          
                |
-| -        | set   | set      | -      | invalid                               
                |
-| -        | set   | -        | set    | external `uri`, `[0, size)`           
                |
-| -        | set   | set      | set    | external `uri`, `[offset, offset + 
size)`             |
-| -        | -     | set      | -      | invalid                               
                |
-| -        | -     | -        | set    | invalid                               
                |
-| -        | -     | set      | set    | this file, `[offset, offset + size)` 
(self-reference) |
-| -        | -     | -        | -      | nothing - invalid                     
                |
+| `inline` | `uri` | `offset` | `size` | Resolves to                           
    |
+|----------|-------|----------|--------|-------------------------------------------|
+| set      | -     | -        | -      | the inline bytes                      
    |

Review Comment:
   Following up on this — the previous wording said `inline` "supplies the 
bytes for readers" without saying whether reading the locator instead is a 
legitimate path. It has to be: a reader that projects `uri`/`offset`/`size` and 
not `inline` cannot tell whether `inline` was set, so it would be out of spec 
through no fault of its own. Made explicit in 5facb7f:
   
   > Both representations must denote the same bytes, so a reader may resolve 
the value from either and obtain the same result; reading `inline` requires no 
external access and is the cheaper path.
   
   And the validation bullet now says what happens if a producer breaks the 
rule:
   
   > If `inline` and a locator are both set, they must denote the same bytes, 
and a reader may resolve the value from either. If they disagree the value is 
invalid; a reader is not required to detect this and may return the bytes of 
either representation.
   
   So the file is at fault when they disagree, and readers are not obliged to 
verify.
   
   One asymmetry worth noting: `checksum` with `ETAG` is scoped to the whole 
file referenced by `uri`, not to the resolved bytes, so an `ETAG` alongside 
`inline` describes the external object rather than the inline payload. That is 
the existing exception in the `checksum` section and I have left it as is.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to