From eabfbb9cc8fd9ba29a2c0594a1b529d0e0265c0c Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Fri, 4 Sep 2015 01:06:15 +0200 Subject: [PATCH] Fix schema qualified table names usage (more). When parsing table names in the target URI, we are careful of splitting the table and schema name and store them into a cons in that case. Not all sources methods got the memo, clean that up. See #182 and #186, a pull request I am now going to be able to accept. Also see #287 that should be helped by being able to apply #186. --- pgloader.asd | 4 ++- src/package.lisp | 14 +++++++++-- src/parsers/command-dbf.lisp | 3 +-- src/pgsql/pgsql.lisp | 10 +------- src/schema.lisp | 45 ++++++++++++++++++++++++++++++++++ src/sources/common/schema.lisp | 25 ------------------- src/sources/db3/db3.lisp | 16 +++++++----- src/sources/ixf/ixf.lisp | 20 +++++++++------ test/dbf.load | 2 +- 9 files changed, 85 insertions(+), 54 deletions(-) create mode 100644 src/schema.lisp delete mode 100644 src/sources/common/schema.lisp diff --git a/pgloader.asd b/pgloader.asd index f7ee1a3..3ded7c8 100644 --- a/pgloader.asd +++ b/pgloader.asd @@ -66,6 +66,9 @@ ;; generic connection api (:file "connection" :depends-on ("utils")) + ;; some table name and schema facilities + (:file "schema" :depends-on ("package")) + ;; package pgloader.pgsql (:module pgsql :depends-on ("package" "params" "utils" "connection") @@ -118,7 +121,6 @@ ((:module "common" :components ((:file "api") - (:file "schema") (:file "casting-rules") (:file "files-and-pathnames") (:file "project-fields"))) diff --git a/src/package.lisp b/src/package.lisp index a434281..1b1fe4a 100644 --- a/src/package.lisp +++ b/src/package.lisp @@ -146,8 +146,15 @@ #:db-user #:db-pass)) -(defpackage #:pgloader.sources +(defpackage #:pgloader.schema (:use #:cl #:pgloader.params #:pgloader.utils #:pgloader.connection) + (:export #:push-to-end + #:with-schema)) + +(defpackage #:pgloader.sources + (:use #:cl + #:pgloader.params #:pgloader.utils #:pgloader.connection + #:pgloader.schema) (:import-from #:pgloader.transforms #:precision #:scale) (:import-from #:pgloader.parse-date #:parse-date-string @@ -175,6 +182,7 @@ ;; common schema facilities #:push-to-end + #:with-schema ;; file based utils for CSV, fixed etc #:with-open-file-or-stream @@ -189,7 +197,9 @@ #:cast)) (defpackage #:pgloader.pgsql - (:use #:cl #:pgloader.params #:pgloader.utils #:pgloader.connection) + (:use #:cl + #:pgloader.params #:pgloader.utils #:pgloader.connection + #:pgloader.schema) (:export #:pgsql-connection #:pgconn-use-ssl #:pgconn-table-name diff --git a/src/parsers/command-dbf.lisp b/src/parsers/command-dbf.lisp index 2ffa80d..f46d5dc 100644 --- a/src/parsers/command-dbf.lisp +++ b/src/parsers/command-dbf.lisp @@ -100,8 +100,7 @@ ,@(pgsql-connection-bindings pg-db-conn gucs) ,@(batch-control-bindings options) ,@(identifier-case-binding options) - (table-name ,(or (getf options :table-name) - (pgconn-table-name pg-db-conn))) + (table-name ',(pgconn-table-name pg-db-conn)) (source-db (with-stats-collection ("fetch" :state state-before) (expand (fetch-file ,dbf-db-conn)))) (source diff --git a/src/pgsql/pgsql.lisp b/src/pgsql/pgsql.lisp index f66ea49..b9a3a9d 100644 --- a/src/pgsql/pgsql.lisp +++ b/src/pgsql/pgsql.lisp @@ -54,15 +54,7 @@ (truncate-tables pgconn (list table-name))) (with-pgsql-connection (pgconn) - (let ((unqualified-table-name - (typecase table-name - (cons (let ((sql (format nil "SET search_path TO ~a;" - (car table-name)))) - (log-message :notice "~a" sql) - (pgsql-execute sql) - (cdr table-name))) - (string table-name)))) - + (with-schema (unqualified-table-name table-name) (when disable-triggers (disable-triggers unqualified-table-name)) (log-message :info "pgsql:copy-from-queue: ~a ~a" table-name columns) diff --git a/src/schema.lisp b/src/schema.lisp new file mode 100644 index 0000000..bd63f92 --- /dev/null +++ b/src/schema.lisp @@ -0,0 +1,45 @@ +;;; +;;; Generic API for pgloader sources +;;; +(in-package :pgloader.schema) + +(defmacro push-to-end (item place) + `(setf ,place (nconc ,place (list ,item)))) + +;;; +;;; TODO: stop using anonymous data structures for database catalogs, +;;; currently list of alists of lists... the madness has found its way in +;;; lots of places tho. +;;; + +;;; +;;; A database catalog is a list of schema each containing a list of tables, +;;; each being a list of columns. +;;; +;;; Column structures details depend on the specific source type and are +;;; implemented in each source separately. +;;; +(defstruct schema name tables) +(defstruct table schema name qname columns) + +;;; +;;; Still lacking round tuits here, so for the moment the representation of +;;; a table name is either a string or a cons built from schema and +;;; table-name. +;;; + +(defmacro with-schema ((var table-name) &body body) + "When table-name is a CONS, SET search_path TO its CAR and return its CDR, + otherwise just return the TABLE-NAME. A PostgreSQL connection must be + established when calling this function." + `(let ((,var + (typecase ,table-name + (cons (let ((sql (format nil "SET search_path TO ~a;" + (car ,table-name)))) + (log-message :notice "~a" sql) + (pgloader.pgsql:pgsql-execute sql) + (cdr ,table-name))) + (string ,table-name)))) + ,@body)) + + diff --git a/src/sources/common/schema.lisp b/src/sources/common/schema.lisp deleted file mode 100644 index 1a7a523..0000000 --- a/src/sources/common/schema.lisp +++ /dev/null @@ -1,25 +0,0 @@ -;;; -;;; Generic API for pgloader sources -;;; -(in-package :pgloader.sources) - -(defmacro push-to-end (item place) - `(setf ,place (nconc ,place (list ,item)))) - -;;; -;;; TODO: stop using anonymous data structures for database catalogs, -;;; currently list of alists of lists... the madness has found its way in -;;; lots of places tho. -;;; - -;;; -;;; A database catalog is a list of schema each containing a list of tables, -;;; each being a list of columns. -;;; -;;; Column structures details depend on the specific source type and are -;;; implemented in each source separately. -;;; -(defstruct schema name tables) -(defstruct table schema name qualified-name columns) - - diff --git a/src/sources/db3/db3.lisp b/src/sources/db3/db3.lisp index fe889ab..f0af6ae 100644 --- a/src/sources/db3/db3.lisp +++ b/src/sources/db3/db3.lisp @@ -20,7 +20,10 @@ (unless (and (slot-boundp db3 'columns) (slot-value db3 'columns)) (setf (slot-value db3 'columns) (list-all-columns (fd-db3 conn) - (or (target db3) (source db3))))) + (or (typecase (target db3) + (cons (cdr (target db3))) + (string (target db3))) + (source db3))))) (let ((transforms (when (slot-boundp db3 'transforms) (slot-value db3 'transforms)))) @@ -118,12 +121,13 @@ :summary summary) (with-pgsql-transaction (:pgconn (target-db db3)) (when create-tables - (log-message :notice "Create table \"~a\"" table-name) - (create-tables (columns db3) - :include-drop include-drop - :if-not-exists t))))) + (with-schema (tname table-name) + (log-message :notice "Create table \"~a\"" tname) + (create-tables (columns db3) + :include-drop include-drop + :if-not-exists t)))))) - (cl-postgres::database-errors (e) + (cl-postgres::database-error (e) (declare (ignore e)) ; a log has already been printed (log-message :fatal "Failed to create the schema, see above.") (return-from copy-database))) diff --git a/src/sources/ixf/ixf.lisp b/src/sources/ixf/ixf.lisp index bd3bc57..2602b2d 100644 --- a/src/sources/ixf/ixf.lisp +++ b/src/sources/ixf/ixf.lisp @@ -19,7 +19,10 @@ (with-connection (conn (source-db source)) (unless (and (slot-boundp source 'columns) (slot-value source 'columns)) (setf (slot-value source 'columns) - (list-all-columns (conn-handle conn) (source source)))) + (list-all-columns (conn-handle conn) + (typecase (target source) + (cons (cdr (target source))) + (string (target source)))))) (let ((transforms (when (slot-boundp source 'transforms) (slot-value source 'transforms)))) @@ -124,14 +127,15 @@ (with-stats-collection ("create, truncate" :state state-before :summary summary) - (with-pgsql-transaction (:pgconn (target-db ixf)) - (when create-tables - (log-message :notice "Create table \"~a\"" table-name) - (create-tables (columns ixf) - :include-drop include-drop - :if-not-exists t))))) + (with-pgsql-transaction (:pgconn (target-db ixf)) + (when create-tables + (with-schema (tname table-name) + (log-message :notice "Create table \"~a\"" tname) + (create-tables (columns ixf) + :include-drop include-drop + :if-not-exists t)))))) - (cl-postgres::database-errors (e) + (cl-postgres::database-error (e) (declare (ignore e)) ; a log has already been printed (log-message :fatal "Failed to create the schema, see above.") (return-from copy-database))) diff --git a/test/dbf.load b/test/dbf.load index e4c1f15..09d790a 100644 --- a/test/dbf.load +++ b/test/dbf.load @@ -5,5 +5,5 @@ */ LOAD DBF FROM data/reg2013.dbf with encoding cp850 - INTO postgresql:///pgloader?reg2013 + INTO postgresql:///pgloader?public.reg2013 WITH truncate, create table, disable triggers;