fix: FieldNotFound error message without valid fields

apache · Jan 18, 2023 · 769fbab · 769fbab
1 parent 8ab3a91
commit 769fbab
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 17 deletions.
diff --git a/datafusion/common/src/column.rs b/datafusion/common/src/column.rs
@@ -148,12 +148,10 @@ impl Column {
 
         Err(DataFusionError::SchemaError(SchemaError::FieldNotFound {
             field: Column::new(self.relation.clone(), self.name),
-            valid_fields: Some(
-                schemas
-                    .iter()
-                    .flat_map(|s| s.fields().iter().map(|f| f.qualified_column()))
-                    .collect(),
-            ),
+            valid_fields: schemas
+                .iter()
+                .flat_map(|s| s.fields().iter().map(|f| f.qualified_column()))
+                .collect(),
         }))
     }
 }

diff --git a/datafusion/common/src/dfschema.rs b/datafusion/common/src/dfschema.rs
@@ -825,6 +825,21 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn select_without_valid_fields() {
+        let schema = DFSchema::empty();
+
+        let err = schema
+            .index_of_column_by_name(Some("t1"), "c0")
+            .err()
+            .unwrap();
+        assert_eq!("Schema error: No field named 't1'.'c0'.", &format!("{err}"));
+
+        // the same check without qualifier
+        let err = schema.index_of_column_by_name(None, "c0").err().unwrap();
+        assert_eq!("Schema error: No field named 'c0'.", &format!("{err}"));
+    }
+
     #[test]
     fn equivalent_names_and_types() {
         let field1_i16_t = DFField::from(Field::new("f1", DataType::Int16, true));

diff --git a/datafusion/common/src/error.rs b/datafusion/common/src/error.rs
@@ -125,7 +125,7 @@ pub enum SchemaError {
     /// No field with this name
     FieldNotFound {
         field: Column,
-        valid_fields: Option<Vec<Column>>,
+        valid_fields: Vec<Column>,
     },
 }
 
@@ -137,13 +137,11 @@ pub fn field_not_found(
 ) -> DataFusionError {
     DataFusionError::SchemaError(SchemaError::FieldNotFound {
         field: Column::new(qualifier, name),
-        valid_fields: Some(
-            schema
-                .fields()
-                .iter()
-                .map(|f| f.qualified_column())
-                .collect(),
-        ),
+        valid_fields: schema
+            .fields()
+            .iter()
+            .map(|f| f.qualified_column())
+            .collect(),
     })
 }
 
@@ -160,11 +158,11 @@ impl Display for SchemaError {
                 } else {
                     write!(f, "'{}'", field.name)?;
                 }
-                if let Some(fields) = valid_fields {
+                if !valid_fields.is_empty() {
                     write!(
                         f,
                         ". Valid fields are {}",
-                        fields
+                        valid_fields
                             .iter()
                             .map(|field| {
                                 if let Some(q) = &field.relation {

diff --git a/datafusion/sql/tests/integration_test.rs b/datafusion/sql/tests/integration_test.rs
@@ -3085,7 +3085,7 @@ fn test_prepare_statement_to_plan_panic_prepare_wrong_syntax() {
 
 #[test]
 #[should_panic(
-    expected = "value: SchemaError(FieldNotFound { field: Column { relation: None, name: \"id\" }, valid_fields: Some([]) })"
+    expected = "value: SchemaError(FieldNotFound { field: Column { relation: None, name: \"id\" }, valid_fields: [] })"
 )]
 fn test_prepare_statement_to_plan_panic_no_relation_and_constant_param() {
     let sql = "PREPARE my_plan(INT) AS SELECT id + $1";