-
Notifications
You must be signed in to change notification settings - Fork 78
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
Allow GROUP BY
queries by providing deserialize_next_tagged
to deserialize the group fields
#69
Conversation
Hey @SafariMonkey, sorry for my absence. I tested this PR and it's working beautifully. Thanks for the work!
In the past, it was not needed, so that's why the #[doc(hidden)] was added. If it's public, maybe a better name for |
A rose by any other name would still smell as sweet. I suggest merging this, and if there's additional aesthetic clarity, it can come in via another pull request. |
I'm fine with @cryptoquick's suggestion. If you want to change the name feel free, but I'm happy enough with it as is. I already gave it a once over before submitting it. |
Thanks for your second awesome contribution! |
@SafariMonkey yes, thanks for working on this even though it's no longer core to your project. And thank you, @Empty2k12 for being such a responsive maintainer, even over the weekend! Rust is a healthier community thanks to both of your efforts. |
I will release a build on crates.io today or tomorrow. Thanks again. |
@cryptoquick As a slight correction, I was only saying before that writing to InfluxDB wouldn't be able to be handled by this library due to its quoting - the reading is still this library. It's only a prototype I'm putting together during some spare moments, hence the long silence last month. I'm happy that these PRs can help the project, and I will continue to submit them should I run into areas that I think could use improvement. I'd also like to echo the appreciation for your work in reviewing these changes and your responsiveness, @Empty2k12. And it's nice to have a PR so well received :) |
@SafariMonkey @cryptoquick Sorry for the long wait, but the v0.2.0 version is now live on crates.io. |
Description
This change adds deserialisation for the metadata of a
GROUP BY
using the methoddeserialize_next_tagged
, which takes aTAG
parameter (so called because that's what the InfluxDB response calls it) which is deserialized from the metadata. BecauseSeries
has a custom deserializer, this meansTaggedSeries
also has to. It's pretty copy paste, with minimal code changes, and leverages the existingSeries
value deserialisation. I originally tried to abstract over having tags and not but it ended up being way hairier and more error prone, so I thought copying ~100 lines of boilerplate is probably fine.I've tested this on my code and it works fine. I also added a test and that passes.
Side note, is the
#[doc(hidden)]
something that can be removed? The user has to use that struct to access the series.Checklist
cargo fmt --all
cargo clippy --all-targets --all-features -- -D warnings
cargo readme -r influxdb -t ../README.tpl > README.md