vlcn-io / cr-sqlite

Convergent, Replicated SQLite. Multi-writer and CRDT support for SQLite
https://vlcn.io
MIT License
2.61k stars 69 forks source link

Move crsql_createCrr to Rust #349

Closed Azarattum closed 9 months ago

Azarattum commented 9 months ago

Closes https://github.com/vlcn-io/cr-sqlite/issues/325.

Functions moved to rust:

Table info is implemented with raw pointers to ensure compatibility with C counterparts. Therefore all the C tests verify the new Rust implementations.

Also we can convert the entire tableinfo.c as long as we convert get-table.c (as it is its only dependency). I can continue doing that here or start a new issue/PR for that.

tantaman commented 9 months ago

also -- I'm assuming you can address the two issues and update the PR even though I approved the pull request?

That was a flow we often used at Facebook (approved with comments to be addressed) but idk if it works on GitHub.

tantaman commented 9 months ago

Also we can convert the entire tableinfo.c as long as we convert get-table.c (as it is its only dependency). I can continue doing that here or start a new issue/PR for that.

Lets do it in a new PR.

Azarattum commented 9 months ago

I love it and learned some idiomatic rust today.

This is curious... I expected to get some things wrong. The big reason why I took this PR is that I wanted to start learning Rust. Actually this is my first real code in Rust 😅

tantaman commented 9 months ago

This is curious...

haha, well I learned more ways to use generic traits and about map_err

tantaman commented 9 months ago

Actually this is my first real code in Rust

Could have fooled me but I took a second look over and found some things that should be addressed.

Azarattum commented 9 months ago

@tantaman what do you think? I've added a Countable util trait since that code repeated too often. is_table_compatible function is considerably simpler now.

tantaman commented 9 months ago

is_table_compatible function is considerably simpler now.

👍