cybertec-postgresql / db_migrator

Other
21 stars 8 forks source link

Index column expressions may be capitalized #18

Closed fljdin closed 1 year ago

fljdin commented 1 year ago

Hi,

On experimenting mssql_migrator development, any capitalized columns were not double-quoted.

WARNING:  Error executing "CREATE UNIQUE INDEX "AK_Department_Name" 
                           ON "HumanResources"."Department" (Name ASC)"
DETAIL:  column "name" does not exist: 
WARNING:  Error executing "CREATE UNIQUE INDEX "AK_Employee_LoginID" 
                           ON "HumanResources"."Employee" (LoginID ASC)"
DETAIL:  column "loginid" does not exist: 
WARNING:  Error executing "CREATE UNIQUE INDEX "AK_Employee_NationalIDNumber" 
                           ON "HumanResources"."Employee" (NationalIDNumber ASC)"
DETAIL:  column "nationalidnumber" does not exist:
...

I made a local patch on db_migrator, but a comment suggested me that would be much harder than this.

diff --git a/db_migrator--1.0.0.sql b/db_migrator--1.0.0.sql
index cec64ac..852280a 100644
--- a/db_migrator--1.0.0.sql
+++ b/db_migrator--1.0.0.sql
@@ -2113,7 +2113,7 @@ BEGIN
       END IF;

       /* fold expressions to lower case */
-      stmt := stmt || separator || expr
+      stmt := stmt || separator || format('%I', expr)
                    || CASE WHEN des THEN ' DESC' ELSE ' ASC' END;
       separator := ', ';
    END LOOP;

What do you think?

Florent

laurenz commented 1 year ago

I think that is an oversight from when I separated db_migrator from ora_migrator. There are the "translate" functions for that case, and db_migrator should call translate_expression_fun on expr.

Each plugin might have its own idea about case folding, and simply double-quoting the expression like your patch does would certainly break any expression index.