From c0ba8de5f2744cf9dcb5d9c0b05be7f0cffb7a2b Mon Sep 17 00:00:00 2001
From: Andrew Lamb <andrew@nerdnetworks.org>
Date: Fri, 29 Nov 2024 07:22:58 -0500
Subject: [PATCH] Improve unsupported compound identifier message

---
 datafusion/sql/src/planner.rs                 | 45 ++++++++++++++-----
 datafusion/sqllogictest/test_files/errors.slt |  2 +-
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs
index ccb2ccf7126f..382c503154dd 100644
--- a/datafusion/sql/src/planner.rs
+++ b/datafusion/sql/src/planner.rs
@@ -622,24 +622,41 @@ pub fn object_name_to_table_reference(
     idents_to_table_reference(idents, enable_normalization)
 }
 
+struct IdentTaker(Vec<Ident>);
+/// Take the next identifier from the back of idents, panic'ing if
+/// there are none left
+impl IdentTaker {
+    fn take(&mut self, enable_normalization: bool) -> String {
+        let ident = self.0.pop().expect("no more identifiers");
+        IdentNormalizer::new(enable_normalization).normalize(ident)
+    }
+}
+
+// impl Display for a nicer error message
+impl std::fmt::Display for IdentTaker {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let mut first = true;
+        for ident in self.0.iter() {
+            if !first {
+                write!(f, ".")?;
+            }
+            write!(f, "{}", ident)?;
+            first = false;
+        }
+
+        Ok(())
+    }
+}
+
 /// Create a [`TableReference`] after normalizing the specified identifier
 pub(crate) fn idents_to_table_reference(
     idents: Vec<Ident>,
     enable_normalization: bool,
 ) -> Result<TableReference> {
-    struct IdentTaker(Vec<Ident>);
-    /// Take the next identifier from the back of idents, panic'ing if
-    /// there are none left
-    impl IdentTaker {
-        fn take(&mut self, enable_normalization: bool) -> String {
-            let ident = self.0.pop().expect("no more identifiers");
-            IdentNormalizer::new(enable_normalization).normalize(ident)
-        }
-    }
-
     let mut taker = IdentTaker(idents);
+    let num_idents = taker.0.len();
 
-    match taker.0.len() {
+    match num_idents {
         1 => {
             let table = taker.take(enable_normalization);
             Ok(TableReference::bare(table))
@@ -655,7 +672,11 @@ pub(crate) fn idents_to_table_reference(
             let catalog = taker.take(enable_normalization);
             Ok(TableReference::full(catalog, schema, table))
         }
-        _ => plan_err!("Unsupported compound identifier '{:?}'", taker.0),
+        _ => plan_err!(
+            "Unsupported compound identifier '{}'. Expected 1, 2 or 3 parts, got {}",
+            taker,
+            num_idents
+        ),
     }
 }
 
diff --git a/datafusion/sqllogictest/test_files/errors.slt b/datafusion/sqllogictest/test_files/errors.slt
index 0dcac97a23aa..b99bbf3dc078 100644
--- a/datafusion/sqllogictest/test_files/errors.slt
+++ b/datafusion/sqllogictest/test_files/errors.slt
@@ -70,7 +70,7 @@ SELECT COUNT(*) FROM nonexistentschema.aggregate_test_100
 statement error Error during planning: table 'nonexistentcatalog\.public\.aggregate_test_100' not found
 SELECT COUNT(*) FROM nonexistentcatalog.public.aggregate_test_100
 
-statement error Error during planning: Unsupported compound identifier '\[Ident \{ value: "way", quote_style: None \}, Ident \{ value: "too", quote_style: None \}, Ident \{ value: "many", quote_style: None \}, Ident \{ value: "namespaces", quote_style: None \}, Ident \{ value: "as", quote_style: None \}, Ident \{ value: "ident", quote_style: None \}, Ident \{ value: "prefixes", quote_style: None \}, Ident \{ value: "aggregate_test_100", quote_style: None \}\]'
+statement error DataFusion error: Error during planning: Unsupported compound identifier 'way\.too\.many\.namespaces\.as\.ident\.prefixes\.aggregate_test_100'\. Expected 1, 2 or 3 parts, got 8
 SELECT COUNT(*) FROM way.too.many.namespaces.as.ident.prefixes.aggregate_test_100