From ea6c91b429e52de92e487d8fe691a38329cb3714 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Thu, 8 Feb 2018 23:33:51 +0100 Subject: [PATCH] Fix "drop default" casting rules for all databases. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The support for drop default in (user defined) casting rules was completely broken in SQLite, because the code didn't even bother looking at what's returning after applying the casting rules. This patch fixes the code so that is uses the pgcol instance's default value, as per after applying casting rules. The bug also existed in a subtle form for MySQL and MS SQL, but would only show up there when the default value is spelled using a known variation of “current timestamp”. --- src/sources/common/casting-rules.lisp | 41 +++++++++++---------- src/sources/mssql/mssql-cast-rules.lisp | 43 +++++++++++++---------- src/sources/mysql/mysql-cast-rules.lisp | 26 ++++++++------ src/sources/sqlite/sqlite-cast-rules.lisp | 32 +++++++++++------ test/sqlite-base64.load | 8 +++-- 5 files changed, 91 insertions(+), 59 deletions(-) diff --git a/src/sources/common/casting-rules.lisp b/src/sources/common/casting-rules.lisp index 3eed256..702a3ba 100644 --- a/src/sources/common/casting-rules.lisp +++ b/src/sources/common/casting-rules.lisp @@ -71,26 +71,31 @@ source (declare (ignore ctype extra)) (when - (and - (or (and t-s-p (string= type rule-source-type)) - (and c-s-p - (string-equal table-name (car rule-source-column)) - (string-equal column-name (cdr rule-source-column)))) - (or (null tm-s-p) (typemod-expr-matches-p typemod-expr typemod)) - (or (null d-s-p) (string= default rule-source-default)) - (or (null u-s-p) (eq unsigned rule-unsigned)) - (or (null n-s-p) (eq not-null rule-source-not-null)) + (or + ;; if we match by column, then table and column is all we + ;; need to compare + (and c-s-p + (string-equal table-name (car rule-source-column)) + (string-equal column-name (cdr rule-source-column))) - ;; current RULE only matches SOURCE when both have an - ;; auto_increment property, or none have it. - (or (and auto-increment rule-source-auto-increment) - (and (not auto-increment) (not rule-source-auto-increment))) + ;; otherwide, we do the full dance + (and + (or (and t-s-p (string= type rule-source-type))) + (or (null tm-s-p) (typemod-expr-matches-p typemod-expr typemod)) + (or (null d-s-p) (string= default rule-source-default)) + (or (null u-s-p) (eq unsigned rule-unsigned)) + (or (null n-s-p) (eq not-null rule-source-not-null)) - ;; current RULE only matches SOURCE when both have an - ;; on-update-current-timestamp property, or none have it. - (or (and on-update-current-timestamp rule-source-updts) - (and (not on-update-current-timestamp) - (not rule-source-updts)))) + ;; current RULE only matches SOURCE when both have an + ;; auto_increment property, or none have it. + (or (and auto-increment rule-source-auto-increment) + (and (not auto-increment) (not rule-source-auto-increment))) + + ;; current RULE only matches SOURCE when both have an + ;; on-update-current-timestamp property, or none have it. + (or (and on-update-current-timestamp rule-source-updts) + (and (not on-update-current-timestamp) + (not rule-source-updts))))) (list :using using :target rule-target)))))) (defun make-pgsql-type (source target using) diff --git a/src/sources/mssql/mssql-cast-rules.lisp b/src/sources/mssql/mssql-cast-rules.lisp index 1bb6732..55e72b0 100644 --- a/src/sources/mssql/mssql-cast-rules.lisp +++ b/src/sources/mssql/mssql-cast-rules.lisp @@ -123,7 +123,7 @@ "Return the PostgreSQL type definition from given MS SQL column definition." (with-slots (schema table-name name type default nullable) field - (declare (ignore schema)) ; FIXME + (declare (ignore schema)) ; FIXME (let* ((ctype (mssql-column-ctype field)) (extra (when (mssql-column-identity field) "auto_increment")) (pgcol @@ -136,27 +136,32 @@ (lambda (val) (if val (format nil "~a" val) :null)))) ;; normalize default values - ;; see *pgsql-default-values* - (setf (column-default pgcol) - (cond ((and (null default) (column-nullable pgcol)) :null) - ((and (stringp default) (string= "NULL" default)) :null) + ;; see pgloader.psql:*pgsql-default-values* + (let ((default (column-default pgcol))) + (setf (column-default pgcol) + (cond + ((and (null default) (column-nullable pgcol)) + :null) - ;; fix stupid N'' behavior from MS SQL column default - ((and (stringp default) - (uiop:string-enclosed-p "N'" default "'")) - (subseq default 2 (+ (length default) -1))) + ((and (stringp default) (string= "NULL" default)) + :null) - ((and (stringp default) - ;; address CURRENT_TIMESTAMP(6) and other spellings - (or (uiop:string-prefix-p "CURRENT_TIMESTAMP" default) - (string= "CURRENT TIMESTAMP" default))) - :current-timestamp) + ;; fix stupid N'' behavior from MS SQL column default + ((and (stringp default) + (uiop:string-enclosed-p "N'" default "'")) + (subseq default 2 (+ (length default) -1))) - ((and (stringp default) - (or (string= "newid()" default) - (string= "newsequentialid()" default))) - :generate-uuid) + ((and (stringp default) + ;; address CURRENT_TIMESTAMP(6) and other spellings + (or (uiop:string-prefix-p "CURRENT_TIMESTAMP" default) + (string= "CURRENT TIMESTAMP" default))) + :current-timestamp) - (t (column-default pgcol)))) + ((and (stringp default) + (or (string= "newid()" default) + (string= "newsequentialid()" default))) + :generate-uuid) + + (t (column-default pgcol))))) pgcol))) diff --git a/src/sources/mysql/mysql-cast-rules.lisp b/src/sources/mysql/mysql-cast-rules.lisp index b56efb6..242ba4a 100644 --- a/src/sources/mysql/mysql-cast-rules.lisp +++ b/src/sources/mysql/mysql-cast-rules.lisp @@ -190,16 +190,22 @@ (apply-casting-rules table-name name dtype ctype default nullable extra))) (setf (column-comment pgcol) comment) - ;; normalize default values - (setf (column-default pgcol) - (cond ((and (stringp default) (string= "NULL" default)) :null) - ((and (stringp default) - ;; address CURRENT_TIMESTAMP(6) and other spellings - (or (uiop:string-prefix-p "CURRENT_TIMESTAMP" default) - (string= "CURRENT TIMESTAMP" default) - (string= "current_timestamp()" default))) - :current-timestamp) - (t (column-default pgcol)))) + ;; normalize default values that are left after applying user defined + ;; casting rules "drop default" and "keep default" + (let ((default (column-default pgcol))) + (setf (column-default pgcol) + (cond + ((and (stringp default) (string= "NULL" default)) + :null) + + ((and (stringp default) + ;; address CURRENT_TIMESTAMP(6) and other spellings + (or (uiop:string-prefix-p "CURRENT_TIMESTAMP" default) + (string= "CURRENT TIMESTAMP" default) + (string= "current_timestamp()" default))) + :current-timestamp) + + (t (column-default pgcol))))) ;; extra user-defined data types (when (or (string-equal "set" dtype) diff --git a/src/sources/sqlite/sqlite-cast-rules.lisp b/src/sources/sqlite/sqlite-cast-rules.lisp index f884597..37a7145 100644 --- a/src/sources/sqlite/sqlite-cast-rules.lisp +++ b/src/sources/sqlite/sqlite-cast-rules.lisp @@ -87,16 +87,28 @@ (setf (column-transform pgcol) (lambda (val) (if val (format nil "~a" val) :null)))) - (setf (column-default pgcol) - (cond ((and (stringp default) (string= "NULL" default)) :null) - ((and (stringp default) - ;; address CURRENT_TIMESTAMP(6) and other spellings - (or (uiop:string-prefix-p "CURRENT_TIMESTAMP" default) - (string= "CURRENT TIMESTAMP" default))) - :current-timestamp) - ((and (stringp default) (string-equal "current_date" default)) - :current-date) - (t (column-default pgcol)))) + ;; normalize default values that comes from the casting rules + ;; (respecting user cast decision to "drop default" or "keep default") + (let ((default (column-default pgcol))) + (setf (column-default pgcol) + (cond + ((and (stringp default) (string= "NULL" default)) + :null) + + ((and (stringp default) + ;; address CURRENT_TIMESTAMP(6) and other spellings + (or (uiop:string-prefix-p "CURRENT_TIMESTAMP" default) + (string= "CURRENT TIMESTAMP" default))) + :current-timestamp) + + ((and (stringp default) (string-equal "current_date" default)) + :current-date) + + ((stringp default) + ;; at least quote the single quotes in there + (cl-ppcre:regex-replace-all "[']" default "''")) + + (t (column-default pgcol))))) pgcol))) diff --git a/test/sqlite-base64.load b/test/sqlite-base64.load index 88a5fec..9f5d7fc 100644 --- a/test/sqlite-base64.load +++ b/test/sqlite-base64.load @@ -1,7 +1,11 @@ load database from 'sqlite/storage.sqlite' - into postgresql:///storage + into postgresql:///pgloader with include drop, create tables, create indexes, reset sequences - set work_mem to '16MB', maintenance_work_mem to '512 MB'; \ No newline at end of file + set work_mem to '16MB', + maintenance_work_mem to '512 MB', + search_path to 'storage' + + before load do $$ create schema if not exists storage; $$; \ No newline at end of file