@Cory Grinstead @Clark Zinzow Adding y'all here to talk about the local readers that Cory is working on. For context, @Clark Zinzow built out the streaming CSV and JSON readers.

@Cory Grinstead Happy to answer any questions you have! And I'm really looking forward to having more performant local readers. 😄

Hey so i finally have some time to get pretty heads down on this today. Considering I wrote most of the json reader in polars, I was planning on trying to port over as much of it as possible to not reinvent the wheel. However, I do think that given it'd be a "substantial portion" of code, it requires including the original MIT license on the ported code.

PR that swaps out json_deserializer for simd_json https://github.com/Eventual-Inc/Daft/pull/2228. I'm seeing ~10-20% faster times on this branch!

That’s incredible @Cory Grinstead!

@Clark Zinzow would prob be the best to review

looks like there is one issue with the simd_json impl, it doesn't guarantee the ordering, so

test_load_json

is failing because the columns are out of order.

@Cory Grinstead Yes, unfortunately preserving the order of keys in

Object

values isn't supported in `simd_json`: https://github.com/simd-lite/simd-json/issues/270

I also left an initial review of the PR - outside of the preservation of column ordering, the only other question I had was around preservation of exponent format for ints/floats when decoding into a string column. Otherwise, the PR looks great!

@Cory Grinstead I left another review! I can merge the PR as soon as you validate my understanding for the unsafe code safety conditions.

I have a working POC of the local json reader! https://github.com/Eventual-Inc/Daft/pull/2264 It still needs quite a bit of work, but thought I'd at least share the initial draft!

After i finish up ☝️ I have a few additional ideas I want to try out. • There’s a few copies that need to be done by using the

StreamDeserializer

, I think if we created our own equivalent, we’d be able to avoid a few of those copies. • We can also make things a bit faster if we use the

simd_json

Tape

directly instead of using the intermediate

Value

struct. It'll allow us to better utilize projection pushdowns.

I see, so skip decoding the columns that we don’t need?

yeah exactly, Given that we have a known schema when decoding, we can skip irrelevant key/value pairs.

It looks like the current streaming implementation applies the projection after parsing

Ah we should fix that and skip decoding those then!

Can you do a follow up and fix that in the streaming reader as well?

Also, is the local reader pr ready to review?

Also, is the local reader pr ready to review?

yes it's ready for review.

@Cory Grinstead I don't think that's the case. IIRC, we push the column selection projection into parsing but include columns that are required for a filter predicate, and that line that you linked is a post-filter selection to ensure we drop any filter predicate columns not included in the projection.

I could be misremembering, but I believe that was my intended implementation at least!

I don't think that's the case. IIRC, we push the column selection projection into parsing but include columns that are required for a filter predicate, and that line that you linked is a post-filter selection to ensure we drop any filter predicate columns not included in the projection.

are there any tests for this scenario? I want to make sure that the new local reader handles this case as well.

I think i got most of the feedback on the JSON PR addressed. There are a few outstanding comments that I'll get to on Friday. (I'll be offline all day tomorrow).

Thanks Cory!