jonathanc-n commented on code in PR #1207:
URL: https://github.com/apache/iceberg-rust/pull/1207#discussion_r2076307849
##########
crates/iceberg/src/puffin/metadata.rs:
##########
@@ -264,18 +293,83 @@ impl FileMetadata {
}
/// Returns the file metadata about a Puffin file
- pub(crate) async fn read(input_file: &InputFile) -> Result<FileMetadata> {
+ ///
+ /// `prefetch_hint` is used to try to fetch the entire footer in one read.
If
+ /// the entire footer isn't fetched in one read the function will call the
`read_no_prefetch`
+ /// option.
+ pub(crate) async fn read(&self, input_file: &InputFile) ->
Result<FileMetadata> {
Review Comment:
@liurenjie1024 fixed!
##########
crates/iceberg/src/puffin/metadata.rs:
##########
@@ -144,18 +144,28 @@ impl FileMetadata {
pub(crate) const MAGIC_LENGTH: u8 = 4;
pub(crate) const MAGIC: [u8; FileMetadata::MAGIC_LENGTH as usize] = [0x50,
0x46, 0x41, 0x31];
- // We use the term FOOTER_STRUCT to refer to the fixed-length portion of
the Footer, as illustrated below.
- //
- // Footer
- // |
- // -------------------------------------------------
- // | |
- // Magic FooterPayload FooterPayloadLength Flags Magic
- // | |
- // -----------------------------
- // |
- // FOOTER_STRUCT
-
+ /// We use the term FOOTER_STRUCT to refer to the fixed-length portion of
the Footer.
+ /// The structure of the Footer specification is illustrated below:
+ ///
+ /// ```text
+ /// Footer
+ /// ┌────────────────────┐
+ /// │ Magic (4 bytes) │
+ /// │ │
+ /// ├────────────────────┤
+ /// │ FooterPayload │
+ /// │ (PAYLOAD_LENGTH) │
+ /// ├────────────────────┤ ◀─┐
+ /// │ FooterPayloadSize │ │
Review Comment:
I used `FooterPayloadSize` instead of `FooterPayloadLength` to line up with
specification
##########
crates/iceberg/src/puffin/metadata.rs:
##########
@@ -285,7 +305,67 @@ impl FileMetadata {
let footer_payload_str =
FileMetadata::extract_footer_payload_as_str(&footer_bytes,
footer_payload_length)?;
+
FileMetadata::from_json_str(&footer_payload_str)
+ //
+ }
+
+ #[allow(dead_code)]
+ pub(crate) async fn read_with_prefetch(
+ input_file: &InputFile,
+ prefetch_hint: u8,
+ ) -> Result<FileMetadata> {
+ if prefetch_hint > 16 {
+ let input_file_length = input_file.metadata().await?.size;
+ let file_read = input_file.reader().await?;
+
+ // Hint cannot be larger than input file
+ if prefetch_hint as u64 > input_file_length {
+ return FileMetadata::read(input_file).await;
Review Comment:
Lets push this pull request forward and do this in a separate pull request
(good first issue). Are we going to be using `tracing` in this crate?
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
