feat : ABI upgrade from abi_stabby to stabby since abi_stable is no longer maintained

[coderfender]

Which issue does this PR close?

• Closes Remove generational-arena from project #20863

Rationale for this change

What changes are included in this PR?

• Replace abi_stable types with stabby equivalents:
  • RVec -> stabby::vec::Vec
  • RString -> stabby::string::String
• Add custom FFI_Option and FFI_Result types since we have raw pointers and self referential pointers which can't implement IStable trait needed for stabby
• Rename macros: rresult → sresult, rresult_return → sresult_return
• Fix library path detection for debug/release builds in the example loader
• Update README

Are these changes tested?

Yes . With df python bindings on DF 52 release

Are there any user-facing changes?

[coderfender]

Linking Tim's PR here #21025

[coderfender]

1. Removed abi_stable package dependency

2. Replaced all abi_stable calls with stabby equivalents

3. Used Vec<K,V> since stabby doesnt have HashMap support
   TODO :

4. Work on WrappedArray / native arrow types integration (Original impl through ABI just assumed safe implementation through sabi. However, stabby's answer is a little different)

5. Refactor / remove async-ffi module ( IMHO, this can be done in a separate followup PR)

[timsaucer]

One of the things I've been thinking about here is doing some scale testing of performance, which I haven't done on the FFI crate really. I was thinking we could do something along the lines of using https://github.com/datafusion-contrib/datafusion-tpch to generate table providers at different scale factors. Then it would seem we could have a series of tests:

1. Pure rust with no FFI work.
2. Pure rust but using two modules and passing table provider via FFI.
3. Expose table provider to python and test with datafusion-python.

The thing I like about doing this is that we would be able to see the impacts of each of the layers between the code, ideally going from 2->3 having near zero impact.

For such a test I would think about setting up a stream, reading in and dumping the data as fast as possible.

Since this is orthogonal to the actual FFI work you're proposing I might try setting this up on a test repo.

[coderfender]

Merged with main and see some referenced older package. Working on fixing it to use stabby

[coderfender]

Updated cargo format , changed table_provider_module to use the direct function pointer call instead of accessor method pattern used in abi_stable

[coderfender]

Testing python bindings now with the new ABI

[coderfender]

The df python bindings rendered through new stabby implementations seem to be working on my local machine

[timsaucer]

Really nice work on this. I've put in a few questions. I think it's really great that the vast majority of the changes are pretty simple and have 1-1 replacement.

[coderfender]

thank you for the comments @timsaucer . I will work on the comments and push a commit shortly

[coderfender]

Pushed a commit to address review comments . Please take a look whenever you get a chance

[coderfender]

I'll rebase this branch blue that we have FFI for Physical optimizer rule change merged to main

[coderfender]

@timsaucer , I updated PhysicalOptimizerRule to use stabby instead of abi_stable as well. Please take a look whenever you get a chance . Thank you

[timsaucer]

After some thorough testing on leaning into the macros of the stabby crate, I have come to agree that your approach here is the best path forward. I think we need additional documentation in the FFI crate's readme to explain why we are limiting ourselves to basically using some of the stabby structs but not leaning into the macros it comes with. That way future developers do not ask the same questions, especially since stabby appears to do many of the things we would want.

[coderfender]

Thank you for the update @timsaucer . Let me go ahead and update the docs to better reflect our approach here

[timsaucer]

Having both a FFIResult<T> and a FFI_Result<T, SString> is confusing. Can we merge down to one?

[coderfender]

Great point ! sure

[coderfender]

Given that there is only String based error usage across migration, we might not even need <T,E> and could consolidate this into just FFIResult<T>

[timsaucer]

Yes, FFI_Result<T> to be consistent with the rest of the repo

[coderfender]

Absolutely! I made changes to have one result used and exposed outside

[timsaucer]

For users who are interested in working on this code, they will probably ask themselves the same thing I asked. Why is it we are not using the core features of stabby? You and I have both come to the same conclusion that especially with the work on getting the record batch stream converted over to using stabby it turns into a much different project, especially when it comes to build times with the heavy macro use.

I think we just need some documentation as mentioned in a prior comment.