From daedae4151c6169ea058dc379b4536e91333e231 Mon Sep 17 00:00:00 2001 From: JakkuSakura Date: Sun, 6 Apr 2025 15:36:04 +0800 Subject: [PATCH 1/2] Enhance error handling and simplify Result type usage in the library --- Cargo.toml | 3 +++ src/error.rs | 7 +++++- src/lib.rs | 17 +++++++++------ src/query_result.rs | 43 ++++++++++++++++++++++++------------- src/session.rs | 6 +----- tests/examples.rs | 52 +++++++++++++++++++++------------------------ 6 files changed, 72 insertions(+), 56 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bde8879..5ea76bd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,3 +9,6 @@ thiserror = "1" [build-dependencies] bindgen = "0.70.1" + +[dev-dependencies] +tempdir = "0.3.7" \ No newline at end of file diff --git a/src/error.rs b/src/error.rs index cf9023f..e1276a2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,9 +1,12 @@ use std::ffi::NulError; +use std::string::FromUtf8Error; #[derive(Debug, thiserror::Error)] pub enum Error { #[error("An unknown error has occurred")] Unknown, + #[error("No result")] + NoResult, #[error("Invalid data: {0}")] InvalidData(String), #[error("Invalid path")] @@ -15,7 +18,9 @@ pub enum Error { #[error("Insufficient dir permissions")] InsufficientPermissions, #[error("Non UTF-8 sequence: {0}")] - NonUtf8Sequence(String), + NonUtf8Sequence(FromUtf8Error), #[error("{0}")] QueryError(String), } + +pub type Result = std::result::Result; diff --git a/src/lib.rs b/src/lib.rs index 2c3e606..5ff5bf6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,9 +17,10 @@ use std::ffi::{c_char, CString}; use crate::arg::Arg; use crate::error::Error; +use crate::error::Result; use crate::query_result::QueryResult; -pub fn execute(query: &str, query_args: Option<&[Arg]>) -> Result, Error> { +pub fn execute(query: &str, query_args: Option<&[Arg]>) -> Result { let mut argv = Vec::with_capacity(query_args.as_ref().map_or(0, |v| v.len()) + 2); argv.push(arg_clickhouse()?.into_raw()); @@ -33,26 +34,28 @@ pub fn execute(query: &str, query_args: Option<&[Arg]>) -> Result) -> Result, Error> { +fn call_chdb(mut argv: Vec<*mut c_char>) -> Result { let argc = argv.len() as i32; let argv = argv.as_mut_ptr(); let result_ptr = unsafe { bindings::query_stable_v2(argc, argv) }; if result_ptr.is_null() { - return Ok(None); + return Err(Error::NoResult); } + let result = QueryResult::new(result_ptr); + let result = result.check_error()?; - Ok(Some(QueryResult(result_ptr).check_error()?)) + Ok(result) } -fn arg_clickhouse() -> Result { +fn arg_clickhouse() -> Result { Ok(CString::new("clickhouse")?) } -fn arg_data_path(value: &str) -> Result { +fn arg_data_path(value: &str) -> Result { Ok(CString::new(format!("--path={}", value))?) } -fn arg_query(value: &str) -> Result { +fn arg_query(value: &str) -> Result { Ok(CString::new(format!("--query={}", value))?) } diff --git a/src/query_result.rs b/src/query_result.rs index 0d1f331..64389e8 100644 --- a/src/query_result.rs +++ b/src/query_result.rs @@ -5,17 +5,23 @@ use std::time::Duration; use crate::bindings; use crate::error::Error; - +use crate::error::Result; #[derive(Clone)] -pub struct QueryResult(pub(crate) *mut bindings::local_result_v2); +pub struct QueryResult { + inner: *mut bindings::local_result_v2, +} impl QueryResult { - pub fn data_utf8(&self) -> Result { - String::from_utf8(self.data_ref().to_vec()) - .map_err(|e| Error::NonUtf8Sequence(e.to_string())) + pub(crate) fn new(inner: *mut bindings::local_result_v2) -> Self { + Self { inner } } + pub fn data_utf8(&self) -> Result { + let buf = self.data_ref(); - pub fn data_utf8_lossy<'a>(&'a self) -> Cow<'a, str> { + String::from_utf8(buf.to_vec()).map_err(Error::NonUtf8Sequence) + } + + pub fn data_utf8_lossy(&self) -> Cow { String::from_utf8_lossy(self.data_ref()) } @@ -24,30 +30,37 @@ impl QueryResult { } pub fn data_ref(&self) -> &[u8] { - let buf = unsafe { (*self.0).buf }; - let len = unsafe { (*self.0).len }; + let inner = self.inner; + let buf = unsafe { (*inner).buf }; + let len = unsafe { (*inner).len }; let bytes: &[u8] = unsafe { slice::from_raw_parts(buf as *const u8, len) }; bytes } pub fn rows_read(&self) -> u64 { - (unsafe { *self.0 }).rows_read + let inner = self.inner; + unsafe { *inner }.rows_read } pub fn bytes_read(&self) -> u64 { - unsafe { (*self.0).bytes_read } + let inner = self.inner; + unsafe { *inner }.bytes_read } pub fn elapsed(&self) -> Duration { - let elapsed = unsafe { (*self.0).elapsed }; + let elapsed = unsafe { (*self.inner).elapsed }; Duration::from_secs_f64(elapsed) } - pub(crate) fn check_error(self) -> Result { - let err_ptr = unsafe { (*self.0).error_message }; + pub(crate) fn check_error(self) -> Result { + self.check_error_ref()?; + Ok(self) + } + pub(crate) fn check_error_ref(&self) -> Result<()> { + let err_ptr = unsafe { (*self.inner).error_message }; if err_ptr.is_null() { - return Ok(self); + return Ok(()); } Err(Error::QueryError(unsafe { @@ -58,6 +71,6 @@ impl QueryResult { impl Drop for QueryResult { fn drop(&mut self) { - unsafe { bindings::free_result_v2(self.0) }; + unsafe { bindings::free_result_v2(self.inner) }; } } diff --git a/src/session.rs b/src/session.rs index 0c1309c..591ea1d 100644 --- a/src/session.rs +++ b/src/session.rs @@ -82,11 +82,7 @@ impl<'a> Default for SessionBuilder<'a> { } impl Session { - pub fn execute( - &self, - query: &str, - query_args: Option<&[Arg]>, - ) -> Result, Error> { + pub fn execute(&self, query: &str, query_args: Option<&[Arg]>) -> Result { let mut argv = Vec::with_capacity( self.default_args.len() + query_args.as_ref().map_or(0, |v| v.len()) + 1, ); diff --git a/tests/examples.rs b/tests/examples.rs index 57b7252..a59f614 100644 --- a/tests/examples.rs +++ b/tests/examples.rs @@ -1,4 +1,5 @@ use chdb_rust::arg::Arg; +use chdb_rust::error::Result; use chdb_rust::execute; use chdb_rust::format::InputFormat; use chdb_rust::format::OutputFormat; @@ -6,63 +7,59 @@ use chdb_rust::log_level::LogLevel; use chdb_rust::session::SessionBuilder; #[test] -fn stateful() { +fn test_stateful() -> Result<()> { // // Create session. // - + let tmp = tempdir::TempDir::new("chdb-rust")?; let session = SessionBuilder::new() - .with_data_path("/tmp/chdb") + .with_data_path(tmp.path()) .with_arg(Arg::LogLevel(LogLevel::Debug)) .with_arg(Arg::Custom("priority".into(), Some("1".into()))) .with_auto_cleanup(true) - .build() - .unwrap(); + .build()?; // // Create database. // - session - .execute("CREATE DATABASE demo; USE demo", Some(&[Arg::MultiQuery])) - .unwrap(); + session.execute("CREATE DATABASE demo; USE demo", Some(&[Arg::MultiQuery]))?; // // Create table. // - session - .execute( - "CREATE TABLE logs (id UInt64, msg String) ENGINE = MergeTree ORDER BY id", - None, - ) - .unwrap(); + session.execute( + "CREATE TABLE logs (id UInt64, msg String) ENGINE = MergeTree() ORDER BY id", + None, + )?; // // Insert into table. // - session - .execute("INSERT INTO logs (id, msg) VALUES (1, 'test')", None) - .unwrap(); + session.execute("INSERT INTO logs (id, msg) VALUES (1, 'test')", None)?; // // Select from table. // + let len = session.execute( + "SELECT COUNT(*) FROM logs", + Some(&[Arg::OutputFormat(OutputFormat::JSONEachRow)]), + )?; - let result = session - .execute( - "SELECT * FROM logs", - Some(&[Arg::OutputFormat(OutputFormat::JSONEachRow)]), - ) - .unwrap() - .unwrap(); + assert_eq!(len.data_utf8_lossy(), "{\"COUNT()\":1}\n"); + let result = session.execute( + "SELECT * FROM logs", + Some(&[Arg::OutputFormat(OutputFormat::JSONEachRow)]), + )?; assert_eq!(result.data_utf8_lossy(), "{\"id\":1,\"msg\":\"test\"}\n"); + Ok(()) } #[test] -fn stateless() { +fn test_stateless() -> Result<()> { let query = format!( "SELECT * FROM file('tests/logs.csv', {})", InputFormat::CSV.as_str() @@ -71,9 +68,8 @@ fn stateless() { let result = execute( &query, Some(&[Arg::OutputFormat(OutputFormat::JSONEachRow)]), - ) - .unwrap() - .unwrap(); + )?; assert_eq!(result.data_utf8_lossy(), "{\"id\":1,\"msg\":\"test\"}\n"); + Ok(()) } From 00354d8a2e5f98c2b6e1a6fa98d199a81f0a0cb9 Mon Sep 17 00:00:00 2001 From: auxten Date: Mon, 7 Apr 2025 05:40:40 +0000 Subject: [PATCH 2/2] Fix libchdb version to v2.1.1 until libchdb v3 based chdb-rust refactor --- .github/workflows/rust.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index ca00651..f3e1160 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -16,7 +16,9 @@ jobs: - uses: actions/checkout@v3 - name: Fetch library run: | - wget https://github.com/chdb-io/chdb/releases/latest/download/linux-x86_64-libchdb.tar.gz + #wget https://github.com/chdb-io/chdb/releases/latest/download/linux-x86_64-libchdb.tar.gz + #Fix libchdb version to v2.1.1 until libchdb v3 based chdb-rust refactor + wget https://github.com/chdb-io/chdb/releases/download/v2.1.1/linux-x86_64-libchdb.tar.gz tar -xzf linux-x86_64-libchdb.tar.gz sudo mv libchdb.so /usr/lib/libchdb.so sudo ldconfig