From 728735513342ed74d997897811e17a7984092f52 Mon Sep 17 00:00:00 2001
From: Carson <cpsievert1@gmail.com>
Date: Thu, 14 May 2026 14:32:30 -0500
Subject: [PATCH] fix: unquote SQL identifiers in VISUALIZE column references

Quoted identifiers like "variable.dotted" were stored with their
surrounding double-quotes intact, causing validation to fail when
matching against Arrow schema column names (which are unquoted).

Promotes the private unquote helper from stat_aggregate to a public
naming::unquote_ident and applies it at parse time in builder.rs.

Fixes posit-dev/ggsql-python#3
---
 src/naming.rs                         | 33 +++++++++++++++++++++++++++
 src/parser/builder.rs                 |  6 ++---
 src/plot/layer/geom/stat_aggregate.rs |  3 +--
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/src/naming.rs b/src/naming.rs
index 5aca72937..46ce8f8fb 100644
--- a/src/naming.rs
+++ b/src/naming.rs
@@ -240,6 +240,23 @@ pub fn quote_ident(name: &str) -> String {
     format!("\"{}\"", name.replace('"', "\"\""))
 }
 
+/// Strip surrounding double-quotes from a SQL quoted identifier and unescape
+/// doubled quotes (`""` → `"`). Returns the input unchanged if it isn't quoted.
+///
+/// # Example
+/// ```
+/// use ggsql::naming;
+/// assert_eq!(naming::unquote_ident("\"variable.dotted\""), "variable.dotted");
+/// assert_eq!(naming::unquote_ident("\"has\"\"quote\""), "has\"quote");
+/// assert_eq!(naming::unquote_ident("plain"), "plain");
+/// ```
+pub fn unquote_ident(name: &str) -> String {
+    match name.strip_prefix('"').and_then(|s| s.strip_suffix('"')) {
+        Some(inner) => inner.replace("\"\"", "\""),
+        None => name.to_string(),
+    }
+}
+
 /// Quote a SQL string literal: wraps in single quotes and escapes embedded
 /// single quotes by doubling them, per the SQL standard.
 ///
@@ -498,6 +515,22 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_unquote_ident() {
+        assert_eq!(unquote_ident("\"variable.dotted\""), "variable.dotted");
+        assert_eq!(unquote_ident("\"has\"\"quote\""), "has\"quote");
+        assert_eq!(unquote_ident("plain"), "plain");
+        assert_eq!(unquote_ident("\"simple\""), "simple");
+    }
+
+    #[test]
+    fn test_quote_unquote_roundtrip() {
+        let names = vec!["hello", "has.dot", "has\"quote", "__ggsql_aes_x__"];
+        for name in names {
+            assert_eq!(unquote_ident(&quote_ident(name)), name);
+        }
+    }
+
     #[test]
     fn test_prefixes_built_from_components() {
         // Verify prefixes are correctly composed from building blocks
diff --git a/src/parser/builder.rs b/src/parser/builder.rs
index 3d5498fa4..7bd5c66ce 100644
--- a/src/parser/builder.rs
+++ b/src/parser/builder.rs
@@ -3,6 +3,7 @@
 //! Takes a tree-sitter parse tree and builds a typed Plot,
 //! handling all the node types defined in the grammar.
 
+use crate::naming;
 use crate::plot::layer::geom::Geom;
 use crate::plot::projection::resolve_coord;
 use crate::plot::scale::{color_to_hex, is_color_aesthetic, is_user_facet_aesthetic, Transform};
@@ -383,7 +384,7 @@ fn parse_mapping_element(node: &Node, source: &SourceTree, mappings: &mut Mappin
             mappings.insert(normalise_aes_name(&aesthetic), value);
         }
         "implicit_mapping" | "identifier" => {
-            let name = source.get_text(&child);
+            let name = naming::unquote_ident(&source.get_text(&child));
             mappings.insert(
                 normalise_aes_name(&name),
                 AestheticValue::standard_column(&name),
@@ -415,8 +416,7 @@ fn parse_explicit_mapping(node: &Node, source: &SourceTree) -> Result<(String, A
 
     let value = match value_child.kind() {
         "column_reference" => {
-            // column_reference is just an identifier wrapper, get its text directly
-            AestheticValue::standard_column(source.get_text(&value_child))
+            AestheticValue::standard_column(naming::unquote_ident(&source.get_text(&value_child)))
         }
         "literal_value" => parse_literal_value(&value_child, source)?,
         _ => {
diff --git a/src/plot/layer/geom/stat_aggregate.rs b/src/plot/layer/geom/stat_aggregate.rs
index 312f68d49..2db7240c0 100644
--- a/src/plot/layer/geom/stat_aggregate.rs
+++ b/src/plot/layer/geom/stat_aggregate.rs
@@ -518,8 +518,7 @@ fn simple_needs_fallback(name: &str, probe_col: &str, dialect: &dyn SqlDialect)
 }
 
 fn unquote(qcol: &str) -> String {
-    let trimmed = qcol.trim_start_matches('"').trim_end_matches('"');
-    trimmed.replace("\"\"", "\"")
+    naming::unquote_ident(qcol)
 }
 
 // =============================================================================