Fix renaming where only the schema changes

src/macaw/core.clj

@@ -160,9 +160,10 @@
     (-> (rewrite/replace-names sql' parsed renames' opts')
         (str/replace #"(?m)^ \n" "\n")
         (unescape-keywords (:non-reserved-words opts)))))
+
 (defn ->ast
-  "Given a sql query, return a clojure ast that represents it.
+  "Given an SQL query, return a clojure AST that represents it.
 
-   This ast can potentially be lossy and generally shouldn't be as part of a round trip back to sql."
+   This AST can potentially be lossy, and generally shouldn't be used as part of a round trip back to SQL."
   [parsed]
   (m.ast/->ast parsed {:with-instance? false}))

src/macaw/rewrite.clj

@@ -4,6 +4,7 @@
    [macaw.util :as u]
    [macaw.walk :as mw])
   (:import
+   (net.sf.jsqlparser.expression Alias)
    (net.sf.jsqlparser.parser ASTNodeAccess SimpleNode)
    (net.sf.jsqlparser.schema Column Table)))
 
@@ -52,9 +53,9 @@
     (str sb)))
 
 (defn- update-query
-  "Emit a SQL string for an updated AST, preserving the comments and whitespace from the original SQL."
+  "Emit the SQL string for an updated AST, preserving the comments and whitespace from the original SQL."
   [updated-ast updated-nodes sql & {:as _opts}]
-  (let [updated-node? (set (map first updated-nodes))
+  (let [updated-node? (into #{} (map first) updated-nodes)
         replacement   (fn [->text visitable]
                         (let [ast-node  (.getASTNode ^ASTNodeAccess visitable)
                               idx-range (node->idx-range ast-node sql)
@@ -63,7 +64,9 @@
         replace-name  (fn [->text]
                         (fn [acc visitable _ctx]
                           (cond-> acc
-                            (updated-node? visitable)
+                            (or (updated-node? visitable)
+                                (when (instance? Column visitable)
+                                 (updated-node? (.getTable ^Column visitable))))
                             (conj (replacement ->text visitable)))))]
     (splice-replacements
      sql
@@ -79,28 +82,36 @@
 
 (defn- rename-table
   [updated-nodes table-renames schema-renames known-tables opts ^Table t _ctx]
-  (when-let [rename (u/find-relevant table-renames (get known-tables t) [:table :schema])]
-    ;; Handle both raw string renames, as well as more precise element based ones.
-    (vswap! updated-nodes conj [t rename])
-    (let [identifier (as-> (val rename) % (:table % %))]
-      (.setName t identifier)))
-  (let [raw-schema-name (.getSchemaName t)
-        schema-name     (collect/normalize-reference raw-schema-name opts)]
-    (when-let [schema-rename (u/seek (comp (partial u/match-component schema-name) key) schema-renames)]
-      (vswap! updated-nodes conj [raw-schema-name schema-rename])
-      (let [identifier (as-> (val schema-rename) % (:table % %))]
-        (.setSchemaName t identifier)))))
+  (let [kt      (get known-tables t)
+        aliases (into #{} (comp
+                           (keep #(.getAlias ^Table %))
+                           (map #(.getName ^Alias %)))
+                      (:instances kt))]
+    ;; Don't rename this node if it's pointing at an alias
+    ;; TODO (2025-11-26) this can have false negatives due to case or quoting
+    (when (or (.getAlias t) (not (aliases (.getName t))))
+      (when-let [rename (u/find-relevant table-renames (get known-tables t) [:table :schema])]
+        ;; Handle both raw string renames, and more precise element based ones.
+        (vswap! updated-nodes conj [t rename])
+        (let [identifier (as-> (val rename) % (:table % %))]
+          (.setName t identifier)))
+      (let [raw-schema-name (.getSchemaName t)
+            schema-name     (collect/normalize-reference raw-schema-name opts)]
+        (when-let [schema-rename (u/seek (comp (partial u/match-component schema-name) key) schema-renames)]
+          (vswap! updated-nodes conj [t schema-rename])
+          (let [identifier (as-> (val schema-rename) % (:table % %))]
+            (.setSchemaName t identifier)))))))
 
 (defn- rename-column
   [updated-nodes column-renames known-columns ^Column c _ctx]
   (when-let [rename (u/find-relevant column-renames (get known-columns c) [:column :table :schema])]
-    ;; Handle both raw string renames, as well as more precise element based ones.
+    ;; Handle both raw string renames, and more precise element based ones.
     (vswap! updated-nodes conj [c rename])
     (let [identifier (as-> (val rename) % (:column % %))]
       (.setColumnName c identifier))))
 
 (defn- alert-unused! [updated-nodes renames]
-  (let [known-rename? (set (map second updated-nodes))]
+  (let [known-rename? (into #{} (map second) updated-nodes)]
     (doseq [[k items] renames]
       (when-let [unknown (first (remove known-rename? items))]
         (throw (ex-info (str "Unknown rename: " unknown) {:type   k
@@ -113,7 +124,7 @@
              [i c])))
 
 (defn replace-names
-  "Given a SQL query and its corresponding (untransformed) AST, apply the given table and column renames."
+  "Given an SQL query and its corresponding (untransformed) AST, apply the given table and column renames."
   [sql parsed-ast renames & {:as opts}]
   (let [{schema-renames :schemas
          table-renames  :tables

src/macaw/util.clj

@@ -35,9 +35,9 @@
   "Check whether the given literal matches the expected literal or pattern."
   [expected actual]
   (when expected
-    (if (instance? Pattern expected)
-      (boolean (re-find expected actual))
-      (= expected actual))))
+    (or (= expected actual)
+        (when (and (instance? Pattern expected) (string? actual))
+          (boolean (re-find expected actual))))))
 
 (defn- match-prefix [element ks-prefix]
   (let [expected (map element ks-prefix)]

test/macaw/core_test.clj

@@ -210,10 +210,16 @@ from foo")
                            :tables  {{:schema "public" :table "DOGS"} "cats"}
                            :columns {{:schema "PUBLIC" :table "dogs" :column "bark"} "meow"}}
                           {:case-insensitive :agnostic
-                           :allow-unused?    true}))))
+                           :allow-unused?    true})))
+
+  (testing "Updating the schema only"
+    (is (= "SELECT bark FROM private.dogs"
+           (m/replace-names "SELECT bark FROM PUBLIC.dogs"
+                            {:schemas {"public" "private"}}
+                            {:case-insensitive :lower})))))
 
 (def ^:private heavily-quoted-query-mixed-case
-  "SELECT RAW, \"Foo\", \"doNg\".\"bAr\", `ding`.`doNg`.`feE` FROM `ding`.`doNg`")
+  "SELECT RAW, \"Foo\", \"doNg\".\"bAr\", `ding`.`doNg`.`feE` FROM `dIng`.`doNg`")
 
 (deftest case-and-quotes-test
   (testing "By default, quoted references are also case insensitive"
@@ -231,12 +237,13 @@ from foo")
                                              :case-insensitive :agnostic
                                              :quotes-preserve-case? true))))
     (testing "The query is unchanged when allowed to run partially"
-      (is (= heavily-quoted-query-mixed-case
-             (m/replace-names heavily-quoted-query-mixed-case
-                              heavily-quoted-query-rewrites
-                              {:case-insensitive      :agnostic
-                               :quotes-preserve-case? true
-                               :allow-unused?         true}))))))
+      (let [with-no-exact-matches (str/replace heavily-quoted-query-mixed-case "ding" "dIng")]
+        (is (= with-no-exact-matches
+               (m/replace-names with-no-exact-matches
+                                heavily-quoted-query-rewrites
+                                {:case-insensitive      :agnostic
+                                 :quotes-preserve-case? true
+                                 :allow-unused?         true})))))))
 
 (def ^:private ambiguous-case-replacements
   {:columns {{:schema "public" :table "DOGS" :column "BARK"}  "MEOW"
@@ -420,10 +427,11 @@ from foo")
   ;; To consider - we could avoid splitting up the renames into column and table portions in the client, as
   ;; qualified targets would allow us to infer such changes. Partial qualification could also work fine where there
   ;; is no ambiguity - even if this is just a nice convenience for testing.
-  #_(is (= "SELECT aa.xx, b.x, b.y FROM aa, b;"
-           (m/replace-names "SELECT a.x, b.x, b.y FROM a, b;"
-                            {:columns {{:schema "public" :table "a" :column "x"}
-                                       {:table "aa" :column "xx"}}})))
+  (is (= "SELECT aa.xx, b.x, b.y FROM aa, b;"
+         (m/replace-names "SELECT a.x, b.x, b.y FROM a, b;"
+                          {:tables  {{:table "a"} "aa"}
+                           :columns {{:schema "public" :table "a" :column "x"}
+                                     {:table "aa" :column "xx"}}})))
 
   (is (= "SELECT qwe FROM orders"
          (m/replace-names "SELECT id FROM orders"
@@ -433,6 +441,15 @@ from foo")
          (m/replace-names "SELECT p.id, q.id FROM public.orders p join private.orders q"
                           {:tables {{:schema "public" :table "orders"} "whatever"}})))
 
+  (is (= "SELECT p.id FROM public.q p"
+         (m/replace-names "SELECT p.id FROM public.p p"
+                          {:tables {{:schema "public" :table "p"} "q"}})))
+
+  (is (= "SELECT p.id FROM public.q P"
+         (m/replace-names "SELECT p.id FROM public.p P"
+                          {:tables {{:schema "public" :table "p"} "q"}}
+                          {:case-insensitive :agnostic})))
+
   (is (ws= "SELECT SUM(public.orders.total) AS s,
             MAX(orders.total) AS max,
             MIN(total) AS min
@@ -461,13 +478,38 @@ from foo")
              :columns {{:schema "public" :table "core_user" :column "boink"}  "sturmunddrang"
                        {:schema "public" :table "snore_user" :column "yoink"} "oink"}}))))
 
+(deftest replace-intended-test
+  (is (= "SELECT S1.T1.C1, T2.C2, C3 FROM S1.T1 JOIN S2.T2"
+         (m/replace-names "SELECT s1.t1.c1, t2.c2, c3 FROM s1.t1 JOIN s2.t2"
+                          {:schemas {"s1" "S1", "s2" "S2"}
+                           :tables  {{:schema "s1" :table "t1"} "T1"
+                                     {:schema "s2" :table "t2"} "T2"}
+                           :columns {{:schema "s1" :table "t1" :column "c1"} "C1"
+                                     {:schema "s2" :table "t2" :column "c2"} "C2"
+                                     {:schema "s2" :table "t2" :column "c3"} "C3"}}))))
+
+(deftest replace-precision-test
+  (is (= "SELECT p.a.x, q.b.y, c.z FROM p.a, q.b, q.c"
+         (m/replace-names "SELECT s1.t1.a, s2.t1.a, t2.a FROM s1.t1, s2.t1, s2.t2"
+                          {:schemas {"s1" "p", "s2" "q"}
+                           :tables  {{:schema "s1" :table "t1"} "a"
+                                     {:schema "s2" :table "t1"} "b"
+                                     {:schema "s2" :table "t2"} "c"}
+                           :columns {{:schema "s1" :table "t1" :column "a"} "x"
+                                     {:schema "s2" :table "t1" :column "a"} "y"
+                                     {:schema "s2" :table "t2" :column "a"} "z"}}))))
+
+(deftest replace-schema-only-test
+  (is (= "SELECT totally_private.orders.x FROM totally_private.orders, private.orders WHERE x = 1"
+         (m/replace-names "SELECT public.orders.x FROM public.orders, private.orders WHERE x = 1"
+                          {:schemas {"public" "totally_private"}}))))
+
 (deftest replace-schema-test
-  ;; Somehow we broke renaming the `x` in the WHERE clause.
-  #_(is (= "SELECT totally_private.purchases.xx FROM totally_private.purchases, private.orders WHERE xx = 1"
-           (m/replace-names "SELECT public.orders.x FROM public.orders, private.orders WHERE x = 1"
-                            {:schemas {"public" "totally_private"}
-                             :tables  {{:schema "public" :table "orders"} "purchases"}
-                             :columns {{:schema "public" :table "orders" :column "x"} "xx"}}))))
+  (is (= "SELECT totally_private.purchases.xx FROM totally_private.purchases, private.orders WHERE xx = 1"
+         (m/replace-names "SELECT public.orders.x FROM public.orders, private.orders WHERE x = 1"
+                          {:schemas {"public" "totally_private"}
+                           :tables  {{:schema "public" :table "orders"} "purchases"}
+                           :columns {{:schema "public" :table "orders" :column "x"} "xx"}}))))
 
 (deftest allow-unused-test
   (is (thrown-with-msg?