alamb commented on code in PR #19519:
URL: https://github.com/apache/datafusion/pull/19519#discussion_r2666116159


##########
datafusion/sql/src/resolve.rs:
##########
@@ -193,19 +213,20 @@ pub fn resolve_table_references(
         relations: BTreeSet::new(),
         all_ctes: BTreeSet::new(),
         ctes_in_scope: vec![],
+        normalizer: IdentNormalizer::new(enable_ident_normalization),
     };
 
     visit_statement(statement, &mut visitor);
 
     let table_refs = visitor
         .relations
         .into_iter()
-        .map(|x| object_name_to_table_reference(x, enable_ident_normalization))
+        .map(|x| object_name_to_table_reference(x, false))

Review Comment:
   why is this always false now?



##########
datafusion/sql/src/resolve.rs:
##########
@@ -49,13 +49,31 @@ struct RelationVisitor {
     relations: BTreeSet<ObjectName>,
     all_ctes: BTreeSet<ObjectName>,
     ctes_in_scope: Vec<ObjectName>,
+    normalizer: IdentNormalizer,
 }
 
 impl RelationVisitor {
+    fn normalize_object_name(&self, name: &ObjectName) -> ObjectName {

Review Comment:
   Could you please add some comments that explain what this is doing and the 
rationale (the WHY)? I think you can adapt your PR description. 



##########
datafusion/sql/src/resolve.rs:
##########
@@ -270,4 +291,57 @@ mod tests {
         assert_eq!(ctes.len(), 1);
         assert_eq!(ctes[0].to_string(), "nodes");
     }
+
+    #[test]
+    fn resolve_table_references_cte_with_quoted_reference() {
+        use crate::parser::DFParser;
+
+        let query = r#"with barbaz as (select 1) select * from "barbaz""#;
+        let statement = 
DFParser::parse_sql(query).unwrap().pop_back().unwrap();
+        let (table_refs, ctes) = resolve_table_references(&statement, 
true).unwrap();
+        assert_eq!(ctes.len(), 1);
+        assert_eq!(ctes[0].to_string(), "barbaz");
+        // Quoted reference should still resolve to the CTE when normalization 
is on
+        assert_eq!(table_refs.len(), 0);
+    }
+
+    #[test]
+    fn resolve_table_references_cte_with_quoted_reference_normalization_off() {
+        use crate::parser::DFParser;
+
+        let query = r#"with barbaz as (select 1) select * from "barbaz""#;
+        let statement = 
DFParser::parse_sql(query).unwrap().pop_back().unwrap();
+        let (table_refs, ctes) = resolve_table_references(&statement, 
false).unwrap();
+        assert_eq!(ctes.len(), 1);
+        assert_eq!(ctes[0].to_string(), "barbaz");
+        // Even with normalization off, quoted reference matches same-case CTE 
name
+        assert_eq!(table_refs.len(), 0);
+    }
+
+    #[test]
+    fn 
resolve_table_references_cte_with_quoted_reference_uppercase_normalization_on() 
{
+        use crate::parser::DFParser;
+
+        let query = r#"with FOObar as (select 1) select * from "FOObar""#;
+        let statement = 
DFParser::parse_sql(query).unwrap().pop_back().unwrap();
+        let (table_refs, ctes) = resolve_table_references(&statement, 
true).unwrap();
+        // CTE name is normalized to lowercase, quoted reference preserves 
case, so they differ
+        assert_eq!(ctes.len(), 1);
+        assert_eq!(ctes[0].to_string(), "foobar");
+        assert_eq!(table_refs.len(), 1);
+        assert_eq!(table_refs[0].to_string(), "FOObar");
+    }
+
+    #[test]
+    fn 
resolve_table_references_cte_with_quoted_reference_uppercase_normalization_off()
 {
+        use crate::parser::DFParser;
+
+        let query = r#"with FOObar as (select 1) select * from "FOObar""#;
+        let statement = 
DFParser::parse_sql(query).unwrap().pop_back().unwrap();
+        let (table_refs, ctes) = resolve_table_references(&statement, 
false).unwrap();
+        // Without normalization, cases match exactly, so quoted reference 
resolves to the CTE
+        assert_eq!(ctes.len(), 1);
+        assert_eq!(ctes[0].to_string(), "FOObar");
+        assert_eq!(table_refs.len(), 0);
+    }

Review Comment:
   Can you also please add some .slt tests so we can see this working when 
running end to end in queries?
   
   Instructions are here: 
https://github.com/apache/datafusion/tree/main/datafusion/sqllogictest



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to