queoahfh
Joined 3 months ago
Karma
7
Submissions
6
Member since
Jul 9
Recent submissions
Comment · 3 months ago
Comment · 3 months ago
Yes, but the original boolean seems to have been used for error handling, and the tagged union is also used for error handling. Why have both simultaneously in the same function instead of just one of the two?
Edit: Looking at the code again, perhaps I was mistaken, since the boolean might not have been for error handling, just the result of the function, and C's limitations regarding error handling led it to using something like elog(), apparently a macro defined in https://github.com/postgres/postgres/blob/master/src/include... .
Comment · 3 months ago
I am concerned about the quality. Even a cursory skim of the code makes the code appear asinine. Unless the genius aspects of the code elude me.
https://github.com/malisper/pgrust/blob/3646a73515a5e4ac7d0b...
https://github.com/malisper/pgrust/blob/3646a73515a5e4ac7d0b...
Comment · 3 months ago
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...
Comment · 3 months ago
What a peculiar kind of rewrite.
Rust:
https://github.com/malisper/pgrust/blob/3646a73515a5e4ac7d0b...
Original:
https://github.com/postgres/postgres/blob/df293aed46e3133df3...
Usage:
https://github.com/malisper/pgrust/blob/3646a73515a5e4ac7d0b...
The return type in the rewrite is both some sort of Error tagged union that supports the Try machinery in Rust; but, it also contains a boolean that apparently must be checked; or something. It seems labyrinthical and possibly broken and terrible.
Comment · 3 months ago
Didn't the initial rewrite of Bun into Rust have an ocean of "unsafe" in it, and wasn't it entirely dysfunctional?
Though often when applicable, a simple tagged union is used instead when that would document the intention better. Like, the Rust version of search_pg_class_full_form::call() returns a Some for cache hit and None for cache miss as far as I can skim, and that group of methods returning that could arguably have returned a basic enum instead with CacheHit(value) and CacheMiss. Though this is a nitpick on my part.