alkis commented on code in PR #603:
URL: https://github.com/apache/parquet-format/pull/603#discussion_r3844616084
##########
LogicalTypes.md:
##########
@@ -727,51 +724,60 @@ 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,
Review Comment:
Yes, new — it came in during review (f4b79fe, 5facb7f) after @wgtmac asked
whether `inline` and `uri` may both be set, which the merged text left
underspecified. Agreed the semantic was more than the spec needs, and dropped
in 17aa46b. The paragraph is now:
> 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; `inline` requires no external
access and is the cheaper path.
So it states what a reader may do and nothing about what the two
representations must contain. I kept that much because a reader that projects
`uri`/`offset`/`size` without `inline` cannot tell whether `inline` was set, so
it needs to know reading the locator is a legitimate path.
##########
LogicalTypes.md:
##########
@@ -727,51 +724,60 @@ 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
|
+
+† The locator fields may all be unset. Otherwise, fields set alongside
`inline` must
+form a locator valid on its own, so `offset` requires `uri` and `size`.
`size` must be set whenever `offset` is set, so any offset-based read always
carries an
-explicit `size`. A self-reference (`uri` not set) must set `offset`, and
therefore also
-`size`. `size` may be omitted only for a whole-file external reference, where
the range
-runs to the end of the referenced file.
+explicit `size`. `size` may be omitted only for a whole-file external
reference, where
+the range runs to the end of the referenced file. `offset` and `size` apply
only to data
+referenced by `uri`; there is no form that addresses a byte range in the
current file
+directly.
-A self-reference points within the same Parquet file using `offset` and `size`
(both
-required). A self-reference is when `uri` is not set. A file containing
self-references
-can be renamed or relocated as a single unit.
+A `uri` is always resolved as an external reference, even when it names the
file that
+contains it. Parquet applies no compression or encryption of its own to the
referenced
+bytes, and a reference remains the writer's responsibility if the file is
copied or
+renamed.
-Parquet files containing self-references must not use Parquet modular
encryption.
-Self-referenced byte ranges are not Parquet encryption modules and therefore
cannot
-be encrypted or authenticated independently. Encryption of external files
referenced
-by `uri` is outside the scope of the Parquet format.
+Encryption of external files referenced by `uri` is outside the scope of the
Parquet
+format. The fields of a `FILE`-annotated group are ordinary columns and are
encoded,
+compressed, and encrypted like any other column, `inline` included.
#### Validation
-* A value must resolve to some referenced data. It resolves only if `inline`,
`uri`, or
- `offset` is set; if none of them are set, the value does not resolve and is
invalid, even
- if `size` is set.
-* A self-reference (`uri` not set) must set `offset`. A value with neither
`uri` nor
- `offset` set (and not `inline`) does not resolve and is invalid.
+* A value must resolve to some referenced data. It resolves only if `inline`
or `uri` is
+ set; if neither is set, the value does not resolve and is invalid, even if
`offset` or
+ `size` is set.
+* `offset` may only be set together with `uri`. A value that sets `offset`
without `uri`
+ does not resolve and is invalid.
* `size` must be set whenever `offset` is set. A value that sets `offset`
without `size`
- is invalid. Because a self-reference must set `offset`, it must also set
`size`.
-* If `inline` is set, it supplies the bytes for readers; producers may treat
`inline` and the
- locator fields as mutually exclusive.
+ is invalid.
+* If `inline` and a locator are both set, they must denote the same bytes, and
a reader
Review Comment:
Agreed, and this is the same argument I made against banning `uri` from
naming the current file earlier in this PR — a rule nothing can check does not
belong in the spec. Nothing verifies that the bytes behind a `uri` match
`inline` without fetching and comparing them. Taking your split in 17aa46b: the
must/invalid language is gone, the permission stays.
> 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.
Worth being explicit about the consequence: two conforming readers of the
same file can now return different bytes for one value and neither is wrong.
That is the honest outcome of not enforcing it, and the previous wording only
pretended otherwise since it also said readers need not check. Happy to cut the
"Producers are expected to" sentence too if you would rather the bullet carry
no writer guidance at all.
##########
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:
Updating my earlier answer to you here, since it no longer holds. Per
@RussellSpitzer above, the same-bytes requirement is gone in 17aa46b because
nothing can verify it.
So the answer to "would inline be a preview slice" is no longer "invalid"
but "unspecified, and a reader may resolve from either, so do not rely on it" —
a reader that happens to read `inline` would get your slice while one that
reads the `uri` gets the full object. Same practical advice, weaker footing.
Your original point stands: if a prefix-for-filtering pattern is wanted, it
should be its own field with explicit semantics rather than a reinterpretation
of `inline`.
--
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]
