Skip to content
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: add precision flag to write subcommand #25935

Merged
merged 2 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions influxdb3/src/commands/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::{
};

use clap::Parser;
use influxdb3_client::Precision;
use secrecy::ExposeSecret;
use tokio::io;

Expand Down Expand Up @@ -53,6 +54,10 @@ pub struct Config {

/// Give a quoted line protocol line via the command line
line_protocol: Option<Vec<String>>,

/// Specify a supported precision (eg: ns, us, ms, s).
#[clap(short = 'p', long = "precision")]
precision: Option<Precision>,
}

pub(crate) async fn command(config: Config) -> Result<()> {
Expand Down Expand Up @@ -83,6 +88,9 @@ pub(crate) async fn command(config: Config) -> Result<()> {
};

let mut req = client.api_v3_write_lp(database_name);
if let Some(precision) = config.precision {
req = req.precision(precision);
}
if config.accept_partial_writes {
req = req.accept_partial(true);
}
Expand Down
95 changes: 95 additions & 0 deletions influxdb3/tests/server/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1767,3 +1767,98 @@ async fn write_and_query_via_string() {
result
);
}

#[test_log::test(tokio::test)]
async fn write_with_precision_arg() {
let server = TestServer::spawn().await;
let server_addr = server.client_addr();
let db_name = "foo";
let table_name = "bar";

struct TestCase {
name: &'static str,
precision: Option<&'static str>,
expected: &'static str,
}

let tests = vec![
TestCase {
name: "default precision is seconds",
precision: None,
expected: "1970-01-01T00:00:01",
},
TestCase {
name: "set seconds precision",
precision: Some("s"),
expected: "1970-01-01T00:00:01",
},
TestCase {
name: "set milliseconds precision",
precision: Some("ms"),
expected: "1970-01-01T00:00:00.001",
},
TestCase {
name: "set microseconds precision",
precision: Some("us"),
expected: "1970-01-01T00:00:00.000001",
},
TestCase {
name: "set nanoseconds precision",
precision: Some("ns"),
expected: "1970-01-01T00:00:00.000000001",
},
];

for test in tests {
let TestCase {
name,
precision,
expected,
} = test;
let name_tag = name.replace(' ', "_");
let lp = format!("{table_name},name={name_tag} theanswer=42 1");

let mut args = vec!["write", "--host", &server_addr, "--database", db_name];
if let Some(precision) = precision {
args.extend(vec!["--precision", precision]);
}
args.push(&lp);

run(args.as_slice());
let result = server
.api_v3_query_sql(&[
("db", db_name),
(
"q",
format!("SELECT * FROM {table_name} WHERE name='{name_tag}'").as_str(),
),
("format", "json"),
])
.await
.json::<Value>()
.await
.unwrap();
assert_eq!(
result,
json!([{
"name": name_tag,
"theanswer": 42.0,
"time": expected,
}]),
"test failed: {name}",
);
}

let lp = format!("{table_name},name=invalid_precision theanswer=42 1");
let output = run_and_err(&[
"write",
"--host",
&server_addr,
"--database",
db_name,
"--precision",
"fake",
&lp,
]);
insta::assert_snapshot!("invalid_precision", output);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: influxdb3/tests/server/cli.rs
expression: output
snapshot_kind: text
---
error: invalid value 'fake' for '--precision <PRECISION>': unrecognized precision unit: fake

For more information, try '--help'.
18 changes: 18 additions & 0 deletions influxdb3_client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ pub enum Error {
#[source]
source: reqwest::Error,
},

#[error("unrecognized precision unit: {0}")]
UnrecognizedUnit(String),
}

impl Error {
Expand Down Expand Up @@ -903,6 +906,21 @@ pub enum Precision {
Nanosecond,
}

impl std::str::FromStr for Precision {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we re-use whatever it is that parses the precision argument from the HTTP API? That way they're always the exact same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Several types are duplicated in this crate, e.g., Format, from the influxdb3_server crate. Perhaps we should pull influxdb3_server as a dependency of influxdb3_client and re-use them directly, or move the types to a central crate.

But this has bit us a couple times now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into the type used for the HTTP API since my initial grep for an appropriate type turned up both, but it looks to me like the HTTP API uses serde deserialization where the HTTP request body refers to the precision types as "second", "millisecond", etc (as opposed to "s", "ms", etc).

Even if we wanted to use the full word for precision unit for the CLI, we would still need to impl FromStr, which is what clap uses to parse arguments.

@hiltontj I did notice the TODOs in the code around consolidating types between crates -- is there a github issue tracking that? I could look into that after this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type Err = Error;

fn from_str(s: &str) -> Result<Self> {
let p = match s {
"s" => Self::Second,
"ms" => Self::Millisecond,
"us" => Self::Microsecond,
"ns" => Self::Nanosecond,
_ => return Err(Error::UnrecognizedUnit(s.into())),
};
Ok(p)
}
}

/// Builder type for composing a request to `/api/v3/write_lp`
///
/// Produced by [`Client::api_v3_write_lp`]
Expand Down