Fix #590 view-backed variadic buffer inference

    Centralize the view inline/external layout boundary, infer missing external buffers from valid view elements, and ignore null slots during buffer-count recovery for Utf8View/BinaryView readers.

Author: guangtao

README.md

@@ -64,6 +64,7 @@ This implementation supports the 1.0 version of the specification, including sup
   * All nested data types
   * Dictionary encodings and messages
   * Extension types
+  * View-backed Utf8/Binary columns, including recovery from under-reported variadic buffer counts by inferring the required external buffers from valid view elements
   * Streaming, file, record batch, and replacement and isdelta dictionary messages
 
 It currently doesn't include support for:

src/arraytypes/views.jl

@@ -21,6 +21,17 @@ struct ViewElement
     offset::Int32
 end
 
+const VIEW_ELEMENT_BYTES = sizeof(ViewElement)
+const VIEW_LENGTH_BYTES = sizeof(Int32)
+const VIEW_INLINE_BYTES = VIEW_ELEMENT_BYTES - VIEW_LENGTH_BYTES
+
+@inline _viewisinline(length::Integer) = length <= VIEW_INLINE_BYTES
+@inline _viewinlinestart(i::Integer) =
+    ((i - 1) * VIEW_ELEMENT_BYTES) + VIEW_LENGTH_BYTES + 1
+@inline _viewinlineend(i::Integer, length::Integer) = _viewinlinestart(i) + length - 1
+@inline _viewinlineslice(inline::Vector{UInt8}, i::Integer, length::Integer) =
+    @view inline[_viewinlinestart(i):_viewinlineend(i, length)]
+
 """
     Arrow.View
 
@@ -45,12 +56,8 @@ Base.size(l::View) = (l.ℓ,)
     if S <: Base.CodeUnits
         # BinaryView
         return !l.validity[i] ? missing :
-               v.length < 13 ?
-               Base.CodeUnits(
-            StringView(
-                @view l.inline[(((i - 1) * 16) + 5):(((i - 1) * 16) + 5 + v.length - 1)]
-            ),
-        ) :
+               _viewisinline(v.length) ?
+               Base.CodeUnits(StringView(_viewinlineslice(l.inline, i, v.length))) :
                Base.CodeUnits(
             StringView(
                 @view l.buffers[v.bufindex + 1][(v.offset + 1):(v.offset + v.length)]
@@ -59,12 +66,10 @@ Base.size(l::View) = (l.ℓ,)
     else
         # Utf8View
         return !l.validity[i] ? missing :
-               v.length < 13 ?
+               _viewisinline(v.length) ?
                ArrowTypes.fromarrow(
             T,
-            StringView(
-                @view l.inline[(((i - 1) * 16) + 5):(((i - 1) * 16) + 5 + v.length - 1)]
-            ),
+            StringView(_viewinlineslice(l.inline, i, v.length)),
         ) :
                ArrowTypes.fromarrow(
             T,

src/table.jl

@@ -732,6 +732,18 @@ const ListTypes =
 const LargeLists = Union{Meta.LargeUtf8,Meta.LargeBinary,Meta.LargeList,Meta.LargeListView}
 const ViewTypes = Union{Meta.Utf8View,Meta.BinaryView,Meta.ListView,Meta.LargeListView}
 
+@inline function _viewbuffercount(validity, views, declared::Integer)
+    count = Int(declared)
+    for i in eachindex(views)
+        validity[i] || continue
+        v = @inbounds views[i]
+        if !_viewisinline(v.length)
+            count = max(count, Int(v.bufindex) + 1)
+        end
+    end
+    return count
+end
+
 function build(field::Meta.Field, batch, rb, de, nodeidx, bufferidx, varbufferidx, convert)
     d = field.dictionary
     if d !== nothing
@@ -910,7 +922,8 @@ function build(
     inline = reinterpret(UInt8, views)  # reuse the (possibly realigned) memory backing `views`
     bufferidx += 1
     buffers = Vector{UInt8}[]
-    for i = 1:rb.variadicBufferCounts[varbufferidx]
+    nvariadic = _viewbuffercount(validity, views, rb.variadicBufferCounts[varbufferidx])
+    for i = 1:nvariadic
         buffer = rb.buffers[bufferidx]
         _, A = reinterp(UInt8, batch, buffer, rb.compression)
         push!(buffers, A)

test/runtests.jl

@@ -264,6 +264,30 @@ end
             @test all(isequal.(values(t), values(tt)))
         end
 
+        @testset "View buffer count inference" begin
+            inline_len = Int32(Arrow.VIEW_INLINE_BYTES)
+            views = Arrow.ViewElement[
+                Arrow.ViewElement(inline_len, Int32(0), Int32(0), Int32(0)),
+                Arrow.ViewElement(inline_len + Int32(148), Int32(0), Int32(0), Int32(0)),
+                Arrow.ViewElement(inline_len + Int32(207), Int32(0), Int32(1), Int32(160)),
+            ]
+            validity = Arrow.ValidityBitmap(UInt8[], 1, length(views), 0)
+            @test Arrow._viewisinline(inline_len)
+            @test !Arrow._viewisinline(inline_len + Int32(1))
+            @test Arrow._viewbuffercount(validity, views, Int32(0)) == 2
+            @test Arrow._viewbuffercount(validity, views, Int32(1)) == 2
+            @test Arrow._viewbuffercount(validity, views, Int32(3)) == 3
+
+            sparse_validity = Arrow.ValidityBitmap(UInt8[0x05], 1, 3, 1)
+            sparse_views = Arrow.ViewElement[
+                Arrow.ViewElement(inline_len + Int32(64), Int32(0), Int32(0), Int32(0)),
+                Arrow.ViewElement(inline_len + Int32(64), Int32(0), Int32(99), Int32(0)),
+                Arrow.ViewElement(inline_len, Int32(0), Int32(0), Int32(0)),
+            ]
+            @test !sparse_validity[2]
+            @test Arrow._viewbuffercount(sparse_validity, sparse_views, Int32(0)) == 1
+        end
+
         @testset "single-partition tobuffer byte equivalence" begin
             t = (col=OffsetArray(["a", "bc", "def"], 0:2),)
             io = IOBuffer()