From d616a2064f1b7cd03858c5ab23c373a5ffdc8a32 Mon Sep 17 00:00:00 2001 From: jhw Date: Wed, 9 Sep 2026 13:49:00 +0800 Subject: [PATCH 1/2] feat(phase-2): restore Clippy compatibility --- crates/cli/src/commands/table/mod.rs | 8 ++++---- crates/s3/src/admin/catalog.rs | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/cli/src/commands/table/mod.rs b/crates/cli/src/commands/table/mod.rs index 5301818..1523c50 100644 --- a/crates/cli/src/commands/table/mod.rs +++ b/crates/cli/src/commands/table/mod.rs @@ -406,10 +406,10 @@ fn properties(values: Vec) -> Result> { Ok(result) } fn require_string(body: &Value, field: &str) -> Result<()> { - if !body + if body .get(field) .and_then(Value::as_str) - .is_some_and(|s| !s.trim().is_empty()) + .is_none_or(|s| s.trim().is_empty()) { return Err(Error::Config(format!("Request requires nonempty {field}"))); } @@ -577,10 +577,10 @@ fn prepare_table(command: TableCommands) -> Result { { return Err(Error::Config("Standard updates use Iceberg requirements; version/location guards require new-metadata-location".into())); } - if !body + if body .get("requirements") .and_then(Value::as_array) - .is_some_and(|v| !v.is_empty()) + .is_none_or(Vec::is_empty) { return Err(Error::Config( "Standard commit requires explicit Iceberg requirements".into(), diff --git a/crates/s3/src/admin/catalog.rs b/crates/s3/src/admin/catalog.rs index c021a84..3202332 100644 --- a/crates/s3/src/admin/catalog.rs +++ b/crates/s3/src/admin/catalog.rs @@ -302,8 +302,7 @@ impl AdminClient { serde_json::from_slice(&bytes) .map_err(|_| Error::General("Invalid catalog JSON response".into()))? }; - if !value.is_object() - && !(request.operation == Op::MaintenanceConfigShow && value.is_null()) + if !(value.is_object() || request.operation == Op::MaintenanceConfigShow && value.is_null()) { return Err(Error::General("Catalog response must be an object".into())); } From 10353c15bd9498aa2cd9dea85431f94b664820f7 Mon Sep 17 00:00:00 2001 From: jhw Date: Wed, 9 Sep 2026 14:10:44 +0800 Subject: [PATCH 2/2] feat(phase-2): align Select scan ranges with RustFS --- crates/cli/src/commands/sql.rs | 58 ++++++++++++------- crates/core/src/lib.rs | 4 +- crates/core/src/select.rs | 101 +++++++++++++++++++++++++++++++++ crates/s3/src/select.rs | 87 ++++++++++++++++++++-------- 4 files changed, 204 insertions(+), 46 deletions(-) diff --git a/crates/cli/src/commands/sql.rs b/crates/cli/src/commands/sql.rs index 797793b..a346faf 100644 --- a/crates/cli/src/commands/sql.rs +++ b/crates/cli/src/commands/sql.rs @@ -343,26 +343,16 @@ fn validate_record_delimiter(name: &str, value: Option<&str>) -> std::result::Re } fn validate_scan_range_args(args: &SqlArgs) -> std::result::Result<(), String> { - if args.scan_start.is_none() && args.scan_end.is_none() { - return Ok(()); + SelectScanRangeOptions { + start: args.scan_start, + end: args.scan_end, } - if matches!(args.input_format, InputFormatArg::Parquet) { - return Err("ScanRange is not supported for Parquet input".to_string()); - } - if matches!(args.input_format, InputFormatArg::Json) - && matches!(args.json_type, JsonTypeArg::Document) - { - return Err("ScanRange is not supported for JSON document input".to_string()); - } - if args.scan_start.is_some_and(|start| start < 0) || args.scan_end.is_some_and(|end| end < 0) { - return Err("ScanRange start and end must be non-negative".to_string()); - } - if let (Some(start), Some(end)) = (args.scan_start, args.scan_end) - && start > end - { - return Err("ScanRange start must not be greater than end".to_string()); - } - Ok(()) + .validate_for_input( + args.input_format.into(), + args.json_type.into(), + args.compression.into(), + ) + .map_err(|error| error.to_string()) } fn exit_code_from_error(error: &rc_core::Error) -> ExitCode { @@ -443,6 +433,36 @@ mod tests { assert_eq!(code, ExitCode::UsageError); } + #[test] + fn sql_allows_scan_range_for_parquet() { + let mut args = base_args("a/b/object.parquet", "SELECT * FROM S3Object"); + args.input_format = InputFormatArg::Parquet; + args.scan_start = Some(1024); + args.scan_end = Some(2047); + + assert!(validate_scan_range_args(&args).is_ok()); + } + + #[test] + fn sql_rejects_compressed_scan_range() { + let mut args = base_args("a/b/object.csv.gz", "SELECT * FROM S3Object"); + args.compression = CompressionArg::Gzip; + args.scan_start = Some(1); + + let error = validate_scan_range_args(&args) + .expect_err("compressed input should reject a non-noop scan range"); + assert!(error.contains("compressed input")); + } + + #[test] + fn sql_allows_noop_compressed_scan_range() { + let mut args = base_args("a/b/object.csv.bz2", "SELECT * FROM S3Object"); + args.compression = CompressionArg::Bzip2; + args.scan_start = Some(0); + + assert!(validate_scan_range_args(&args).is_ok()); + } + #[test] fn sql_exit_code_from_backend_errors() { let cases = [ diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index e70e225..88c6ac3 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -70,8 +70,8 @@ pub use retry::{RetryBuilder, is_retryable_error, retry_with_backoff}; pub use select::{ SelectCompression, SelectCsvFileHeaderInfo, SelectCsvInputOptions, SelectCsvOutputOptions, SelectInputFormat, SelectJsonInputOptions, SelectJsonInputType, SelectJsonOutputOptions, - SelectOptions, SelectOutputFormat, SelectQuoteFields, SelectScanRangeOptions, - SelectSseCustomerOptions, + SelectOptions, SelectOutputFormat, SelectQuoteFields, SelectScanRangeError, + SelectScanRangeOptions, SelectSseCustomerOptions, }; pub use traits::{ AbortMultipartUploadRequest, BucketNotification, Capabilities, CopyObjectOptions, diff --git a/crates/core/src/select.rs b/crates/core/src/select.rs index c384058..b11f1f3 100644 --- a/crates/core/src/select.rs +++ b/crates/core/src/select.rs @@ -1,5 +1,7 @@ //! S3 Select domain types (no AWS SDK types). +use thiserror::Error; + /// Object payload format for S3 Select input. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum SelectInputFormat { @@ -90,6 +92,51 @@ pub struct SelectScanRangeOptions { pub end: Option, } +/// Invalid combinations or values for an S3 Select scan range. +#[derive(Debug, Clone, PartialEq, Eq, Error)] +pub enum SelectScanRangeError { + #[error("ScanRange is not supported for JSON document input")] + JsonDocument, + #[error("ScanRange is not supported for compressed input")] + CompressedInput, + #[error("ScanRange start and end must be non-negative")] + NegativeBounds, + #[error("ScanRange start must not be greater than end")] + ReversedBounds, +} + +impl SelectScanRangeOptions { + /// Validate this range against the selected input serialization. + pub fn validate_for_input( + &self, + input_format: SelectInputFormat, + json_input_type: SelectJsonInputType, + compression: SelectCompression, + ) -> std::result::Result<(), SelectScanRangeError> { + if self.start.is_none() && self.end.is_none() { + return Ok(()); + } + if matches!(input_format, SelectInputFormat::Json) + && matches!(json_input_type, SelectJsonInputType::Document) + { + return Err(SelectScanRangeError::JsonDocument); + } + let is_noop = self.start == Some(0) && self.end.is_none(); + if !matches!(compression, SelectCompression::None) && !is_noop { + return Err(SelectScanRangeError::CompressedInput); + } + if self.start.is_some_and(|start| start < 0) || self.end.is_some_and(|end| end < 0) { + return Err(SelectScanRangeError::NegativeBounds); + } + if let (Some(start), Some(end)) = (self.start, self.end) + && start > end + { + return Err(SelectScanRangeError::ReversedBounds); + } + Ok(()) + } +} + /// SSE-C parameters for encrypted objects. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct SelectSseCustomerOptions { @@ -130,3 +177,57 @@ impl Default for SelectOptions { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn scan_range_allows_parquet_input() { + let range = SelectScanRangeOptions { + start: Some(1024), + end: Some(2047), + }; + + range + .validate_for_input( + SelectInputFormat::Parquet, + SelectJsonInputType::Lines, + SelectCompression::None, + ) + .expect("Parquet scan ranges should be supported"); + } + + #[test] + fn scan_range_rejects_compressed_input() { + let range = SelectScanRangeOptions { + start: Some(1), + end: None, + }; + + assert_eq!( + range.validate_for_input( + SelectInputFormat::Csv, + SelectJsonInputType::Lines, + SelectCompression::Gzip, + ), + Err(SelectScanRangeError::CompressedInput) + ); + } + + #[test] + fn scan_range_allows_noop_for_compressed_input() { + let range = SelectScanRangeOptions { + start: Some(0), + end: None, + }; + + range + .validate_for_input( + SelectInputFormat::Csv, + SelectJsonInputType::Lines, + SelectCompression::Bzip2, + ) + .expect("RustFS accepts a no-op scan range for compressed input"); + } +} diff --git a/crates/s3/src/select.rs b/crates/s3/src/select.rs index c46dac6..bda6f39 100644 --- a/crates/s3/src/select.rs +++ b/crates/s3/src/select.rs @@ -101,31 +101,13 @@ fn build_scan_range(options: &SelectOptions) -> Result> { if scan_range.start.is_none() && scan_range.end.is_none() { return Ok(None); } - if matches!(options.input_format, SelectInputFormat::Parquet) { - return Err(Error::General( - "ScanRange is not supported for Parquet input.".to_string(), - )); - } - if matches!(options.input_format, SelectInputFormat::Json) - && matches!(options.json_input.input_type, SelectJsonInputType::Document) - { - return Err(Error::General( - "ScanRange is not supported for JSON document input.".to_string(), - )); - } - if scan_range.start.is_some_and(|start| start < 0) || scan_range.end.is_some_and(|end| end < 0) - { - return Err(Error::General( - "ScanRange start and end must be non-negative.".to_string(), - )); - } - if let (Some(start), Some(end)) = (scan_range.start, scan_range.end) - && start > end - { - return Err(Error::General( - "ScanRange start must not be greater than end.".to_string(), - )); - } + scan_range + .validate_for_input( + options.input_format, + options.json_input.input_type, + options.compression, + ) + .map_err(|error| Error::General(error.to_string()))?; Ok(Some( ScanRange::builder() .set_start(scan_range.start) @@ -630,6 +612,61 @@ mod tests { assert!(matches!(error, Error::General(msg) if msg.contains("JSON document"))); } + #[test] + fn scan_range_allows_parquet_input() { + let options = SelectOptions { + expression: "SELECT * FROM S3Object".to_string(), + input_format: SelectInputFormat::Parquet, + scan_range: SelectScanRangeOptions { + start: Some(1024), + end: Some(2047), + }, + ..SelectOptions::default() + }; + + let scan_range = build_scan_range(&options) + .expect("Parquet scan range should be valid") + .expect("scan range should be configured"); + assert_eq!(scan_range.start(), Some(1024)); + assert_eq!(scan_range.end(), Some(2047)); + } + + #[test] + fn scan_range_rejects_compressed_input() { + let options = SelectOptions { + expression: "SELECT * FROM S3Object".to_string(), + compression: SelectCompression::Gzip, + scan_range: SelectScanRangeOptions { + start: Some(1), + end: None, + }, + ..SelectOptions::default() + }; + + let error = build_scan_range(&options) + .expect_err("compressed input should reject a non-noop scan range"); + assert!(matches!(error, Error::General(message) if message.contains("compressed input"))); + } + + #[test] + fn scan_range_allows_noop_for_compressed_input() { + let options = SelectOptions { + expression: "SELECT * FROM S3Object".to_string(), + compression: SelectCompression::Bzip2, + scan_range: SelectScanRangeOptions { + start: Some(0), + end: None, + }, + ..SelectOptions::default() + }; + + assert!( + build_scan_range(&options) + .expect("no-op compressed scan range should be valid") + .is_some() + ); + } + #[test] fn scan_range_rejects_start_after_end() { let options = SelectOptions {