From aa8ae159e2c5714bd1913d8dc381a74b460719a9 Mon Sep 17 00:00:00 2001
From: Dimitri Fontaine <dim@tapoueh.org>
Date: Sun, 18 Nov 2018 18:21:51 +0100
Subject: [PATCH] Improve error handling when applying Citus distribution
 rules.

Make it so that we generate a proper error message to the user when failing
to figure out the PATH to the distribution key, rather than failing with an
internal error about The value NIL is not of type PGLOADER.CATALOG:TABLE.
---
 src/load/migrate-database.lisp | 13 ++++---
 src/package.lisp               |  3 +-
 src/utils/citus.lisp           | 65 +++++++++++++++++++++++-----------
 3 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/src/load/migrate-database.lisp b/src/load/migrate-database.lisp
index 3037571..db7d096 100644
--- a/src/load/migrate-database.lisp
+++ b/src/load/migrate-database.lisp
@@ -347,10 +347,15 @@
 
     ;; apply catalog level transformations to support the database migration
     ;; that's CAST rules, index WHERE clause rewriting and ALTER commands
-    (process-catalog copy catalog
-                     :alter-table alter-table
-                     :alter-schema alter-schema
-                     :distribute distribute)
+    (handler-case
+        (process-catalog copy catalog
+                         :alter-table alter-table
+                         :alter-schema alter-schema
+                         :distribute distribute)
+
+      (citus-rule-is-missing-from-list (e)
+        (log-message :fatal "~a" e)
+        (return-from copy-database)))
 
     ;; if asked, first drop/create the tables on the PostgreSQL side
     (handler-case
diff --git a/src/package.lisp b/src/package.lisp
index 20820ab..307eb3d 100644
--- a/src/package.lisp
+++ b/src/package.lisp
@@ -298,7 +298,8 @@
         #:pgloader.monitor)
   (:export #:citus-distribute-schema
            #:citus-format-sql-select
-           #:citus-backfill-table-p))
+           #:citus-backfill-table-p
+           #:citus-rule-is-missing-from-list))
 
 (defpackage #:pgloader.utils
   (:use #:cl
diff --git a/src/utils/citus.lisp b/src/utils/citus.lisp
index 65743af..0bcf329 100644
--- a/src/utils/citus.lisp
+++ b/src/utils/citus.lisp
@@ -172,6 +172,18 @@
 ;;; itself to the table-citus-rule slot so that we later know to generate a
 ;;; proper SELECT query that includes the backfilling.
 ;;;
+(define-condition citus-rule-is-missing-from-list (error)
+  ((rule  :initarg :rule :accessor citus-rule))
+  (:report
+   (lambda (err stream)
+     (let ((*print-circle* nil))
+       (format stream
+               "Failed to add column ~s to table ~a for lack of a FROM clause in the distribute rule:~%    distribute ~a using ~a from ?"
+               (column-name (citus-distributed-rule-using (citus-rule err)))
+               (format-table-name (citus-distributed-rule-table (citus-rule err)))
+               (format-table-name (citus-distributed-rule-table (citus-rule err)))
+               (column-name (citus-distributed-rule-using (citus-rule err))))))))
+
 (defgeneric apply-citus-rule (rule)
   (:documentation "Apply a Citus distribution RULE to given TABLE."))
 
@@ -206,28 +218,41 @@
         ;; it up in the last entry of the FROM rule's list.
         (let* ((last-from-rule (car (last (citus-distributed-rule-from rule))))
                (column-definition
-                (find (column-name (citus-distributed-rule-using rule))
-                      (table-field-list last-from-rule)
-                      :test #'string=
-                      :key #'column-name))
+                (when last-from-rule
+                  (find (column-name (citus-distributed-rule-using rule))
+                        (table-field-list last-from-rule)
+                        :test #'string=
+                        :key #'column-name)))
                (new-column
-                (make-column :name (column-name column-definition)
-                             :type-name (column-type-name column-definition)
-                             :nullable (column-nullable column-definition)
-                             :transform (column-transform column-definition))))
-          ;;
-          ;; Here also we need to add the new column to the PKEY definition,
-          ;; in first position.
-          ;;
-          (add-column-to-pkey table (column-name new-column))
+                (when column-definition
+                  (make-column :name (column-name column-definition)
+                               :type-name (column-type-name column-definition)
+                               :nullable (column-nullable column-definition)
+                               :transform (column-transform column-definition)))))
 
-          ;;
-          ;; We need to backfill the distribution key in the data, which
-          ;; we're implementing with a JOIN when we SELECT from the source
-          ;; table. We add the new field here.
-          ;;
-          (push new-column (table-field-list table))
-          (push new-column (table-column-list table))))))
+          (if column-definition
+              (progn
+                ;;
+                ;; Here also we need to add the new column to the PKEY
+                ;; definition, in first position.
+                ;;
+                (add-column-to-pkey table (column-name new-column))
+
+                ;;
+                ;; We need to backfill the distribution key in the data,
+                ;; which we're implementing with a JOIN when we SELECT from
+                ;; the source table. We add the new field here.
+                ;;
+                (push new-column (table-field-list table))
+                (push new-column (table-column-list table)))
+
+              ;;
+              ;; We don't have any table-field-list in the citus rule,
+              ;; meaning that the distribute ... using ... clause is lacking
+              ;; the FROM part, and we need it.
+              ;;
+              (error
+               (make-condition 'citus-rule-is-missing-from-list :rule rule)))))))
 
 
 (defun add-column-to-pkey (table column-name)