refactor: address Edd's review comments

predicate/src/delete_predicate.rs

@@ -1,4 +1,4 @@
-use std::convert::TryInto;
+use std::{collections::BTreeSet, convert::TryInto};
 
 use chrono::DateTime;
 use data_types::timestamp::TimestampRange;
@@ -102,13 +102,13 @@ impl DeletePredicate {
     }
 
     /// Return all columns participating in the expressions of delete predicate minus the time column if any
-    pub fn all_column_names_but_time(&self) -> hashbrown::HashSet<&str> {
-        // Get column names of the predicate expressions
-        self.exprs
-            .iter()
-            .map(|e| e.column())
-            .filter(|col_name| col_name != &TIME_COLUMN_NAME)
-            .collect::<hashbrown::HashSet<&str>>()
+    pub fn all_column_names_but_time<'a>(&'a self, cols: &mut BTreeSet<&'a str>) {
+        for expr in &self.exprs {
+            let col = expr.column();
+            if col != TIME_COLUMN_NAME {
+                cols.insert(col);
+            }
+        }
     }
 }
 

query/src/exec/context.rs

@@ -255,7 +255,7 @@ impl IOxExecutionContext {
         debug!(text=%sql, "planning SQL query");
         let logical_plan = ctx.inner.create_logical_plan(sql)?;
         let s = logical_plan.display_graphviz().to_string();
-        debug!(?s, "logical plan");
+        debug!(plan=?s, "logical plan");
         ctx.prepare_plan(&logical_plan).await
     }
 

query/src/lib.rs

@@ -24,8 +24,8 @@ use predicate::{
     predicate::{Predicate, PredicateMatch},
 };
 
-use hashbrown::{HashMap, HashSet};
-use std::{fmt::Debug, iter::FromIterator, sync::Arc};
+use hashbrown::HashMap;
+use std::{collections::BTreeSet, fmt::Debug, iter::FromIterator, sync::Arc};
 
 pub mod exec;
 pub mod frontend;
@@ -61,19 +61,14 @@ pub trait QueryChunkMeta: Sized {
     /// This order is to be consistent with Schema::primary_key
     fn delete_predicate_columns(&self) -> Vec<&str> {
         // get all column names but time
-        let mut col_names: HashSet<&str> = hashbrown::HashSet::new();
+        let mut col_names = BTreeSet::new();
         for pred in self.delete_predicates() {
-            let cols = pred.all_column_names_but_time();
-            for col in cols {
-                col_names.insert(col);
-            }
+            //let cols = pred.all_column_names_but_time();
+            pred.all_column_names_but_time(&mut col_names);
         }
 
         // convert to vector
-        let mut column_names: Vec<&str> = Vec::from_iter(col_names);
-
-        // Sort it
-        column_names.sort_unstable();
+        let mut column_names = Vec::from_iter(col_names);
 
         // Now add time column to the end of the vector
         // Since time range is a must in the delete predicate, time column must be in this list