Fix more table name quoting, fix #163 again.

Now that we can have several threads doing COPY, each of them need to
know about the *pgsql-reserved-keywords* list. Make sure that's the case
and in passing fix some call sites to apply-identifier-case.

Also, more disturbingly, fix the code so that TRUNCATE is called from
the main thread before giving control to the COPY threads, rather than
having two concurrent threads doing the TRUNCATE twice. It's rather
strange that we got no complaint from the field on that part...
This commit is contained in:
Dimitri Fontaine 2015-12-08 11:52:43 +01:00
parent dca3dacf4b
commit b4bfa18877
8 changed files with 57 additions and 47 deletions

View File

@ -407,23 +407,21 @@
;; once we are done running the load-file, compare the loaded data with
;; our expected data file
(let* ((expected-subdir (directory-namestring
(asdf:system-relative-pathname
:pgloader "test/regress/expected/")))
(expected-data-file (make-pathname :defaults load-file
:type "out"
:directory expected-subdir)))
(destructuring-bind (target *pg-settings*)
(parse-target-pg-db-uri load-file)
(log-message :log "Comparing loaded data against ~s"
expected-data-file)
(bind ((expected-subdir (directory-namestring
(asdf:system-relative-pathname
:pgloader "test/regress/expected/")))
(expected-data-file (make-pathname :defaults load-file
:type "out"
:directory expected-subdir))
((target *pg-settings*) (parse-target-pg-db-uri load-file))
(*pgsql-reserved-keywords* (list-reserved-keywords target))
(let ((expected-data-source
(expected-data-source
(parse-source-string-for-type
:copy (uiop:native-namestring expected-data-file)))
;; change target table-name schema
(expected-data-target
;; change target table-name schema
(expected-data-target
(let ((e-d-t (clone-connection target)))
(setf (pgconn-table-name e-d-t)
(cons "expected"
@ -432,29 +430,33 @@
(cons (cdr (pgconn-table-name e-d-t))))))
e-d-t)))
;; prepare expected table in "expected" schema
(with-pgsql-connection (target)
(with-schema (unqualified-table-name (pgconn-table-name target))
(let ((drop (format nil "drop table if exists expected.~a;"
unqualified-table-name))
(create (format nil "create table expected.~a(like ~a);"
unqualified-table-name unqualified-table-name)))
(log-message :notice "~a" drop)
(pomo:query drop)
(log-message :notice "~a" create)
(pomo:query create))))
(log-message :log "Comparing loaded data against ~s" expected-data-file)
;; load expected data
(load-data :from expected-data-source
:into expected-data-target
:options '(:truncate t)
:start-logger nil))
;; prepare expected table in "expected" schema
(with-pgsql-connection (target)
(with-schema (unqualified-table-name (pgconn-table-name target))
(let* ((tname (apply-identifier-case unqualified-table-name))
(drop (format nil "drop table if exists expected.~a;"
tname))
(create (format nil "create table expected.~a(like ~a);"
tname tname)))
(log-message :notice "~a" drop)
(pomo:query drop)
(log-message :notice "~a" create)
(pomo:query create))))
;; load expected data
(load-data :from expected-data-source
:into expected-data-target
:options '(:truncate t)
:start-logger nil)
;; now compare both
(with-pgsql-connection (target)
(with-schema (unqualified-table-name (pgconn-table-name target))
(let* ((cols (loop :for (name type)
:in (list-columns-query unqualified-table-name)
(let* ((tname (apply-identifier-case unqualified-table-name))
(cols (loop :for (name type)
:in (list-columns-query tname)
;;
;; We can't just use table names here, because
;; PostgreSQL support for the POINT datatype fails
@ -469,9 +471,9 @@
(sql (format nil
"select count(*) from (select ~{~a~^, ~} from ~a except select ~{~a~^, ~} from expected.~a) ss"
cols
unqualified-table-name
tname
cols
unqualified-table-name))
tname))
(diff-count (pomo:query sql :single)))
(log-message :notice "~a" sql)
(log-message :notice "Got a diff of ~a rows" diff-count)
@ -483,7 +485,7 @@
(progn
(log-message :log "Regress fail.")
#-pgloader-image (values diff-count +os-code-error-regress+)
#+pgloader-image (uiop:quit +os-code-error-regress+))))))))))
#+pgloader-image (uiop:quit +os-code-error-regress+)))))))))
;;;

View File

@ -601,7 +601,7 @@
(defpackage #:pgloader
(:use #:cl
#:pgloader.params #:pgloader.utils #:pgloader.parser
#:pgloader.connection)
#:pgloader.connection #:metabang.bind)
(:import-from #:pgloader.pgsql
#:pgconn-table-name
#:pgsql-connection
@ -611,7 +611,9 @@
#:list-columns-query)
(:import-from #:pgloader.pgsql
#:with-pgsql-connection
#:with-schema)
#:with-schema
#:list-reserved-keywords
#:apply-identifier-case)
(:export #:*version-string*
#:*state*
#:*fd-path-root*

View File

@ -15,6 +15,7 @@
#:*client-min-messages*
#:*log-min-messages*
#:*report-stream*
#:*pgsql-reserved-keywords*
#:*identifier-case*
#:*preserve-index-names*
#:*copy-batch-rows*
@ -94,6 +95,9 @@
;;;
;;; When converting from other databases, how to deal with case sensitivity?
;;;
(defvar *pgsql-reserved-keywords* nil
"We need to always quote PostgreSQL reserved keywords")
(defparameter *identifier-case* :downcase
"Dealing with source databases casing rules.")

View File

@ -48,9 +48,6 @@
*writer-batch* for us to send down to PostgreSQL, and when that's done
update stats."
(let ((seconds 0))
(when truncate
(truncate-tables pgconn (list table-name)))
(with-pgsql-connection (pgconn)
(with-schema (unqualified-table-name table-name)
(with-disabled-triggers (unqualified-table-name
@ -63,7 +60,8 @@
:until (eq :end-of-data mesg)
:for (rows batch-seconds) :=
(let ((start-time (get-internal-real-time)))
(list (copy-batch unqualified-table-name columns batch read)
(list (copy-batch (apply-identifier-case unqualified-table-name)
columns batch read)
(elapsed-time-since start-time)))
:do (progn
;; The SBCL implementation needs some Garbage Collection

View File

@ -3,9 +3,6 @@
;;;
(in-package pgloader.pgsql)
(defvar *pgsql-reserved-keywords* nil
"We need to always quote PostgreSQL reserved keywords")
(defun quoted-p (s)
"Return true if s is a double-quoted string"
(and (eq (char s 0) #\")

View File

@ -120,6 +120,13 @@
;; http://www.anarazel.de/talks/pgconf-eu-2015-10-30/concurrency.pdf
;;
;; Let's just hardcode 2 threads for that then.
;;
;; Also, we need to do the TRUNCATE here before starting the
;; threads, so that it's done just once.
(when truncate
(truncate-tables (clone-connection (target-db copy))
(list (target copy))))
(loop :for w :below 2
:do (lp:submit-task channel
#'pgloader.pgsql:copy-from-queue
@ -127,7 +134,6 @@
(target copy)
fmtq
:columns (copy-column-list copy)
:truncate truncate
:disable-triggers disable-triggers))
;; now wait until both the tasks are over, and kill the kernel

View File

@ -17,6 +17,7 @@
(*log-min-messages* . ,*log-min-messages*)
;; needed in create index specific kernels
(*pgsql-reserved-keywords* . ',*pgsql-reserved-keywords*)
(*preserve-index-names* . ,*preserve-index-names*)
;; bindings updates for libs

View File

@ -1,14 +1,14 @@
LOAD CSV
FROM INLINE
INTO postgresql:///pgloader?public.header
INTO postgresql:///pgloader?"group"
WITH truncate,
fields terminated by ',',
csv header
BEFORE LOAD DO
$$ drop table if exists header; $$,
$$ CREATE TABLE header
$$ drop table if exists "group"; $$,
$$ CREATE TABLE "group"
(
somefields text,
rekplcode text,