alkis commented on code in PR #603:
URL: https://github.com/apache/parquet-format/pull/603#discussion_r3814403676
##########
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
|
+| - | 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
|
`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. A byte range within the
current file
+cannot be referenced: `offset` and `size` apply only to data referenced by
`uri`.
Review Comment:
Good point, and I agree the previous wording created exactly the grey area
you describe. Fixed in f4b79fe.
I did not take either of your two options though, because I think a ban on
`uri` naming the current file is not checkable: relative URIs, copies, renames,
and object-store aliases all defeat it, and a file that was legal when written
becomes illegal when someone copies it. Neither the writer nor the reader can
verify the rule, so it would be a grey area of a different kind.
Instead the external path is now uniform, which is what @emkornfield was
getting at:
> `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 `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.
So the `uri` form is specified rather than left ambiguous, and it is
specified as the same thing in all cases: no inheritance, no Parquet-applied
transforms, no special handling. A writer pointing a `uri` at its own file gets
external-reference semantics and owns the consequences. That is not a
self-reference back door (@wgtmac) because nothing about it is privileged.
I also chose not to retain the `uri`-unset form. Keeping it would mean
re-specifying the compression and encryption ramifications, which is what this
PR is removing, and the mid-sized-value use case it existed for is better
served by non-contiguous pages in the page layer. More on that in a follow-up
proposal.
--
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]
