Constraints - naming and comments

docs/columns.md

@@ -11,8 +11,9 @@ The `createTable` and `addColumns` methods both take a `columns` argument that s
 - `notNull` _[boolean]_ - set to true to make this column not null
 - `default` _[string]_ - adds DEFAULT clause for column. Accepts null, a literal value, or a `pgm.func()` expression.
 - `check` _[string]_ - sql for a check constraint for this column
-- `references` _[[Name](migrations.md#type)]_ - a table name that this column is a foreign key to
+- `references` _[[Name](migrations.md#type) or string]_ - a table name that this column is a foreign key to
 - `referencesConstraintName` _[string]_ - name of the created constraint
+- `referencesConstraintComment` _[string]_ - comment on the created constraint
 - `onDelete` _[string]_ - adds ON DELETE constraint for a reference column
 - `onUpdate` _[string]_ - adds ON UPDATE constraint for a reference column
 - `match` _[string]_ - `FULL` or `SIMPLE`

docs/constraints.md

@@ -15,10 +15,12 @@
   - `exclude` _[string]_ - sql for an exclude constraint
   - `deferrable` _[boolean]_ - flag for deferrable table constraint
   - `deferred` _[boolean]_ - flag for initially deferred deferrable table constraint
+  - `comment` _[string]_ - comment on a singular, named constraint
   - `foreignKeys` _[object or array of objects]_ - foreign keys specification
     - `columns` _[[Name](migrations.md#type) or array of [Names](migrations.md#type)]_ - names of columns
     - `references` _[[Name](migrations.md#type)]_ - names of foreign table and column names
-    - `referencesConstraintName` _[string]_ - name of the created constraint
+    - `referencesConstraintName` _[string]_ - name of the created constraint (only necessary when creating multiple constraints)
+    - `referencesConstraintComment` _[string]_ - comment on the individual foreign key constraint
     - `onDelete` _[string]_ - action to perform on delete
     - `onUpdate` _[string]_ - action to perform on update
     - `match` _[string]_ - `FULL` or `SIMPLE`

docs/tables.md

@@ -12,7 +12,7 @@
   - `temporary` _[bool]_ - default false
   - `ifNotExists` _[bool]_ - default false
   - `inherits` _[[Name](migrations.md#type)]_ - table(s) to inherit from
-  - `constraints` _[object]_ - table constraints see [add constraint](constraints.md#pgmaddconstraint-tablename-constraint_name-expression-)
+  - `constraints` _[object]_ - table constraints see `expression` of [add constraint](constraints.md#pgmaddconstraint-tablename-constraint_name-expression-)
   - `like` either:
     - _[[Name](migrations.md#type)]_ - table(s) to inherit from
     - or _[object]_

lib/operations/tables.js

@@ -12,19 +12,11 @@ const {
 const { parseSequenceOptions } = require("./sequences");
 
 const parseReferences = options => {
-  const {
-    references,
-    referencesConstraintName,
-    match,
-    onDelete,
-    onUpdate
-  } = options;
+  const { references, match, onDelete, onUpdate } = options;
   const clauses = [];
-  if (referencesConstraintName) {
-    clauses.push(`CONSTRAINT "${referencesConstraintName}"`);
-  }
   clauses.push(
-    typeof references === "string"
+    typeof references === "string" &&
+      (references.startsWith('"') || references.endsWith(")"))
       ? `REFERENCES ${references}`
       : template`REFERENCES "${references}"`
   );
@@ -40,12 +32,8 @@ const parseReferences = options => {
   return clauses.join(" ");
 };
 
-const parseDeferrable = options => {
-  const { deferrable, deferred } = options;
-  return deferrable
-    ? `DEFERRABLE INITIALLY ${deferred ? "DEFERRED" : "IMMEDIATE"}`
-    : null;
-};
+const parseDeferrable = options =>
+  `DEFERRABLE INITIALLY ${options.deferred ? "DEFERRED" : "IMMEDIATE"}`;
 
 const parseColumns = (tableName, columns, extendingTypeShorthands = {}) => {
   let columnsWithOptions = _.mapValues(columns, column =>
@@ -89,6 +77,8 @@ const parseColumns = (tableName, columns, extendingTypeShorthands = {}) => {
         notNull,
         check,
         references,
+        referencesConstraintName,
+        referencesConstraintComment,
         deferrable,
         generated
       } = options;
@@ -112,7 +102,21 @@ const parseColumns = (tableName, columns, extendingTypeShorthands = {}) => {
         constraints.push(`CHECK (${check})`);
       }
       if (references) {
-        constraints.push(parseReferences(options));
+        const name =
+          referencesConstraintName ||
+          (referencesConstraintComment ? `${tableName}_fk_${columnName}` : "");
+        constraints.push(
+          (name ? `CONSTRAINT "${name}" ` : "") + parseReferences(options)
+        );
+        if (referencesConstraintComment) {
+          comments.push(
+            comment(
+              `CONSTRAINT "${name}" ON`,
+              tableName,
+              referencesConstraintComment
+            )
+          );
+        }
       }
       if (deferrable) {
         constraints.push(parseDeferrable(options));
@@ -137,72 +141,101 @@ const parseColumns = (tableName, columns, extendingTypeShorthands = {}) => {
 
       return template`"${columnName}" ${sType}${constraintsStr}`;
     }),
-    constraints: {
-      ...(multiplePrimaryColumns ? { primaryKey: primaryColumns } : {})
-    },
+    constraints: multiplePrimaryColumns ? { primaryKey: primaryColumns } : {},
     comments
   };
 };
 
-const parseConstraints = (table, options, genName) => {
+const parseConstraints = (table, options, optionName) => {
   const {
     check,
     unique,
     primaryKey,
     foreignKeys,
     exclude,
-    deferrable
+    deferrable,
+    comment: optionComment
   } = options;
   const tableName = typeof table === "object" ? table.name : table;
-  const constraints = [];
+  let constraints = [];
+  const comments = [];
   if (check) {
     if (_.isArray(check)) {
       check.forEach((ch, i) => {
-        const name = genName ? `CONSTRAINT "${tableName}_chck_${i + 1}" ` : "";
-        constraints.push(`${name}CHECK (${ch})`);
+        const name = optionName || `${tableName}_chck_${i + 1}`;
+        constraints.push(`CONSTRAINT "${name}" CHECK (${ch})`);
       });
     } else {
-      const name = genName ? `CONSTRAINT "${tableName}_chck" ` : "";
-      constraints.push(`${name}CHECK (${check})`);
+      const name = optionName || `${tableName}_chck`;
+      constraints.push(`CONSTRAINT "${name}" CHECK (${check})`);
     }
   }
   if (unique) {
     const uniqueArray = _.isArray(unique) ? unique : [unique];
     const isArrayOfArrays = uniqueArray.some(uniqueSet => _.isArray(uniqueSet));
     (isArrayOfArrays ? uniqueArray : [uniqueArray]).forEach(uniqueSet => {
       const cols = _.isArray(uniqueSet) ? uniqueSet : [uniqueSet];
-      const name = genName
-        ? `CONSTRAINT "${tableName}_uniq_${cols.join("_")}" `
-        : "";
-      constraints.push(`${name}UNIQUE (${quote(cols).join(", ")})`);
+      const name = optionName || `${tableName}_uniq_${cols.join("_")}`;
+      constraints.push(
+        `CONSTRAINT "${name}" UNIQUE (${quote(cols).join(", ")})`
+      );
     });
   }
   if (primaryKey) {
-    const name = genName ? `CONSTRAINT "${tableName}_pkey" ` : "";
+    const name = optionName || `${tableName}_pkey`;
     const key = quote(_.isArray(primaryKey) ? primaryKey : [primaryKey]).join(
       ", "
     );
-    constraints.push(`${name}PRIMARY KEY (${key})`);
+    constraints.push(`CONSTRAINT "${name}" PRIMARY KEY (${key})`);
   }
   if (foreignKeys) {
     (_.isArray(foreignKeys) ? foreignKeys : [foreignKeys]).forEach(fk => {
-      const { columns } = fk;
+      const {
+        columns,
+        referencesConstraintName,
+        referencesConstraintComment
+      } = fk;
       const cols = _.isArray(columns) ? columns : [columns];
-      const name = genName
-        ? `CONSTRAINT "${tableName}_fk_${cols.join("_")}" `
-        : "";
+      const name =
+        referencesConstraintName ||
+        optionName ||
+        `${tableName}_fk_${cols.join("_")}`;
       const key = quote(cols).join(", ");
-      constraints.push(`${name}FOREIGN KEY (${key}) ${parseReferences(fk)}`);
+      constraints.push(
+        `CONSTRAINT "${name}" FOREIGN KEY (${key}) ${parseReferences(fk)}`
+      );
+      if (referencesConstraintComment) {
+        comments.push(
+          comment(
+            `CONSTRAINT "${name}" ON`,
+            tableName,
+            referencesConstraintComment
+          )
+        );
+      }
     });
   }
   if (exclude) {
-    const name = genName ? `CONSTRAINT "${tableName}_excl" ` : "";
-    constraints.push(`${name}EXCLUDE ${exclude}`);
+    const name = optionName || `${tableName}_excl`;
+    constraints.push(`CONSTRAINT "${name}" EXCLUDE ${exclude}`);
   }
 
-  return deferrable
-    ? constraints.map(constraint => `${constraint} ${parseDeferrable(options)}`)
-    : constraints;
+  if (deferrable) {
+    constraints = constraints.map(
+      constraint => `${constraint} ${parseDeferrable(options)}`
+    );
+  }
+  if (optionComment) {
+    if (!optionName)
+      throw new Error("cannot comment on unspecified constraints");
+    comments.push(
+      comment(`CONSTRAINT "${optionName}" ON`, tableName, optionComment)
+    );
+  }
+  return {
+    constraints,
+    comments
+  };
 };
 
 const parseLike = like => {
@@ -240,12 +273,12 @@ function createTable(typeShorthands) {
     } = options;
     const {
       columns: columnLines,
-      constraints: columnsConstraints,
+      constraints: crossColumnConstraints,
       comments: columnComments = []
     } = parseColumns(tableName, columns, typeShorthands);
     const dupes = _.intersection(
       Object.keys(optionsConstraints),
-      Object.keys(columnsConstraints)
+      Object.keys(crossColumnConstraints)
     );
     if (dupes.length > 0) {
       const dupesStr = dupes.join(", ");
@@ -254,8 +287,11 @@ function createTable(typeShorthands) {
       );
     }
 
-    const constraints = { ...optionsConstraints, ...columnsConstraints };
-    const constraintLines = parseConstraints(tableName, constraints, true);
+    const constraints = { ...optionsConstraints, ...crossColumnConstraints };
+    const {
+      constraints: constraintLines,
+      comments: constraintComments
+    } = parseConstraints(tableName, constraints, "");
     const tableDefinition = [...columnLines, ...constraintLines].concat(
       like ? [parseLike(like)] : []
     );
@@ -267,7 +303,7 @@ function createTable(typeShorthands) {
     const createTableQuery = template`CREATE TABLE${temporaryStr}${ifNotExistsStr} "${tableName}" (
 ${formatLines(tableDefinition)}
 )${inheritsStr};`;
-    const comments = columnComments;
+    const comments = [...columnComments, ...constraintComments];
     if (typeof tableComment !== "undefined") {
       comments.push(comment("TABLE ", tableName, tableComment));
     }
@@ -407,14 +443,15 @@ const undoRenameConstraint = (tableName, constraintName, newName) =>
   renameConstraint(tableName, newName, constraintName);
 
 function addConstraint(tableName, constraintName, expression) {
-  const constraint = constraintName ? `CONSTRAINT "${constraintName}" ` : "";
-  const constraintStr = formatLines(
+  const { constraints, comments } =
     typeof expression === "string"
-      ? [expression]
-      : parseConstraints(tableName, expression, false),
-    `  ADD ${constraint}`
-  );
-  return template`ALTER TABLE "${tableName}"\n${constraintStr};`;
+      ? { constraints: [expression], comments: [] }
+      : parseConstraints(tableName, expression, constraintName);
+  const constraintStr = formatLines(constraints, "  ADD ");
+  return [
+    template`ALTER TABLE "${tableName}"\n${constraintStr};`,
+    ...comments
+  ].join("\n");
 }
 
 function dropConstraint(tableName, constraintName, { ifExists, cascade } = {}) {

test/tables-test.js

@@ -145,6 +145,68 @@ describe("lib/operations/tables", () => {
   CONSTRAINT "my_table_name_uniq_c" UNIQUE ("c")
 );`);
     });
+
+    it("creates comments on foreign keys", () => {
+      const sql = Tables.createTable()(
+        "my_table_name",
+        {
+          a: { type: "integer" }
+        },
+        {
+          constraints: {
+            foreignKeys: {
+              columns: ["a"],
+              references: "other_table",
+              referencesConstraintComment: "example comment"
+            }
+          }
+        }
+      );
+      expect(sql).to.equal(`CREATE TABLE "my_table_name" (
+  "a" integer,
+  CONSTRAINT "my_table_name_fk_a" FOREIGN KEY ("a") REFERENCES "other_table"
+);
+COMMENT ON CONSTRAINT "my_table_name_fk_a" ON "my_table_name" IS $pg1$example comment$pg1$;`);
+    });
+
+    it("creates comments on column foreign keys", () => {
+      const sql = Tables.createTable()("my_table_name", {
+        a: {
+          type: "integer",
+          references: "other_table (a)",
+          referencesConstraintComment: "fk a comment"
+        },
+        b: {
+          type: "integer",
+          references: "other_table_two",
+          referencesConstraintName: "fk_b",
+          referencesConstraintComment: "fk b comment"
+        }
+      });
+      expect(sql).to.equal(`CREATE TABLE "my_table_name" (
+  "a" integer CONSTRAINT "my_table_name_fk_a" REFERENCES other_table (a),
+  "b" integer CONSTRAINT "fk_b" REFERENCES "other_table_two"
+);
+COMMENT ON CONSTRAINT "my_table_name_fk_a" ON "my_table_name" IS $pg1$fk a comment$pg1$;
+COMMENT ON CONSTRAINT "fk_b" ON "my_table_name" IS $pg1$fk b comment$pg1$;`);
+    });
+
+    it("creates no comments on unnamed constraints", () => {
+      expect(() =>
+        Tables.createTable()(
+          "my_table_name",
+          {
+            a: { type: "integer" }
+          },
+          {
+            constraints: {
+              primaryKey: "a",
+              comment: "example comment"
+            }
+          }
+        )
+      ).to.throw("cannot comment on unspecified constraints");
+    });
   });
 
   describe(".dropColumns", () => {
@@ -155,4 +217,16 @@ describe("lib/operations/tables", () => {
   DROP "c2";`);
     });
   });
+
+  describe(".addConstraint", () => {
+    it("can create comments", () => {
+      const sql = Tables.addConstraint("my_table", "my_constraint_name", {
+        primaryKey: "a",
+        comment: "this is an important primary key"
+      });
+      expect(sql).to.equal(`ALTER TABLE "my_table"
+  ADD CONSTRAINT "my_constraint_name" PRIMARY KEY ("a");
+COMMENT ON CONSTRAINT "my_constraint_name" ON "my_table" IS $pg1$this is an important primary key$pg1$;`);
+    });
+  });
 });