[GitHub] [arrow-julia] baumgold commented on a diff in pull request #446: Return SubArrays when possible for arrow list types

["baumgold (via GitHub)" <gi...@apache.org>]

baumgold commented on code in PR #446:
URL: https://github.com/apache/arrow-julia/pull/446#discussion_r1213881772


##########
src/append.jl:
##########
@@ -197,7 +197,13 @@ end
 function is_equivalent_schema(sch1::Tables.Schema, sch2::Tables.Schema)
     (sch1.names == sch2.names) || (return false)
     for (t1,t2) in zip(sch1.types, sch2.types)
-        (t1 === t2) || (return false)
+        tt1 = Base.nonmissingtype(t1)
+        tt2 = Base.nonmissingtype(t2)
+        if t1 == t2 || (tt1 <: AbstractVector && tt2 <: AbstractVector && eltype(tt1) == eltype(tt2))

Review Comment:
   This looks good to me.  Small nit-pick... small simplification without the continue:
   
   ```julia
   t1 != t2 && (tt1 <: AbstractVector && tt2 <: AbstractVector && eltype(tt1) != eltype(tt2)) && return false
   ```



##########
src/table.jl:
##########
@@ -637,16 +639,23 @@ function build(f::Meta.Field, L::ListTypes, batch, rb, de, nodeidx, bufferidx, c
     bufferidx += 1
     len = rb.nodes[nodeidx].length
     nodeidx += 1
+    meta = buildmetadata(f.custom_metadata)
     if L isa Meta.Utf8 || L isa Meta.LargeUtf8 || L isa Meta.Binary || L isa Meta.LargeBinary
         buffer = rb.buffers[bufferidx]
         bytes, A = reinterp(UInt8, batch, buffer, rb.compression)
         bufferidx += 1
+        T = juliaeltype(f, meta, convert)

Review Comment:
   This can move out of the if/else statement to reduce duplication since it's used in both the if and the else.



-- 
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