From c66d84adec634f10200ae1307e72c7ced7489c83 Mon Sep 17 00:00:00 2001 From: Thomas Schmelzer Date: Thu, 8 Oct 2026 07:28:34 +0400 Subject: [PATCH 1/3] fix: replace unwrap-after-is_some and drop unreachable Trino match arm (#972) fed_rewriter::create_sources checked manual_info.is_some() and then unwrapped it; use if let instead. The Option producer in the Trino source already matches all six serde_json::Value variants, so the trailing _ arm could never run. Co-Authored-By: Claude Opus 5.5 --- connectorx/src/fed_rewriter.rs | 3 +-- connectorx/src/sources/trino/mod.rs | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/connectorx/src/fed_rewriter.rs b/connectorx/src/fed_rewriter.rs index 4038342111..44809d676c 100644 --- a/connectorx/src/fed_rewriter.rs +++ b/connectorx/src/fed_rewriter.rs @@ -88,8 +88,7 @@ fn create_sources( let db_manual = jvm.create_instance("java.util.HashMap", InvocationArg::empty())?; for (db_name, db_info) in db_map.iter() { - if db_info.manual_info.is_some() { - let manual_info = db_info.manual_info.as_ref().unwrap(); + if let Some(manual_info) = &db_info.manual_info { let schema_info = jvm.create_instance("java.util.HashMap", InvocationArg::empty())?; for (name, columns) in manual_info { let arr_instance = jvm.java_list("java.lang.String", columns.to_vec())?; diff --git a/connectorx/src/sources/trino/mod.rs b/connectorx/src/sources/trino/mod.rs index 891a887638..4c05d20052 100644 --- a/connectorx/src/sources/trino/mod.rs +++ b/connectorx/src/sources/trino/mod.rs @@ -562,7 +562,6 @@ macro_rules! impl_produce_text { .parse() .map_err(|_| anyhow!("Trino cannot convert complex value at ({}, {}): {:?}", ridx, cidx, value))?) } - _ => throw!(anyhow!("Trino unknown value at position: ({}, {}): {:?}", ridx, cidx, value)) } } } From dd22b12249e17c4e97306db47cf730539be8293a Mon Sep 17 00:00:00 2001 From: Thomas Schmelzer Date: Thu, 8 Oct 2026 07:28:34 +0400 Subject: [PATCH 2/3] chore: clear the remaining clippy warnings (#972) - Allow clippy::result_large_err crate-wide, with the reason: the Trino and BigQuery client errors are held by value and boxing them would change public error types. - dispatcher: //! instead of ///! for the module doc; name the prepare() return type as Prepared. - constants: drop single-condition cfg(any(..)). - mssql: Cow::into_owned instead of cloning the Cow; if let instead of single-arm match. - connectorx-cpp: From instead of Into; return the struct directly instead of via a let binding. Co-Authored-By: Claude Opus 5.5 --- connectorx-cpp/src/lib.rs | 6 ++--- connectorx-cpp/src/plan.rs | 13 +++++---- connectorx/src/constants.rs | 2 +- connectorx/src/dispatcher.rs | 27 +++++++++---------- connectorx/src/lib.rs | 5 ++++ connectorx/src/sources/mssql/tiberius_impl.rs | 24 ++++++++--------- 6 files changed, 38 insertions(+), 39 deletions(-) diff --git a/connectorx-cpp/src/lib.rs b/connectorx-cpp/src/lib.rs index a258c9d3b3..0c88602254 100644 --- a/connectorx-cpp/src/lib.rs +++ b/connectorx-cpp/src/lib.rs @@ -251,12 +251,10 @@ pub unsafe extern "C" fn connectorx_scan(conn: *const c_char, query: *const c_ch result.push(cx_rb); } - let res = CXResult { + CXResult { data: CXSlice::<_>::new_from_vec(result), header: CXSlice::<_>::new_from_vec(names), - }; - - res + } } #[repr(C)] diff --git a/connectorx-cpp/src/plan.rs b/connectorx-cpp/src/plan.rs index b1baf33b10..5a3a0ede80 100644 --- a/connectorx-cpp/src/plan.rs +++ b/connectorx-cpp/src/plan.rs @@ -1,22 +1,21 @@ use crate::CXFederatedPlan; use connectorx::fed_rewriter::Plan; use libc::c_char; -use std::convert::Into; use std::ffi::CString; -impl Into for Plan { - fn into(self) -> CXFederatedPlan { +impl From for CXFederatedPlan { + fn from(plan: Plan) -> Self { CXFederatedPlan { - db_name: CString::new(self.db_name.as_str()) + db_name: CString::new(plan.db_name.as_str()) .expect("new CString error") .into_raw() as *const c_char, - db_alias: CString::new(self.db_alias.as_str()) + db_alias: CString::new(plan.db_alias.as_str()) .expect("new CString error") .into_raw() as *const c_char, - sql: CString::new(self.sql.as_str()) + sql: CString::new(plan.sql.as_str()) .expect("new CString error") .into_raw() as *const c_char, - cardinality: self.cardinality, + cardinality: plan.cardinality, } } } diff --git a/connectorx/src/constants.rs b/connectorx/src/constants.rs index 4c926c2e19..e995ce8826 100644 --- a/connectorx/src/constants.rs +++ b/connectorx/src/constants.rs @@ -28,7 +28,7 @@ pub const RECORD_BATCH_SIZE: usize = 64 * KILO; ))] pub const DB_BUFFER_SIZE: usize = 32; -#[cfg(any(feature = "src_oracle"))] +#[cfg(feature = "src_oracle")] pub const ORACLE_ARRAY_SIZE: u32 = KILO as u32; #[cfg(all(not(debug_assertions), feature = "federation"))] diff --git a/connectorx/src/dispatcher.rs b/connectorx/src/dispatcher.rs index e02dae783a..4d9604d9d5 100644 --- a/connectorx/src/dispatcher.rs +++ b/connectorx/src/dispatcher.rs @@ -1,5 +1,5 @@ -///! This module provides [`dispatcher::Dispatcher`], the core struct in ConnectorX -///! that drives the data loading from a source to a destination. +//! This module provides [`dispatcher::Dispatcher`], the core struct in ConnectorX +//! that drives the data loading from a source to a destination. use crate::{ data_order::{coordinate, DataOrder}, destinations::{Destination, DestinationPartition}, @@ -13,6 +13,16 @@ use log::debug; use rayon::prelude::*; use std::marker::PhantomData; +/// What [`Dispatcher::prepare`] returns: the negotiated data order, the source and +/// destination partitions, and the source and destination schemas. +pub type Prepared<'w, S, D> = ( + DataOrder, + Vec<::Partition>, + Vec<::Partition<'w>>, + Vec<::TypeSystem>, + Vec<::TypeSystem>, +); + /// A dispatcher takes a `S: Source`, a `D: Destination`, a `TP: Transport` and a vector of `queries` as input to /// load data from `S` to `D` using the queries. pub struct Dispatcher<'a, S, D, TP> { @@ -47,18 +57,7 @@ where self.src.set_pre_execution_queries(pre_execution_queries); } - pub fn prepare( - mut self, - ) -> Result< - ( - DataOrder, - Vec, - Vec>, - Vec, - Vec, - ), - TP::Error, - > { + pub fn prepare(mut self) -> Result, TP::Error> { debug!("Prepare"); let dorder = coordinate(S::DATA_ORDERS, D::DATA_ORDERS)?; self.src.set_data_order(dorder)?; diff --git a/connectorx/src/lib.rs b/connectorx/src/lib.rs index 4c6ff56746..9bbf0d2720 100644 --- a/connectorx/src/lib.rs +++ b/connectorx/src/lib.rs @@ -1,5 +1,10 @@ #![allow(clippy::upper_case_acronyms)] #![warn(missing_docs)] +// The Trino and BigQuery client errors (232 and 176 bytes) are held by value in +// the source error enums and propagate into `ConnectorXOutError`. Boxing them +// would change public error types; the error path is cold, so the larger +// `Result` is not worth an API break. +#![allow(clippy::result_large_err)] //! # ConnectorX //! diff --git a/connectorx/src/sources/mssql/tiberius_impl.rs b/connectorx/src/sources/mssql/tiberius_impl.rs index 67a36521cf..0c482b99a8 100644 --- a/connectorx/src/sources/mssql/tiberius_impl.rs +++ b/connectorx/src/sources/mssql/tiberius_impl.rs @@ -61,7 +61,7 @@ pub fn mssql_config(url: &Url) -> Config { } config.port(url.port().unwrap_or(1433)); // remove the leading "/" - config.database(decode(&url.path()[1..])?.to_owned()); + config.database(decode(&url.path()[1..])?.into_owned()); // Using SQL Server authentication. #[allow(unused)] let params: HashMap = url.query_pairs().into_owned().collect(); @@ -75,15 +75,15 @@ pub fn mssql_config(url: &Url) -> Config { _ => { debug!("mssql auth through sqlserver authentication"); config.authentication(AuthMethod::sql_server( - decode(url.username())?.to_owned(), - decode(url.password().unwrap_or(""))?.to_owned(), + decode(url.username())?.into_owned(), + decode(url.password().unwrap_or(""))?.into_owned(), )); } }; #[cfg(all(not(windows), not(feature = "integrated-auth-gssapi")))] config.authentication(AuthMethod::sql_server( - decode(url.username())?.to_owned(), - decode(url.password().unwrap_or(""))?.to_owned(), + decode(url.username())?.into_owned(), + decode(url.password().unwrap_or(""))?.into_owned(), )); match params.get("trust_server_certificate") { @@ -91,10 +91,9 @@ pub fn mssql_config(url: &Url) -> Config { _ => {} }; - match params.get("trust_server_certificate_ca") { - Some(v) => config.trust_cert_ca(v), - _ => {} - }; + if let Some(v) = params.get("trust_server_certificate_ca") { + config.trust_cert_ca(v); + } match params.get("encrypt") { Some(v) if v.to_lowercase() == "true" => config.encryption(EncryptionLevel::Required), @@ -102,10 +101,9 @@ pub fn mssql_config(url: &Url) -> Config { _ => config.encryption(EncryptionLevel::NotSupported), }; - match params.get("appname") { - Some(appname) => config.application_name(decode(appname)?.to_owned()), - _ => {} - }; + if let Some(appname) = params.get("appname") { + config.application_name(decode(appname)?.into_owned()); + } config } From 8085315afcc3634b3990cfa328dc4f8d1c551438 Mon Sep 17 00:00:00 2001 From: Thomas Schmelzer Date: Thu, 8 Oct 2026 07:28:34 +0400 Subject: [PATCH 3/3] ci: fail clippy on warnings (#972) Add -D warnings to both clippy steps. missing_docs stays a warning via --force-warn while its backlog (enabled in #986) is worked down. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d3d6628bd..d2461b4a00 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,8 +44,11 @@ jobs: - name: Is the code formatted? run: cargo fmt --all -- --check -q + # -D warnings fails the build on any new warning. missing_docs is kept as a + # warning (lib.rs enables it to track the documentation backlog) until that + # backlog is cleared; --force-warn stops -D warnings from promoting it. - name: Clippy linting - run: cargo clippy --features all + run: cargo clippy --features all -- -D warnings --force-warn missing_docs - name: Check each feature gate run: just test-feature-gate @@ -180,7 +183,7 @@ jobs: key: ${{ runner.os }}-cargo-python-${{ matrix.python-version }}-${{ secrets.CACHE_VERSION }}-${{ hashFiles('**/Cargo.lock') }} - name: Clippy linting - run: cargo clippy --features all --all + run: cargo clippy --features all --all -- -D warnings --force-warn missing_docs - name: Cache venv uses: actions/cache@v6