fix: Order by mentioning missing column multiple times (#13158)

* fix: Order by mentioning missing column multiple times

Previously we crashed on queries where the order by mentioned a missing
columns multiple times.

Closes #13157

* Use HashSet to avoid quadratic algorithm

* Use IndexSet to maintain old order and be deterministic

Author: eejbyfeldt

datafusion/expr/src/logical_plan/builder.rs

@@ -57,6 +57,7 @@ use datafusion_common::{
     UnnestOptions,
 };
 use datafusion_expr_common::type_coercion::binary::type_union_resolution;
+use indexmap::IndexSet;
 
 /// Default table name for unnamed table
 pub const UNNAMED_TABLE: &str = "?table?";
@@ -567,7 +568,7 @@ impl LogicalPlanBuilder {
     /// See <https://github.com/apache/datafusion/issues/5065> for more details
     fn add_missing_columns(
         curr_plan: LogicalPlan,
-        missing_cols: &[Column],
+        missing_cols: &IndexSet<Column>,
         is_distinct: bool,
     ) -> Result<LogicalPlan> {
         match curr_plan {
@@ -612,7 +613,7 @@ impl LogicalPlanBuilder {
 
     fn ambiguous_distinct_check(
         missing_exprs: &[Expr],
-        missing_cols: &[Column],
+        missing_cols: &IndexSet<Column>,
         projection_exprs: &[Expr],
     ) -> Result<()> {
         if missing_exprs.is_empty() {
@@ -677,15 +678,16 @@ impl LogicalPlanBuilder {
         let schema = self.plan.schema();
 
         // Collect sort columns that are missing in the input plan's schema
-        let mut missing_cols: Vec<Column> = vec![];
+        let mut missing_cols: IndexSet<Column> = IndexSet::new();
         sorts.iter().try_for_each::<_, Result<()>>(|sort| {
             let columns = sort.expr.column_refs();
 
-            columns.into_iter().for_each(|c| {
-                if !schema.has_column(c) {
-                    missing_cols.push(c.clone());
-                }
-            });
+            missing_cols.extend(
+                columns
+                    .into_iter()
+                    .filter(|c| !schema.has_column(c))
+                    .cloned(),
+            );
 
             Ok(())
         })?;

datafusion/sqllogictest/test_files/order.slt

@@ -335,6 +335,14 @@ select column1 from foo order by log(column2);
 3
 5
 
+# Test issue: https://github.com/apache/datafusion/issues/13157
+query I
+select column1 from foo order by column2 % 2, column2;
+----
+1
+3
+5
+
 # Cleanup
 statement ok
 drop table foo;