Skip to content

Commit

Permalink
Fix type instability in getproperty(::Schema, ::Symbol) (#340)
Browse files Browse the repository at this point in the history
  • Loading branch information
MilesCranmer authored Oct 20, 2023
1 parent 289eb93 commit b3ec016
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/Tables.jl
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,17 @@ function Base.getproperty(sch::Schema{names, types}, field::Symbol) where {names
if field === :names
return names === nothing ? getfield(sch, :storednames) : names
elseif field === :types
T = getfield(sch, :storedtypes)
return types === nothing ? (T !== nothing ? T : nothing) : Tuple(fieldtype(types, i) for i = 1:fieldcount(types))
if types === nothing
return getfield(sch, :storedtypes)
else
ncol = fieldcount(types)
if ncol <= 512
# Type stable, but slower to compile
return ntuple(i -> fieldtype(types, i), Val(ncol))
else
return Tuple(fieldtype(types, i) for i=1:ncol)
end
end
else
throw(ArgumentError("unsupported property for Tables.Schema"))
end
Expand Down
11 changes: 11 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,17 @@ end

# 228
@test Tables.columntable(NamedTuple[]) === NamedTuple()

@testset "Type stability of schema(x).types" begin
_get_types(X) = Tables.schema(X).types

x = (a=[1.0], b=[1.0])
@inferred Tuple{Type{Float64},Type{Float64}} _get_types(x)

# Trigger other branch which lacks type stability:
x_wide = NamedTuple([Symbol("x$(i)") => [1.0] for i=1:513])
@inferred NTuple{513,DataType} _get_types(x_wide)
end
end

@testset "Materializer" begin
Expand Down

0 comments on commit b3ec016

Please sign in to comment.