-
Notifications
You must be signed in to change notification settings - Fork 15
Typesense derive enhancements #59
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
base: main
Are you sure you want to change the base?
Conversation
I thought it's supposed to wrap. Because there are optional fields possible, no? How else would you update a field and set it to null? That needs to be added to tests. |
| let flattened_fields = quote! { | ||
| <#inner_type as typesense::prelude::Document>::collection_schema().fields | ||
| .into_iter() | ||
| .map(|mut f| { | ||
| // Use the dynamically determined prefix here | ||
| f.name = format!("{}.{}", #prefix, f.name); | ||
| if #is_vec && !f.r#type.ends_with("[]") { | ||
| f.r#type.push_str("[]"); | ||
| } | ||
| f | ||
| }) | ||
| .collect::<Vec<_>>() | ||
| }; |
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.
Don't do it in a runtime, at least use const logic.
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.
I'm not sure how to optimize this. Could you clarify "const logic"?
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.
Would really appreciate if you can optimize it.
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.
Ok, I will look into it later.
typesense_derive/src/lib.rs
Outdated
| fn collection_schema() -> ::typesense::models::CollectionSchema { | ||
| let name = Self::COLLECTION_NAME.to_owned(); | ||
| let fields = vec![#(#typesense_fields,)*]; | ||
| let fields = vec![#(#typesense_fields,)*].into_iter().flatten().collect(); |
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.
Same issue: must not be .flatten().collect() in runtime.
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.
Used .extend() instead of .flatten().collect(), is this the ideal approach?
let mut fields = Vec::new();
#(fields.extend(#typesense_fields);)*|
Attributes |
Yes, you're right. I have reverted the change and added test for it. |
Change Summary
This PR added support for more attributes:
Collection-level attributes
Added support for
token_separatorsandsymbols_to_index:Field-level attributes
sort,index,store,infix,stem,range_index,optionallocale,vec_dist,num_dimtype: override the auto inferred typerename: rename a Typesense field (This must be used with#[serde(rename = "new_name")])flatten: generate schema for fields of nested structsskip: skip generating schema for a fieldBoolean attributes can be specified as a shorthand flag e.g.
#[typesense(index)]or explicitly set a value e.g.#[typesense(index = false)])Full example:
Bug fix
This PR also fixed the partial struct derive wrapping an already-optional field in an
Option<>