rok commented on code in PR #635: URL: https://github.com/apache/parquet-format/pull/635#discussion_r4456827876
########## LogicalTypes.md: ########## @@ -743,14 +745,15 @@ are: equality comparison and not otherwise interpreted. `checksum` applies to the resolved bytes, except for `ETAG`, which is the -object-store eTag for the whole file referenced by `uri`. +object-store eTag for the whole file referenced by `uri`. If `inline` and `uri` are both +set, `checksum` describes the source at `uri` as it was when `inline` was copied from it. +Like the locator, it is then provenance only. ##### inline The referenced bytes stored inline in the value. If `inline` is set, it supplies the -bytes and any locator fields (`uri`, `offset`, `size`) that are set record where those -bytes came from. A reader may resolve the value from `inline` or from the locator, -whichever suits it. +bytes and any locator fields (`uri`, `offset`, `size`) that are set are provenance +only. Review Comment: How do you define "provenance only"? Does that mean data at `uri` will never be read? ########## LogicalTypes.md: ########## @@ -796,10 +800,8 @@ compressed, and encrypted like any other column, `inline` included. does not resolve and is invalid. * `size` must be set whenever `offset` is set. A value that sets `offset` without `size` is invalid. -* If `inline` and a locator are both set, a reader may resolve the value from either. - Producers are expected to write the same bytes in both, but a reader is not required - to check this and may return the bytes of either. Producers may treat `inline` and the - locator fields as mutually exclusive. +* If `inline` is set, it supplies the bytes for readers; producers may treat `inline` and + the locator fields as mutually exclusive. Review Comment: This makes file type mutable where it previously wasn't. @alkis you might have an opinion on this. ########## LogicalTypes.md: ########## @@ -712,8 +713,9 @@ If set and non-zero, readers must seek to this offset to retrieve the referenced The byte length of the referenced data. Must be zero or a positive integer if set; a value of 0 indicates empty referenced data. `size` must be set whenever `offset` is set. -It may be omitted only for a whole-file external reference (`uri` set, `offset` not set), -in which case the range runs to the end of the referenced file. +It may be omitted for a whole-file external reference (`uri` set, `offset` not set), +in which case the range runs to the end of the referenced file. `size` may also be set +with `inline` and without `uri`, in which case it must equal the length of `inline`. Review Comment: Pruning and stats indeed make sense. Could we peek into BYTE_ARRAY prefixed with length instead of adding `length`? What bothers me is that we'd be writing it twice - once as BYTE_ARRAY prefix and once as a `length` field. -- 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]
