refactor

Author: adriangb

datafusion/physical-plan/src/topk/mod.rs

@@ -347,61 +347,59 @@ impl TopK {
 
         // Are the new thresholds more selective than our existing ones?
         let should_update = {
-            if let Some(current) = self.filter.thresholds.write().as_mut() {
+            let mut current = self.filter.thresholds.write();
+            if let Some(current) = current.as_mut() {
                 assert!(current.len() == thresholds.len());
                 // Check if new thresholds are more selective than current ones
                 let mut more_selective = false;
                 for ((current_value, new_value), sort_expr) in
                     current.iter().zip(thresholds.iter()).zip(self.expr.iter())
                 {
-                    // Handle null cases
-                    let (current_is_null, new_is_null) =
-                        (current_value.is_null(), new_value.is_null());
-
-                    match (current_is_null, new_is_null) {
-                        (true, true) => {
-                            // Both null, continue checking next values
-                        }
-                        (true, false) => {
-                            // Current is null, new is not null
-                            // For nulls_first: null < non-null, so new value is less selective
-                            // For nulls_last: null > non-null, so new value is more selective
-                            more_selective = !sort_expr.options.nulls_first;
-                            break;
-                        }
-                        (false, true) => {
-                            // Current is not null, new is null
-                            // For nulls_first: non-null > null, so new value is more selective
-                            // For nulls_last: non-null < null, so new value is less selective
-                            more_selective = sort_expr.options.nulls_first;
-                            break;
+                    match current_value.partial_cmp(new_value) {
+                        Some(ordering) => {
+                            match ordering {
+                                Ordering::Equal => {
+                                    // Continue checking next values
+                                }
+                                Ordering::Less => {
+                                    // For descending sort: new > current means more selective
+                                    // For ascending sort: new > current means less selective
+                                    more_selective = sort_expr.options.descending;
+                                    break;
+                                }
+                                Ordering::Greater => {
+                                    // For descending sort: new < current means less selective
+                                    // For ascending sort: new < current means more selective
+                                    more_selective = !sort_expr.options.descending;
+                                    break;
+                                }
+                            }
                         }
-                        (false, false) => {
-                            // Neither is null, compare values
-                            match current_value.partial_cmp(new_value) {
-                                Some(ordering) => {
-                                    match ordering {
-                                        Ordering::Equal => {
-                                            // Continue checking next values
-                                        }
-                                        Ordering::Less => {
-                                            // For descending sort: new > current means more selective
-                                            // For ascending sort: new > current means less selective
-                                            more_selective = sort_expr.options.descending;
-                                            break;
-                                        }
-                                        Ordering::Greater => {
-                                            // For descending sort: new < current means less selective
-                                            // For ascending sort: new < current means more selective
-                                            more_selective =
-                                                !sort_expr.options.descending;
-                                            break;
-                                        }
-                                    }
+                        None => {
+                            // One of the values is null or not comparable
+                            let current_is_null = current_value.is_null();
+                            let new_is_null = new_value.is_null();
+                            match (current_is_null, new_is_null) {
+                                (true, true) => {
+                                    // Both null, continue checking next values
+                                }
+                                (true, false) => {
+                                    // Current is null, new is not null
+                                    // For nulls_first: null < non-null, so new value is less selective
+                                    // For nulls_last: null > non-null, so new value is more selective
+                                    more_selective = !sort_expr.options.nulls_first;
+                                    break;
+                                }
+                                (false, true) => {
+                                    // Current is not null, new is null
+                                    // For nulls_first: non-null > null, so new value is more selective
+                                    // For nulls_last: non-null < null, so new value is less selective
+                                    more_selective = sort_expr.options.nulls_first;
+                                    break;
                                 }
-                                None => {
-                                    // If values can't be compared, don't update
-                                    more_selective = false;
+                                (false, false) => {
+                                    // Neither is null, we can't compare them
+                                    // This means we can't determine selectivity, so we assume that the new filter is more selective
                                     break;
                                 }
                             }
@@ -415,6 +413,7 @@ impl TopK {
                 more_selective
             } else {
                 // No current thresholds, so update with the new ones
+                *current = Some(thresholds.clone());
                 true
             }
         };