Earlier quoted context omitted.
From what I skimmed manually, not that many, but the code itself seems labyrinthical. Like, why have both Rust Try-supporting Error-like tagged union, but also booleans, for error handling, in the same function? https://github.com/malisper/pgrust/blob/3646a73515a5e4ac7d0b... https://github.com/malisper/pgrust/blob/3646a73515a5e4ac7d0b...
I'm not sure what you mean? The rust code you're showing mimics the Postgres code: https://github.com/postgres/postgres/blob/2e6578292a9184dcaa... The boolean being returned is the return value of the function. It's not used to return an error.
An unfortunate aspect is that the code has become a bit more bloated in some regards due to usage of Result, instead of an implicit elog() macro and similar. Passing Result around, in some ways as an alternative to an unwinding exception, is cleaner in some ways, but it also bloats the code somewhat.
The rewrite also could have simpler code in some cases, like
https://github.com/malisper/pgrust/blob/3646a73515a5e4ac7d0b...
could perhaps just be
match syscache_seams::search_pg_class_full_form::call(ctx.mcx(), relationId)? {
Some(form) => Ok(form.relhassubclass),
None => {
Err(ereport(ERROR)
.errmsg(format!("cache lookup failed for relation {relationId}"))
.into_error())
}
}
I see a lot of MemoryContext. I am not sure how much that bloats the code (though the C code is bloated due to C's issues and problems, like re-using collections and such). Does it incur an overhead?