cybertec-postgresql / db_migrator

Other
21 stars 8 forks source link

New execute_statement() function #35

Closed fljdin closed 1 year ago

fljdin commented 1 year ago

Dedicated function to print the exception detail and log it. Refactor existing code to execute migration related statement within a single method.

Closes #34

fljdin commented 1 year ago

Anyway, I ran the ora_migrator regression test, and it failed. The first difference was:

 SELECT db_migrate_tables(
    plugin => 'ora_migrator'
 );
-WARNING:  Error loading table data for testschema1.baddata
-DETAIL:  invalid byte sequence for encoding "UTF8": 0x00: 
- db_migrate_tables 
--------------------
-                 1
-(1 row)
-
+WARNING:  Error executing "INSERT INTO testschema1.baddata SELECT * FROM testschema1."baddata^G""
+DETAIL:  invalid byte sequence for encoding "UTF8": 0xf6 0x6e 0x20 0x6b: 
+ERROR:  "baddata" is not a foreign table
+HINT:  Use DROP TABLE to remove a table.
+CONTEXT:  SQL statement "DROP FOREIGN TABLE testschema1.baddata"
+PL/pgSQL function public.db_migrate_tables(name,name,boolean) line 45 at EXECUTE
 SELECT db_migrate_functions(
    plugin => 'ora_migrator'
 );

Looking at your changes to materialize_foreign_table(), I see why: Before, the following was run in a subtransaction (a BEGIN / EXCEPTION / END block) as follows

  • rename the foreign table
  • create a local table like the foreign table (plus partitions and subpartitions)
  • copy the data from the foreign table if desired
  • drop the foreign table

So if one of these steps failed, the whole subtransaction was rolled back, and all the above was undone.

Your refactored code did away with that subtransaction and relies entirely on execute_statement() to handle errors. Now in my regression tests, table baddata contains bad data that make copying the data from the foreign table to the local table fail. With my original code, renaming the foreign table and creating the local table was rolled back, while the refactored code leaves them in place, which leads to the above difference and errors further down the road.

_Originally posted by @laurenz in https://github.com/cybertec-postgresql/db_migrator/issues/33#issuecomment-1718894479_

laurenz commented 1 year ago

That pull request has the same problem:

 SELECT db_migrate_tables(
    plugin => 'ora_migrator'
 );
-WARNING:  Error loading table data for testschema1.baddata
+WARNING:  Error executing "INSERT INTO testschema1.baddata SELECT * FROM testschema1."baddata^G""
 DETAIL:  invalid byte sequence for encoding "UTF8": 0x00: 
- db_migrate_tables 
--------------------
-                 1
-(1 row)
-
+ERROR:  "baddata" is not a foreign table
+HINT:  Use DROP TABLE to remove a table.
+CONTEXT:  SQL statement "DROP FOREIGN TABLE testschema1.baddata"
+PL/pgSQL function public.db_migrate_tables(name,name,boolean) line 45 at EXECUTE
 SELECT db_migrate_functions(
    plugin => 'ora_migrator'
 );

The problem is that materialize_foreign_table() has to undo everything it did before it failed: If copying the data fails, the table has to be dropped, and the foreign table has to be renamed back. Since you probably cannot do that using a subtransaction any more, you might have to do it explicitly.

fljdin commented 1 year ago

Another workaround could be iterate through an ARRAY of statements, allowing us to handle exceptions and sub-transaction behavior. The method will be renamed to execute_statements()

CREATE FUNCTION execute_statements(
   operation      text,
   schema         name,
   object_name    name,
   statements     text[],
   pgstage_schema name DEFAULT NAME 'pgsql_stage'
) RETURNS boolean
  LANGUAGE plpgsql VOLATILE STRICT SET search_path = pg_catalog AS
$$DECLARE
   stmt   text;
   errmsg text;
   detail text;
BEGIN
   BEGIN
      FOREACH stmt IN ARRAY statements
      LOOP
         EXECUTE stmt;
      END LOOP;
   EXCEPTION
      WHEN others THEN
         /* raise exception and insert into migrate_log table */
         RETURN FALSE;
   END;

   RETURN TRUE;
END$$;

My local change did not break ora_migrator tests anymore:

@@ -178,7 +178,7 @@
 SELECT db_migrate_tables(
    plugin => 'ora_migrator'
 );
-WARNING:  Error loading table data for testschema1.baddata
+WARNING:  Error executing "INSERT INTO testschema1.baddata SELECT * FROM testschema1."baddata""
 DETAIL:  invalid byte sequence for encoding "UTF8": 0x00: 
  db_migrate_tables 
 -------------------
@@ -220,7 +220,7 @@
 SELECT db_migrate_constraints(
    plugin => 'ora_migrator'
 );
-WARNING:  Error creating primary key or unique constraint on table testschema1.baddata
+WARNING:  Error executing "ALTER TABLE testschema1.baddata ADD CONSTRAINT baddata_pkey PRIMARY KEY (id) NOT DEFERRABLE INITIALLY IMMEDIATE"
 DETAIL:  relation "testschema1.baddata" does not exist: 
  db_migrate_constraints 
 ------------------------
laurenz commented 1 year ago

Sure, that is fine by me.