alkis commented on code in PR #603:
URL: https://github.com/apache/parquet-format/pull/603#discussion_r3826219784
##########
LogicalTypes.md:
##########
@@ -727,51 +724,61 @@ object-store eTag for the whole file referenced by `uri`.
##### 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 are
provenance
-only.
+bytes and any locator fields (`uri`, `offset`, `size`) that are set are
provenance only.
+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. A locator set alongside `inline` records where those bytes
came from,
+and must not be a partial or otherwise different representation of the value.
#### Resolution
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 (same as any
locator) |
+| - | 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 | invalid
|
+| - | - | - | - | nothing - invalid
|
+
+† Any combination of the locator fields that is valid on its own. A locator set
+alongside `inline` must satisfy the same rules as one used on its own, so
`offset`
+requires `uri` and `size`.
Review Comment:
You are right, and that reading would have made the most common case — a
group that defines only `inline` — invalid. Applied your suggestion verbatim in
c2a8c6f.
For the record on the row below it: `nothing - invalid` still applies only
when `inline` is unset, since that row has `-` in the `inline` column, so there
is no conflict with the footnote.
Also a heads up that I reverted the HTML table I briefly pushed (9220368,
reverted in d7a7137) — the markdown table with this footnote is back, so the
version you are reviewing is current.
--
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]
