Added mapping for xid
#2003
Added mapping for xid
#2003
Conversation
|
@YohDeadfall I think this looks good, outside of the one minor change. I think #2014 is going to force a pgrx v0.14.0, so now is a great time to go ahead and get this finished and committed. |
| } | ||
|
|
||
| #[inline] | ||
| pub const fn to_u32(self) -> u32 { |
There was a problem hiding this comment.
Is it really fine? I would like to avoid any further name bikeshading as it's a little disaster even if another method added and this marked as deprecated. Shouldn't it be into_u32?
There was a problem hiding this comment.
honestly, we should call this into_inner(self) and then do the same over on pg_sys::Oid and close #2011.
And there's also no need for this from_u32(xid: u32) function either, because of impl From<u32> for TransactionId down below.
There was a problem hiding this comment.
from_u32 is for const only since traits cannot have that modifier on the functions. It can be removed, but then it won't be possible to use during compiler time.
If there's no need in const fns then I see no reason in from_smthg and into_smthg at all as there are from and into. Makes sense?
| } | ||
|
|
||
| #[inline] | ||
| pub const fn to_u32(self) -> u32 { |
There was a problem hiding this comment.
honestly, we should call this into_inner(self) and then do the same over on pg_sys::Oid and close #2011.
And there's also no need for this from_u32(xid: u32) function either, because of impl From<u32> for TransactionId down below.
|
|
||
| /// An `xid` type from PostgreSQL | ||
| #[repr(transparent)] | ||
| #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] |
There was a problem hiding this comment.
I think we also want to impl Display too. Have it just be the u32 value. It's not uncommon to want to convert a transaction id into a string for logging purposes
eeeebbbbrrrr
left a comment
eeeebbbbrrrr
left a comment
There was a problem hiding this comment.
Thanks for your diligence on this one, @YohDeadfall !
2 participants
No description provided.