You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2026/06/04 17:59:58 UTC

[PR] Minor: improve PageStore docs with a temp-file spilling example [arrow-rs]

alamb opened a new pull request, #10074:
URL: https://github.com/apache/arrow-rs/pull/10074

   # Which issue does this PR close?
   
   Follow-on to #10020, implementing the documentation suggestion from https://github.com/apache/arrow-rs/pull/10020#discussion_r3350282205. There is no separate tracking issue.
   
   # Rationale for this change
   
   The original `with_page_store_factory` example used a `HashMap`-backed store with deliberately sparse keys to demonstrate that the writer treats `PageKey`s as opaque — useful trivia, but it made the example longer and harder to follow. A temp-file-backed store is both simpler to read and much closer to what users would actually build, since spilling pages off the heap is the whole point of the API.
   
   # What changes are included in this PR?
   
   Replaces the doctest with a `TempFilePageStore` (one temp file per column chunk: `put` appends the page, `take` seeks it back) and reflows the surrounding prose. Also tidies the `PageStore` trait docs to link out to the example. This is documentation only — no code or behavior changes.
   
   # Are these changes tested?
   
   Yes — the example is a runnable doctest that writes a record batch through the spilling store and asserts the round-tripped data matches.
   
   # Are there any user-facing changes?
   
   Documentation only; no public API or behavior changes.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Minor: improve PageStore docs with a temp-file spilling example [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10074:
URL: https://github.com/apache/arrow-rs/pull/10074#issuecomment-4653153296

   > Thanks @alamb !
   > 
   > Do you think it'd be better to export this as public code instead of a docstring? Then the docstring could show using the code and the code would be a good example that we can add tests to, etc.
   
   I am torn about this -- the example implementation is really quite bad (does many small I/Os, no buffering or prefetching, etc) so if we included it with arrow-rs I think people would have a crappy experience. WIth an example at least the crappy experience would be their own code (that they copy pasted...)
   
   But now that I write that it seems a poor justification :think
   
   
   
   > The only other note I'd add for the example is to write to a file instead of an in-memory buffer. The destination being memory makes the argument for spilling buffers to disk weaker IMO.
   
   
   I don't quite follow this question. The example does write to a file 
   
   ```rut
       ///     file: File,
   ``` 
   🤔 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Minor: improve PageStore docs with a temp-file spilling example [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10074:
URL: https://github.com/apache/arrow-rs/pull/10074#issuecomment-4624837012

   FYI @adriangb 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Minor: improve PageStore docs with a temp-file spilling example [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10074:
URL: https://github.com/apache/arrow-rs/pull/10074#issuecomment-4663834677

   > I agree if a correct implementation using files is complex (either because of APIs, buffering, etc.) then providing a bad one is not helpful. But if it were easy to make a correct implementation maybe providing it is better than providing an incomplete example?
   
   I don't think there is any "incorrect" or "incomplete" about this example -- it is more like it takes one end of a tradeoff extreme (minimal buffering) and I can imagine many other points along the tradeoff space that could be interesting
   
   I would personally prefer to wait for some more specific examples of people using this API before we committed to adding another implementation in arrow-rs. 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Minor: improve PageStore docs with a temp-file spilling example [arrow-rs]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #10074:
URL: https://github.com/apache/arrow-rs/pull/10074#issuecomment-4652078829

   (btw I can't approve this, i'm not a committer)


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Minor: improve PageStore docs with a temp-file spilling example [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10074:
URL: https://github.com/apache/arrow-rs/pull/10074#issuecomment-4792573410

   Thank you @Jefffrey and @adriangb 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Minor: improve PageStore docs with a temp-file spilling example [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #10074:
URL: https://github.com/apache/arrow-rs/pull/10074


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Minor: improve PageStore docs with a temp-file spilling example [arrow-rs]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #10074:
URL: https://github.com/apache/arrow-rs/pull/10074#issuecomment-4653640412

   I am referring to this part:
   
   ```rust
       /// let mut writer =
       ///     ArrowWriter::try_new_with_options(&mut buffer, to_write.schema(), options).unwrap();
   ```
   
   (the actual bytes are still written to memory)


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Minor: improve PageStore docs with a temp-file spilling example [arrow-rs]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #10074:
URL: https://github.com/apache/arrow-rs/pull/10074#issuecomment-4653650159

   I agree if a correct implementation using files is complex (either because of APIs, buffering, etc.) then providing a bad one is not helpful. But if it were easy to make a correct implementation maybe providing it is better than providing an incomplete example?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Minor: improve PageStore docs with a temp-file spilling example [arrow-rs]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #10074:
URL: https://github.com/apache/arrow-rs/pull/10074#issuecomment-4663881390

   Sounds good to me 😄 


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org