-
Notifications
You must be signed in to change notification settings - Fork 38
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
Fixed query creation for Option
values
#57
base: main
Are you sure you want to change the base?
Conversation
Option
values
per the docs the timestamp and tags are actually both optional so it shouldn't panic if there are no valid tags or a timestamp (and probably support optional timestamps as well) |
The timestamp and the tags are now optional. This means it is now also allowed to not use any tags, which addresses #49. @poster515 thanks for posting the link to the docs and pointing this out. But the behavior issue from above is still present when the following struct would be used: #[derive(WriteDataPoint)]
#[measurement = "something"]
struct Item6 {
#[influxdb(tag)]
tag1: Option<String>,
#[influxdb(field)]
field1: String,
#[influxdb(timestamp)]
time: u64,
} Here the |
This seems like a question of "where should the fault occur?". The failure to write the point either happens in this code or at the influx server itself. I kinda think simply not panicking and letting the write fail at the server is an easy approach but am not a maintainer lol |
Thanks for the contribution and constructive discussion. I hope to give this a closer look later this week. I am leaning towards ensuring that at least 1 field is written and returning an Err if that isn't the case. I'd expect performance overhead to be acceptable. But I'd be happy to add that to this PR myself |
Thanks for looking at the PR. It would be nice if you could add this to the PR, because I am currently traveling and won't have much time. |
@UnHolds @Boudewijn26 Is this PR abandoned? |
@LevBeta My apologies for my tardiness. I've been quite busy. I'll try to get to this soon |
If an option type is used, and it is None, then the tag / field will be excluded from the query, because this is the only working way to transmit null values to the database. (Issue #56 )
This behavior introduces a bug that may lead to an invalid query. The problem arises when a struct contains only option values for the tags and or fields, because then if every option value is
None
every tag / field will be excluded. This breaks the rule that at least one tag / field must exist.influxdb2/influxdb2-derive/src/expand_writable.rs
Lines 99 to 107 in 11b8b9d
In my opinion, this bug is more acceptable than the currently non-functional implementation for options.
I just started out with Rust, so feedback is highly appreciated :)