This is an automated email from the ASF dual-hosted git repository. quinnj pushed a commit to branch preservation/storage-20261008/julia-dev-Arrow-b25a15d2 in repository https://gitbox.apache.org/repos/asf/arrow-julia.git
commit caec5ec11bece2344584948d7ebc9ebb27db2287 Author: Jacob Quinn <[email protected]> AuthorDate: Thu Oct 8 22:53:52 2026 -0600 chore: preserve local work before storage cleanup [skip ci] Preservation snapshot of unfinished local work. This commit saves existing changes; it does not assert functional validation. AI disclosure: This work was prepared with assistance from OpenAI Codex. --- Arrow_Review.md | 72 +++++++++++++++++++++++++++++++++++ ISSUE-420.md | 64 +++++++++++++++++++++++++++++++ ISSUE-474.md | 85 +++++++++++++++++++++++++++++++++++++++++ ISSUE-540.md | 41 ++++++++++++++++++++ ISSUE-580.md | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 377 insertions(+) diff --git a/Arrow_Review.md b/Arrow_Review.md new file mode 100644 index 0000000..400366f --- /dev/null +++ b/Arrow_Review.md @@ -0,0 +1,72 @@ +# Arrow Review + +1. Question +have we done an in-depth review/analysis of the vendored FlatBuffers code? it was originally ported from the Go implementation. we should do a pass to identify bugs, perf issues, and functionality gaps. it probably wouldn't take much to get it up to a state where we could actually upstream to the FlatBuffers.jl package. we should also compare w/ the actual current FlatBuffers.jl implementation and figure out the current state of everything and make a plan forward (kinda dumb to keep vend [...] + +2. Question +"Canonical padding and unused-bit checks remain production work." uh....what does that mean? is there work to do for this in the core module? + +3. Follow up +""" +# What this deliberately gives up, so the constraint is informed: +# * Eager, deterministic unmap (e.g. delete-a-mapped-file-now on Windows): +# unmapping happens when the GC collects the mapping. Revisit if real +# demand appears, likely as an opt-in layer once upstream offers a +# public API. +""" +I actually think we can still support this via explicit API. i.e I think it would be reasonable to provide an `Arrow.close(::Arrow.Table)` like API, or maybe call it `Arrow.close!` to semantically distinguish from the traditional Base IO-related `close`. The idea is it would do a force munmap on underlying array storage (i.e. calling `finalize(buf)`) so we could still deterministically say a file was done being used for windows. We could carry a simple `@atomic closed::Bool` field on Own [...] + +4. Clean up +We have a ton of code comments + docs that mention the rewrite process, decisions made/not made/deferred, how stuff *used* to be, etc. We need to clean all that up. Code comments + docs should always reflect the current state of things, nothing more/less. For historical context, we can look at git history and maybe I'll do a blog post write up or give a JuliaCon talk on the whys, how, where we came from. + +5. Follow up +I think we could rework the _value and _value_of functions to allow passing a desired element type down and basically asserting that we get that back out. For the most part, it maps already to teh ArrowType for primitives (i.e IntType & Int64). But I think it will be a key piece of actually allowing a useable trim-compile workload; i.e. I can provide you with a static schema for the columns (names & types) I know i'm operating on: arrow materializes using the core C-data structures, then [...] + +6. Clean up +""" +_malloc!(root::ExportedRoot, n::Integer, + register! = push!, deallocate! = Libc.free) = begin +""" +Big no-no; we don't want to ever use the short-form method definition form (w/ =) with a begin-end block. + +7. Question +There have been a couple C-data attempts in Arrow.jl over the years; none were really that serious from waht I remember, but +just recently some kind put up a few PRs. I haven't had a chance to look at them, but I feel a little bad if he put a ton of time into them just have me throw everything away. We should compare what he has and at least see if there are parts we can incorporate or overall APIs we should use. + +8. Follow up +We should make sure the C-data entrypoints are explicitly trim-compile-able (I mean, they're C, so they *should be*). I can see that being a killer use-case: I'm going to trim-compile this super fast binary that gets data into Julia, does some munging/calculating, then spits it back out somewhere using C-data at the seams. + +9. Follow up +Let's remove all the demo stuff at the end of the cdata.jl file + +10. Follow up +"experimental V4 IPC compression is outside this prove-out" what ist he experimental compression? how hard to support? or is it actually old and we *don't* want to support it? + +11. Clean up +get rid of _threaded_cursor_stress in ipc_read.jl + +12. Follow up +I do really like the design of ArrowCore with C-data & IPC as layers on top. we're doing *a lot* more work though, *it seems*, for serialize/deserialize. I know it's all justified because we want to do all the validation/verification stuff and we're a lot safer now, but I do worry about the performance cost of it all. At the end of the day, arrow data is supposed to be *fast*. Can we do some benchmarking on arrow rewrite vs. Arrow.jl 2.X and then pick, what, pyarrow as probably the indus [...] + +13. Question +Why do we need _resolvefilter in scan.jl? I thought Tables.bind basically handled that? + +14. Question +in scan.jl, we have `return table, Tables.Scan(residualselect, residualfilter, limit, offset, scan.validate)`; +what are the cases where we have residuals of any of these (select, filter, limit, offset, etc.)? it feels like we should +be able to handle all of them? and never have residuals? + +15. Question +in scan.jl, we have statistics data/batch/meta stuff? what's that all about? is it in the actual arrow spec? but at least one of our comments mentioned having our own stats spec or something? + +16. Follow up +_statcmp and _stateq in scan.jl need to be full function defs, or actually one-line method defs. + +17. Follow up +Ok, finished reviewing all the Arrow rewrite; it's in pretty good shape! I didn't realize we hadn't done the facade layer yet though. So that's the next request: let's build out the Arrow.Table, Arrow.Stream, Arrow.write, etc. layers on top of what we have in ipc_read.jl/ipc_write.jl. We should make sure to allow to pass a Tables.Scan to Arrow.Table and that will utilize the new scan pushdown. We also need to figure out a good interface for getting the RangedFile stuff exposed; brainstor [...] + +18. Follow up +In parallel, we've been working on rewrites for both Arrow & CSV. One of the pieces in CSV is we have a new CompactString type that actually should match the ArrowCore string representation. I want to make sure we can "automatically" get zero-copy Arrow.Table when reading from CSV and have it all "just work". + +19. Follow up +How is our overall round-tripping testing vs. other frameworks? extensive? breadth & depth? stream/file/c-data/ipc/all the things? I want those cross-impl tests to be the meat of our testing. yes we can still have some unit tests on other things, and a few facade/interface tests, but the meat should be exhaustive round-tripping with like pyarrow and nanoarrow. \ No newline at end of file diff --git a/ISSUE-420.md b/ISSUE-420.md new file mode 100644 index 0000000..7755826 --- /dev/null +++ b/ISSUE-420.md @@ -0,0 +1,64 @@ +# Issue #420 Triage Report + +## Summary +Test failure when running with >1 thread. Race condition causes `MethodError: no method matching resize!(::Arrow.DictEncoded{...}, ::Int64)`. + +## Issue Type +Bug (Threading/Race Condition) + +## Root Cause Analysis + +The issue is related to dictionary encoding operations when reading Arrow data with multiple threads. The error occurs when: +1. Multiple partitions with dict-encoded columns are read +2. Threads try to append/combine dict-encoded arrays +3. Race conditions cause invalid operations on DictEncoded arrays + +## Related Fixes + +### Fix #535 (fc8b899) - Make DictEncoding reading threadsafe +Merged: Nov 26, 2024 + +This commit made DictEncoding reading threadsafe by adding proper locking around dictionary encoding operations. This likely addresses issue #420. + +### Fix #582 (b8ebfc5) - Avoid race condition in multithreaded writes +Merged: Jan 12, 2026 + +This commit disabled multithreaded WRITING as a stopgap to fix a race condition in dictionary encoding. The commit message notes: + +> "There is a race condition fundamental to the current architecture for creating and writing dictionary encodings. The relevant lock is created on a worker thread and thus there is a race to _create_ the lock and initialize the relevant data structure." + +## Current Status + +The issue likely has two components: +1. **Reading** (constructing Arrow.Table) - Fixed by #535 +2. **Writing** (Arrow.write with partitions) - Mitigated by #582 (multithreaded writing disabled) + +## Verification Needed + +Test if the original reproduction still fails: + +```julia +using Tables, Arrow, PooledArrays + +for _ in 1:10000 + t = Tables.partitioner( + ( + (a=Arrow.toarrowvector(PooledArray([1,2,3 ])),), + (a=Arrow.toarrowvector(PooledArray([1,2,3,4])),), + (a=Arrow.toarrowvector(PooledArray([1,2,3,4,5])),), + ) + ) + tt = Arrow.Table(Arrow.tobuffer(t)) +end +``` + +## Resolution + +- [x] Reading side fixed by #535 (DictEncoding reading threadsafe) +- [x] Writing side mitigated by #582 (multithreaded writing disabled) +- [ ] Verify original reproduction no longer fails +- [ ] Close issue if fixed + +## Recommendation + +Run the reproduction test with the latest code. If it passes, close issue #420 referencing the fixes in #535 and #582. diff --git a/ISSUE-474.md b/ISSUE-474.md new file mode 100644 index 0000000..8ad38f0 --- /dev/null +++ b/ISSUE-474.md @@ -0,0 +1,85 @@ +# Issue #474 Triage Report + +## Summary +Bus errors occur when writing DataFrames to arrow files. + +## Issue Type +Bug (partial user error, but could be better handled) + +## Root Cause Analysis + +### Confirmed Case (stuartthomas25's reproduction) +When reading an arrow file with `Arrow.Table(filename)`, the file is memory-mapped via `Mmap.mmap()`. The returned `Arrow.Table` holds references to this mmap'd memory. + +If you then call `Arrow.write(filename, table)` on the SAME file: +1. Opening for write truncates the file +2. The mmap'd memory becomes invalid +3. Reading from the table (to write) causes a bus error (SIGBUS) + +**Reproduction:** +```julia +using Arrow + +# Write initial data +Arrow.write("test.arrow", (a=[1,2,3],)) + +# Read it back (this mmaps the file) +t = Arrow.Table("test.arrow") + +# Try to overwrite - CRASHES with bus error +Arrow.write("test.arrow", t) +``` + +### simsurace's Original Case (intermittent) +The original reporter mentions writing to DIFFERENT file names but still getting intermittent errors. This could be: +1. A different bug related to memory management +2. Threading issues (note the stack trace shows threading) +3. GC collecting mmap'd memory incorrectly + +This case needs more investigation to reproduce. + +## Potential Fixes + +### Option 1: Documentation +Document clearly that you cannot write an Arrow.Table back to its source file without first copying the data. + +### Option 2: Runtime Check +Before writing, check if any source arrays reference mmap'd memory pointing to the target file: + +```julia +function write(file_path::String, tbl; kwargs...) + # Check if tbl contains references to this file + if has_mmap_reference_to_file(tbl, file_path) + error("Cannot write to '$file_path': the source table contains data mmap'd from this file. Use Arrow.write(file_path, Arrow.Table(copy(tbl))) instead.") + end + # ... rest of write +end +``` + +### Option 3: Automatic Copy +Add a `copy=false` keyword argument. When `true`, copy all data before writing: + +```julia +Arrow.write("test.arrow", t; copy=true) # Safe overwrite +``` + +## Recommendation + +1. **Short-term**: Add documentation warning about this behavior +2. **Medium-term**: Implement Option 2 (runtime check with helpful error message) +3. **Long-term**: Consider Option 3 for convenience + +## Resolution Status +- [ ] Document mmap behavior and write-to-same-file limitation +- [ ] Consider adding runtime detection/warning +- [ ] Investigate simsurace's intermittent case separately (may be different bug) + +## Additional Notes +The workaround mentioned by @sprig (write to temp file, then rename) is reasonable but has atomicity concerns. The safest workaround is: + +```julia +t = Arrow.Table("data.arrow") +# Copy all columns to regular Julia arrays +copied = (;(k => copy(v) for (k, v) in pairs(Tables.columntable(t)))...) +Arrow.write("data.arrow", copied) +``` diff --git a/ISSUE-540.md b/ISSUE-540.md new file mode 100644 index 0000000..5d662f2 --- /dev/null +++ b/ISSUE-540.md @@ -0,0 +1,41 @@ +# Issue #540 Triage Report + +## Summary +Files written by `polars.DataFrame.write_ipc` with `string_view` type are read incorrectly by Arrow.jl. + +## Issue Type +Bug (FIXED) + +## Root Cause +Commit `de59e48` (Fix bug where inline bytes weren't valid for Utf8View data) fixed this issue. + +The bug was in `src/table.jl` in the `build` function for ViewTypes: + +**Before (buggy):** +```julia +inline, views = reinterp(ViewElement, batch, buffer, rb.compression) +``` + +**After (fixed):** +```julia +_, views = reinterp(ViewElement, batch, buffer, rb.compression) +inline = reinterpret(UInt8, views) # reuse the (possibly realigned) memory backing `views` +``` + +The `reinterp` function returns `(bytes, array)` where: +- `bytes`: the raw byte buffer (original or decompressed) +- `array`: the typed array (ViewElement in this case) + +The bug assigned `bytes` to `inline`, which was wrong. The inline data should be a byte view of the `views` array itself (since inline string data is stored within the ViewElement structs). + +## Resolution +- Fixed in commit `de59e48` (May 9, 2025) +- Included in releases: v2.8.1-rc1, v2.8.1-rc2 +- Will be in v2.8.1 final release + +## Recommendation +**Close issue #540** with a note that the fix is included in v2.8.1. + +## Test Coverage +- Test file `test/reject_reason_trimmed.arrow` added with the fix +- Test verifies Utf8View data is read correctly diff --git a/ISSUE-580.md b/ISSUE-580.md new file mode 100644 index 0000000..e136bcb --- /dev/null +++ b/ISSUE-580.md @@ -0,0 +1,115 @@ +# Issue #580 Triage Report + +## Summary +Arrow.Stream does not work with non-seekable I/O (FIFOs, sockets, named pipes). Two separate issues in read and write paths. + +## Issue Type +Bug + +## Reproduction +- Reproduced: Yes (per issue reporter's detailed MWEs) +- Julia version tested: 1.x +- Package version tested: v2.8.0 + +## Root Cause Analysis + +### Read-side Issue +**Location**: `src/table.jl:28` + +```julia +tobytes(io::IOStream) = Mmap.mmap(io) +``` + +For non-seekable I/O like FIFOs: +- `filesize(io)` returns 0 +- `Mmap.mmap(io)` returns an empty `UInt8[]` +- Result: Stream appears empty, no data read + +**Fix**: Check if IO is seekable before attempting mmap, fallback to `Base.read(io)`: + +```julia +function tobytes(io::IOStream) + # Check if the stream is seekable (regular file vs FIFO/socket) + try + pos = position(io) + seek(io, pos) # Verify seeking works + return Mmap.mmap(io) + catch + return Base.read(io) + end +end +``` + +Or use a simpler heuristic based on `filesize`: + +```julia +function tobytes(io::IOStream) + # mmap doesn't work for non-seekable streams (FIFOs, etc.) + # where filesize returns 0 but data is available + fs = filesize(io) + if fs > 0 + return Mmap.mmap(io) + else + return Base.read(io) + end +end +``` + +### Write-side Issue +**Location**: `src/write.jl:569-573` + +```julia +if msg.blockmsg + push!( + blocks[msg.isrecordbatch ? 1 : 2], + Block(position(io), metalen + 8, msg.bodylen), + ) +end +``` + +The `blockmsg` flag is set in `makemessage()` based on message type (RecordBatch/DictionaryBatch), NOT based on whether we're writing file format. Block tracking calls `position(io)`, which fails on non-seekable streams. + +**Location of blockmsg setting**: `src/write.jl:604-609` + +```julia +Message( + ... + headerType == Meta.RecordBatch || headerType == Meta.DictionaryBatch, # blockmsg - ALWAYS true for these types + ... +) +``` + +**Fix Options**: + +1. **Option A**: Pass `writetofile` flag to message creation and only set `blockmsg=true` when writing file format + +2. **Option B**: Modify `Base.write(io::IO, msg::Message, ...)` to accept `writetofile` parameter and skip block tracking + +3. **Option C**: Check if IO is seekable before calling `position(io)`: + +```julia +if msg.blockmsg && applicable(position, io) + try + pos = position(io) + push!(blocks[msg.isrecordbatch ? 1 : 2], Block(pos, metalen + 8, msg.bodylen)) + catch + # Non-seekable stream, skip block tracking (only valid for streaming format) + end +end +``` + +## Recommended Fix + +The cleanest fix is **Option A** - explicitly track whether we need block positions: + +1. Add `trackblocks::Bool` parameter to `makemessage()` or the `Message` struct +2. Set it to `writetofile` value when creating messages in Writer +3. Only record block positions when `trackblocks && msg.blockmsg` + +## Resolution +- [ ] Fix `tobytes(io::IOStream)` to handle non-seekable I/O in `src/table.jl:28` +- [ ] Fix block tracking to not call `position(io)` for streaming format in `src/write.jl` +- [ ] Add tests with non-seekable I/O (can use `IOBuffer` or mock streams) + +## Additional Notes +The issue reporter indicated they will submit a PR. This analysis can guide review of their PR.
