From 25e5ea9ac3b3217633f78fb4c5560c44e58f6a75 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Thu, 8 Jun 2017 11:22:55 +0200 Subject: [PATCH] Refactor error handling in complete-pgsql-database. Given new SQLite test case from issue #563 we see that pgloader doesn't handle errors gracefully in post-copy stage. That's because the API were not properly defined, we should use pgsql-execute-with-timing rather than other construct here, because it allows the "on error resume next" behavior we want with after load DDL statements. See #563. --- src/pgsql/connection.lisp | 7 ++- src/pgsql/pgsql-create-schema.lisp | 76 ++++++++++++++++------------- src/sources/common/db-methods.lisp | 23 ++++----- test/sqlite-testpk.load | 9 ++++ test/sqlite/test_pk.db | Bin 0 -> 12288 bytes test/sqlite/test_pk.sql | 14 ++++++ 6 files changed, 81 insertions(+), 48 deletions(-) create mode 100644 test/sqlite-testpk.load create mode 100644 test/sqlite/test_pk.db create mode 100644 test/sqlite/test_pk.sql diff --git a/src/pgsql/connection.lisp b/src/pgsql/connection.lisp index cf26baf..d6fe832 100644 --- a/src/pgsql/connection.lisp +++ b/src/pgsql/connection.lisp @@ -239,12 +239,15 @@ (pomo:with-transaction () (pgsql-execute-with-timing section label sql :count count)))) -(defun pgsql-execute-with-timing (section label sql &key (count 1)) +(defun pgsql-execute-with-timing (section label sql + &key + client-min-messages + (count 1)) "Execute given SQL and resgister its timing into STATE." (multiple-value-bind (res secs) (timing (handler-case - (pgsql-execute sql) + (pgsql-execute sql :client-min-messages client-min-messages) (cl-postgres:database-error (e) (log-message :error "~a" e) (update-stats section label :errs 1 :rows (- count))))) diff --git a/src/pgsql/pgsql-create-schema.lisp b/src/pgsql/pgsql-create-schema.lisp index b712952..8818d3d 100644 --- a/src/pgsql/pgsql-create-schema.lisp +++ b/src/pgsql/pgsql-create-schema.lisp @@ -104,7 +104,11 @@ :include-drop include-drop :client-min-messages client-min-messages)) -(defun create-triggers (catalog &key (client-min-messages :notice)) +(defun create-triggers (catalog + &key + label + (section :post) + (client-min-messages :notice)) "Create the catalog objects that come after the data has been loaded." (let ((sql-list (loop :for table :in (table-list catalog) @@ -113,9 +117,9 @@ :append (loop :for trigger :in (table-trigger-list table) :collect (format-create-sql (trigger-procedure trigger)) :collect (format-create-sql trigger))))) - (loop :for sql :in sql-list - :do (pgsql-execute sql :client-min-messages client-min-messages) - :count t))) + (pgsql-execute-with-timing section label sql-list + :count (length sql-list) + :client-min-messages client-min-messages))) ;;; @@ -183,20 +187,22 @@ (pgsql-execute sql)) :count t)))) -(defun create-pgsql-fkeys (catalog) +(defun create-pgsql-fkeys (catalog &key (section :post) label) "Actually create the Foreign Key References that where declared in the MySQL database" - (loop :for table :in (table-list catalog) - :sum (loop :for fkey :in (table-fkey-list table) - :for sql := (format-create-sql fkey) - :do (pgsql-execute sql) - :count t) - :sum (loop :for index :in (table-index-list table) - :sum (loop :for fkey :in (index-fk-deps index) - :for sql := (format-create-sql fkey) - :do (log-message :debug "EXTRA FK DEPS!") - :do (pgsql-execute sql) - :count t)))) + (let ((fk-sql-list + (loop :for table :in (table-list catalog) + :append (loop :for fkey :in (table-fkey-list table) + :for sql := (format-create-sql fkey) + :collect sql) + :append (loop :for index :in (table-index-list table) + :do (loop :for fkey :in (index-fk-deps index) + :for sql := (format-create-sql fkey) + :do (log-message :debug "EXTRA FK DEPS! ~a" sql) + :collect sql))))) + ;; and now execute our list + (pgsql-execute-with-timing section label fk-sql-list + :count (length fk-sql-list)))) @@ -384,7 +390,7 @@ $$; " tables))) ;;; ;;; Comments ;;; -(defun comment-on-tables-and-columns (catalog) +(defun comment-on-tables-and-columns (catalog &key label (section :post)) "Install comments on tables and columns from CATALOG." (let* ((quote ;; just something improbably found in a table comment, to use as @@ -400,20 +406,24 @@ $$; " tables))) "_" (map 'string #'code-char (loop :repeat 5 - :collect (+ (random 26) (char-code #\A))))))) - (loop :for table :in (table-list catalog) - :for sql := (when (table-comment table) - (format nil "comment on table ~a is $~a$~a$~a$" - (table-name table) - quote (table-comment table) quote)) - :count (when sql - (pgsql-execute-with-timing :post "Comments" sql)) + :collect (+ (random 26) (char-code #\A)))))) - :sum (loop :for column :in (table-column-list table) - :for sql := (when (column-comment column) - (format nil "comment on column ~a.~a is $~a$~a$~a$" - (table-name table) - (column-name column) - quote (column-comment column) quote)) - :count (when sql - (pgsql-execute-with-timing :post "Comments" sql)))))) + (sql-list + ;; table level comments + (loop :for table :in (table-list catalog) + :when (table-comment table) + :collect (format nil "comment on table ~a is $~a$~a$~a$" + (table-name table) + quote (table-comment table) quote) + + ;; for each table, append column level comments + :append + (loop :for column :in (table-column-list table) + :when (column-comment column) + :collect (format nil + "comment on column ~a.~a is $~a$~a$~a$" + (table-name table) + (column-name column) + quote (column-comment column) quote))))) + (pgsql-execute-with-timing section label sql-list + :count (length sql-list)))) diff --git a/src/sources/common/db-methods.lisp b/src/sources/common/db-methods.lisp index 7d91097..d72a664 100644 --- a/src/sources/common/db-methods.lisp +++ b/src/sources/common/db-methods.lisp @@ -147,28 +147,25 @@ ;; and indexes are imported before doing that. ;; (when foreign-keys - (with-stats-collection ("Create Foreign Keys" :section :post - :use-result-as-read t - :use-result-as-rows t) - (create-pgsql-fkeys catalog))) + (create-pgsql-fkeys catalog + :section :post + :label "Create Foreign Keys")) ;; ;; Triggers and stored procedures -- includes special default values ;; (when create-triggers - (with-stats-collection ("Create Triggers" :section :post - :use-result-as-read t - :use-result-as-rows t) - (with-pgsql-transaction (:pgconn (target-db copy)) - (create-triggers catalog))))) + (with-pgsql-transaction (:pgconn (target-db copy)) + (create-triggers catalog + :section :post + :label "Create Triggers")))) ;; ;; And now, comments on tables and columns. ;; - (with-stats-collection ("Install Comments" :section :post - :use-result-as-read t - :use-result-as-rows t) - (comment-on-tables-and-columns catalog)))) + (comment-on-tables-and-columns catalog + :section :post + :label "Install Comments"))) (defmethod instanciate-table-copy-object ((copy db-copy) (table table)) "Create an new instance for copying TABLE data." diff --git a/test/sqlite-testpk.load b/test/sqlite-testpk.load new file mode 100644 index 0000000..b9f6612 --- /dev/null +++ b/test/sqlite-testpk.load @@ -0,0 +1,9 @@ +load database + from 'sqlite/test_pk.db' + into postgresql:///pgloader + + before load do + $$ drop schema if exists sqlite cascade; $$, + $$ create schema if not exists sqlite; $$ + + set search_path to 'sqlite'; diff --git a/test/sqlite/test_pk.db b/test/sqlite/test_pk.db new file mode 100644 index 0000000000000000000000000000000000000000..42d37dbfa52e6e5d007c95b5469dcbe64546f166 GIT binary patch literal 12288 zcmeI#%}T>S5Ww->R4D}fxD}DYfQL39UVMSZvP!WQ-R9C$sieZ9rl7%leFC4;r*Kmm zO^P19l>fkHcaq66`Q^0ZyP?bH^00V$p5=0`4wcrbC!&-J{Al=5Mv?E7iJx_4{ns*3 zXS=;U|1{YDR0kjaLLh(u0tg_000IagfB*srAn>mO%afqlYPa?KQ9iqWny2o~y}CtK z&W*k`abhHiFNQ|SyV4ECN10AtDuYpCu8fsiJGhDMM6S)`v|jY+vb0)h&84ws)Hmaw zjf$G?_qOXdqA<#Iv(@S7<#DaaX;DzC4t{NSRkzvmW;>O}!>Bl$`$Ip000IagfB*sr pAb