-
Notifications
You must be signed in to change notification settings - Fork 590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(type): improve the Fields
derive macro
#14934
Conversation
Signed-off-by: Runji Wang <[email protected]>
Fields
derive macroFields
derive macro
Signed-off-by: Runji Wang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM
@@ -570,10 +570,17 @@ pub trait ToOwnedDatum { | |||
fn to_owned_datum(self) -> Datum; | |||
} | |||
|
|||
impl ToOwnedDatum for DatumRef<'_> { | |||
impl<T: Into<ScalarImpl>> ToOwnedDatum for T { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH I'd prefer introducing a new trait for doing such conversion. By ToOwnedDatum
I suppose it's only for the conversion from the ref type to an owned type, just like the std's ToOwned
trait with GAT support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But logically types impl Into<ScalarImpl>
can be converted ToOwnedDatum
. I think it would be convenient to reuse this trait.
Signed-off-by: Runji Wang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Runji Wang <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR adds
primary_key
andinto_owned_row
support to theFields
trait and derive macro.This is a preparation for refactoring system catalogs #13433.
Checklist
./risedev check
(or alias,./risedev c
)Documentation