2024-02-15 kernel meeting notes

[zachschuermann]

summary

merged robert's PR!! discussion on nick's - and some concern of visitor complexity.

action items

PRs in flight:

• Switch to new data passing API #109
• Leverage more engine capabilities in data skipping 2/n #83
• start implementing FFI #91

attendees

@roeap @zachschuermann @nicklan @ryan-johnson-databricks @hntd187 @vkorukanti

notes stream

PRs in flight (listed above).

• Robert's PR going in!
• unblocks nick's (time for a big rebase)

need to create filter data engine side

after nick + robert PR then we unblock:

• final removal of arrow from the rest of kernel API
• arrow should only be in default client etc.
• DV impl is done (we give back engine data in bool array) - may want to do something more friendly (e.g. expect filtering API then give back something with deleted rows already removed)

for nick's PR:

• in arrow, list or map you get entire column and ask for specific row
• nick does 'caller' needs to say which row they are interested in
• should instead just remember which row you are operating on
• on revisiting, realized lifetimes are hard
• engine gives out references, kernel can copy if we want
• API is: poke at map for row x
• could do Box<dyn List<item>>
• do we want to box every list/map?

ryan: concern. data visitor pattern feeling very complex. might be going a bad direction.

• will explore today to see if we can simplify.

robert: should we just ask for a copy every time? do we want ownership in the first place?

main concern is StringType. anything that implements Copy is easy. Memory backing the thing is owned by rust. For FFI we would have memory backed by non-rust.

nick: less about data format (always have to convert between engine/kernel format). more about allocations/frees across boundary. kernel can do all allocations for the things its building and rust will track lifetimes of that. a solution with the engine making copies may lead to a new kind of complexity in which we have to move lifetime tracking from the engine over to rust. main goal of the visitor model was making the lifetimes much more sane :) - a core part of FFI

ryan: should we keep trivial cast warning enabled? nope

From last time

nick: should we just parse all stats at once? we should have a clear picture of when we would use this? add microbenchmark

return result in C: (1) slot in parameters, return code or (2) null means error, not-null means success

• slot tricky for trait (trait is fat pointer)
• if engine needs to know about trait: vtable
• or doesn't need to know, we put in Box and leak the box
• vtable can get messy

module naming: rename defaultclient/simpleclient. good to have non-async client.

from last time: nick's scan result. doing selection vector now? yep! has a large boolean vector with it. Allocating with lots of false is fast. True takes a while.

from last time: How can we flatten roaring bitmap? Maybe build the dumb thing now (for loop) and take this on as an optimization later? Roaring bitmap stores indicies but we would want a sparse array.
Aside (zach): does allocating for a selection vector cause memory management issues?
Allocate big vector, then set indicies in the treemap.

Ryan: 3 things

1. FFI and error result types
   FFI version of EngineInterface
2. traits/vtable implementation: what to do if the engine already has rust trait version of something? we want to allow connectors to use normal (default) expression eval but bring their own custom parquet. How?
3. we have lots of iterators. can we unify them? Then just one way of iterating. Get rid once everything is EngineData. Lifetimes are clean. Engine knows how to iterate. Relies on other PRs, going to wait for the dust to settle.

From nick/robert: unsafe in kernel used in one place to do Box::into_raw can we work around this? Fixed: we can implement a method that takes the box rather than the type itself. self if a box of the thing, then into_any, then downcast. yay!
Box<trait> -> Box<any> -> Box<concrete> then turn into record batch. No transmute.