add test coverage for read_file_slice_from_paths API

Author: yunchipang

crates/core/src/file_group/reader.rs

@@ -486,6 +486,7 @@ mod tests {
         let base_uri = get_base_uri_with_valid_props_minimum();
         let reader = FileGroupReader::new_with_options(&base_uri, empty_options())?;
 
+        // Test with actual test files and empty log files - should trigger base_file_only logic
         let base_file_path = TEST_SAMPLE_BASE_FILE;
         let log_file_paths = vec![];
 
@@ -507,22 +508,26 @@ mod tests {
     }
 
     #[test]
-    fn test_read_file_slice_from_paths_with_log_files() -> Result<()> {
+    fn test_read_file_slice_from_paths_read_optimized_mode() -> Result<()> {
         let base_uri = get_base_uri_with_valid_props_minimum();
-        let reader = FileGroupReader::new_with_options(&base_uri, empty_options())?;
+        let reader = FileGroupReader::new_with_options(
+            &base_uri,
+            [(HudiReadConfig::UseReadOptimizedMode.as_ref(), "true")],
+        )?;
 
         let base_file_path = TEST_SAMPLE_BASE_FILE;
         let log_file_paths = vec![TEST_SAMPLE_LOG_FILE.to_string()];
 
         let result = reader.read_file_slice_from_paths_blocking(base_file_path, log_file_paths);
 
-        // The actual file reading might fail due to missing test data, which is expected
+        // In read-optimized mode, log files should be ignored
+        // This should behave the same as read_file_slice_by_base_file_path
         match result {
-            Ok(_batch) => {
-                // Test passes if we get a valid batch
+            Ok(_) => {
+                // Test passes if we get a result - the method correctly ignored log files
             }
             Err(e) => {
-                // Expected for missing test data - verify it's a storage/file not found error
+                // Expected for missing test data
                 let error_msg = e.to_string();
                 assert!(
                     error_msg.contains("not found") || error_msg.contains("No such file"),
@@ -536,26 +541,22 @@ mod tests {
     }
 
     #[test]
-    fn test_read_file_slice_from_paths_read_optimized_mode() -> Result<()> {
+    fn test_read_file_slice_from_paths_with_log_files() -> Result<()> {
         let base_uri = get_base_uri_with_valid_props_minimum();
-        let reader = FileGroupReader::new_with_options(
-            &base_uri,
-            [(HudiReadConfig::UseReadOptimizedMode.as_ref(), "true")],
-        )?;
+        let reader = FileGroupReader::new_with_options(&base_uri, empty_options())?;
 
         let base_file_path = TEST_SAMPLE_BASE_FILE;
         let log_file_paths = vec![TEST_SAMPLE_LOG_FILE.to_string()];
 
         let result = reader.read_file_slice_from_paths_blocking(base_file_path, log_file_paths);
 
-        // In read-optimized mode, log files should be ignored
-        // This should behave the same as read_file_slice_by_base_file_path
+        // The actual file reading might fail due to missing test data, which is expected
         match result {
-            Ok(_) => {
-                // Test passes if we get a result - the method correctly ignored log files
+            Ok(_batch) => {
+                // Test passes if we get a valid batch
             }
             Err(e) => {
-                // Expected for missing test data
+                // Expected for missing test data - verify it's a storage/file not found error
                 let error_msg = e.to_string();
                 assert!(
                     error_msg.contains("not found") || error_msg.contains("No such file"),
@@ -613,4 +614,52 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn test_read_file_slice_from_paths_read_optimized_forces_base_only() -> Result<()> {
+        let base_uri = get_base_uri_with_valid_props_minimum();
+        let reader = FileGroupReader::new_with_options(
+            &base_uri,
+            [(HudiReadConfig::UseReadOptimizedMode.as_ref(), "true")],
+        )?;
+
+        let base_file_path = "test.parquet";
+        // Even with log files provided, read-optimized mode should ignore them
+        let log_paths = vec!["log1.log".to_string(), "log2.log".to_string()];
+
+        let result = reader.read_file_slice_from_paths_blocking(base_file_path, log_paths);
+
+        assert!(result.is_err());
+        let error_msg = result.unwrap_err().to_string();
+        assert!(error_msg.contains("Failed to read path"));
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_read_file_slice_from_paths_with_non_empty_logs_attempts_merge() -> Result<()> {
+        let base_uri = get_base_uri_with_valid_props_minimum();
+        // Explicitly disable read-optimized mode to force merge path
+        let reader = FileGroupReader::new_with_options(
+            &base_uri,
+            [(HudiReadConfig::UseReadOptimizedMode.as_ref(), "false")],
+        )?;
+
+        let base_file_path = "test.parquet";
+        let log_paths = vec!["log1.log".to_string()];
+
+        let result = reader.read_file_slice_from_paths_blocking(base_file_path, log_paths);
+
+        // We expect this to fail, but it should exercise the merge logic path
+        assert!(result.is_err());
+        // The error could be either base file not found or log scanner issues
+        let error_msg = result.unwrap_err().to_string();
+        assert!(
+            error_msg.contains("Failed to read path")
+                || error_msg.contains("not found")
+                || error_msg.contains("No such file")
+        );
+
+        Ok(())
+    }
 }

python/tests/test_file_group_read.py

@@ -20,7 +20,10 @@
 from hudi import HudiFileGroupReader
 
 TEST_SAMPLE_BASE_FILE = "san_francisco/780b8586-3ad0-48ef-a6a1-d2217845ce4a-0_0-8-0_20240402123035233.parquet"
-TEST_SAMPLE_LOG_FILE = ".780b8586-3ad0-48ef-a6a1-d2217845ce4a-0_20240402123035233.log.1_0-8-0"
+TEST_SAMPLE_LOG_FILE = (
+    ".780b8586-3ad0-48ef-a6a1-d2217845ce4a-0_20240402123035233.log.1_0-8-0"
+)
+
 
 def test_file_group_api_read_file_slice(get_sample_table):
     table_path = get_sample_table
@@ -45,19 +48,22 @@ def test_file_group_api_read_file_slice_from_paths(get_sample_table):
 
     batch = file_group_reader.read_file_slice_from_paths(TEST_SAMPLE_BASE_FILE, [])
     assert batch.num_rows > 0
-    
-    batch_original = file_group_reader.read_file_slice_by_base_file_path(TEST_SAMPLE_BASE_FILE)
+
+    batch_original = file_group_reader.read_file_slice_by_base_file_path(
+        TEST_SAMPLE_BASE_FILE
+    )
     assert batch.num_rows == batch_original.num_rows
     assert batch.num_columns == batch_original.num_columns
-    
+
     t_new = pa.Table.from_batches([batch]).select([0, 5, 6, 9]).sort_by("ts")
-    t_original = pa.Table.from_batches([batch_original]).select([0, 5, 6, 9]).sort_by("ts")
+    t_original = (
+        pa.Table.from_batches([batch_original]).select([0, 5, 6, 9]).sort_by("ts")
+    )
     assert t_new.to_pylist() == t_original.to_pylist()
-    
+
     try:
         batch_with_logs = file_group_reader.read_file_slice_from_paths(
-            TEST_SAMPLE_BASE_FILE, 
-            [TEST_SAMPLE_LOG_FILE]
+            TEST_SAMPLE_BASE_FILE, [TEST_SAMPLE_LOG_FILE]
         )
         assert batch_with_logs.num_rows >= 0
     except Exception: