From d829ca9773b42e6c15af9f62a85e567ab2778589 Mon Sep 17 00:00:00 2001
From: Forbes Lindesay <forbes@lindesay.co.uk>
Date: Mon, 19 Oct 2020 18:20:56 +0100
Subject: [PATCH] fix: support update in pg-typed (#110)

The syntax was wrong for `update` and `insertOrUpdate`. Both of these now have tests.
---
 packages/pg-typed/src/__tests__/index.test.ts | 85 +++++++++++++++++++
 packages/pg-typed/src/index.ts                | 18 ++--
 packages/sql/src/SQLQuery.ts                  |  2 +-
 3 files changed, 98 insertions(+), 7 deletions(-)

diff --git a/packages/pg-typed/src/__tests__/index.test.ts b/packages/pg-typed/src/__tests__/index.test.ts
index 52a33882..a80a521d 100644
--- a/packages/pg-typed/src/__tests__/index.test.ts
+++ b/packages/pg-typed/src/__tests__/index.test.ts
@@ -80,4 +80,89 @@ test('create users', async () => {
     ]
   `);
   expect(await users(db).selectOne({id: ellie.id})).toEqual(ellie);
+
+  const updated = await photos(db).update(
+    {cdn_url: 'http://example.com/1'},
+    {
+      metadata: {rating: 5},
+    },
+  );
+  expect(updated.map((u) => [u.cdn_url, u.metadata])).toMatchInlineSnapshot(`
+    Array [
+      Array [
+        "http://example.com/1",
+        Object {
+          "rating": 5,
+        },
+      ],
+    ]
+  `);
+
+  const [forbes2, john] = await users(db).insertOrUpdate(
+    ['screen_name'],
+    {screen_name: 'Forbes', bio: 'Author of @databases'},
+    {screen_name: 'John', bio: 'Just a random name'},
+  );
+  expect(forbes2.id).toBe(forbes.id);
+  expect([forbes2.bio, john.bio]).toMatchInlineSnapshot(`
+    Array [
+      "Author of @databases",
+      "Just a random name",
+    ]
+  `);
+
+  await photos(db).delete({cdn_url: 'http://example.com/2'});
+
+  expect(
+    (await photos(db).select().orderByAsc('cdn_url').all()).map(
+      (u) => u.cdn_url,
+    ),
+  ).toMatchInlineSnapshot(`
+    Array [
+      "http://example.com/1",
+      "http://example.com/3",
+      "http://example.com/4",
+    ]
+  `);
+
+  const [john2, martin] = await users(db).insertOrIgnore(
+    {screen_name: 'John', bio: 'Updated bio'},
+    {screen_name: 'Martin', bio: 'Updated bio'},
+  );
+  expect(john2).toBe(null);
+  expect(martin!.screen_name).toBe('Martin');
+  expect(martin!.bio).toBe('Updated bio');
+
+  expect(
+    (await users(db).select().orderByAsc('screen_name').first())?.screen_name,
+  ).toMatchInlineSnapshot(`"Ellie"`);
+  expect(
+    (await users(db).select().orderByDesc('screen_name').first())?.screen_name,
+  ).toMatchInlineSnapshot(`"Martin"`);
+
+  expect(
+    (await users(db).select().orderByAsc('screen_name').all()).map((u) => [
+      u.screen_name,
+      u.bio,
+    ]),
+  ).toMatchInlineSnapshot(`
+    Array [
+      Array [
+        "Ellie",
+        null,
+      ],
+      Array [
+        "Forbes",
+        "Author of @databases",
+      ],
+      Array [
+        "John",
+        "Just a random name",
+      ],
+      Array [
+        "Martin",
+        "Updated bio",
+      ],
+    ]
+  `);
 });
diff --git a/packages/pg-typed/src/index.ts b/packages/pg-typed/src/index.ts
index 139ee91b..b6e315ee 100644
--- a/packages/pg-typed/src/index.ts
+++ b/packages/pg-typed/src/index.ts
@@ -118,7 +118,7 @@ class Table<TRecord, TInsertParameters> {
         );
         const query = sql`INSERT INTO ${this._tableID} (${columnNames}) VALUES (${values})`;
         if (onConflict) {
-          return sql`${query} ON CONFLICT DO ${onConflict(row)} RETURNING *`;
+          return sql`${query} ${onConflict(row)} RETURNING *`;
         } else {
           return sql`${query} RETURNING *`;
         }
@@ -134,14 +134,18 @@ class Table<TRecord, TInsertParameters> {
   }
 
   async insertOrUpdate<TRecordsToInsert extends readonly TInsertParameters[]>(
+    conflictKeys: [keyof TRecord, ...(keyof TRecord)[]],
     ...rows: TRecordsToInsert
   ): Promise<{[key in keyof TRecordsToInsert]: TRecord}> {
     const {sql} = this._underlyingDb;
     return this._insert(
       (row) =>
-        sql`UPDATE ${sql.join(
+        sql`ON CONFLICT (${sql.join(
+          conflictKeys.map((k) => sql.ident(k)),
+          sql`, `,
+        )}) DO UPDATE SET ${sql.join(
           Object.keys(row).map(
-            (key) => sql`${sql.ident(key)}=${sql.ident('EXCLUDED.', key)}`,
+            (key) => sql`${sql.ident(key)}=EXCLUDED.${sql.ident(key)}`,
           ),
           sql`, `,
         )}`,
@@ -151,9 +155,11 @@ class Table<TRecord, TInsertParameters> {
 
   async insertOrIgnore<TRecordsToInsert extends readonly TInsertParameters[]>(
     ...rows: TRecordsToInsert
-  ): Promise<TRecord[]> {
+  ): Promise<{[key in keyof TRecordsToInsert]: TRecord | null}> {
     const {sql} = this._underlyingDb;
-    return this._insert(() => sql`NOTHING`, ...rows);
+    return (await this._insert(() => sql`ON CONFLICT DO NOTHING`, ...rows)).map(
+      (row) => row ?? null,
+    ) as any;
   }
 
   async update(
@@ -169,7 +175,7 @@ class Table<TRecord, TInsertParameters> {
       sql`, `,
     );
     return await this.untypedQuery(
-      sql`UPDATE ${this._tableID} ${where} SET ${setClause} RETURNING *`,
+      sql`UPDATE ${this._tableID} SET ${setClause} ${where} RETURNING *`,
     );
   }
 
diff --git a/packages/sql/src/SQLQuery.ts b/packages/sql/src/SQLQuery.ts
index 966cf3e8..acdc63f6 100644
--- a/packages/sql/src/SQLQuery.ts
+++ b/packages/sql/src/SQLQuery.ts
@@ -410,7 +410,7 @@ function escapePGIdentifier(str: string): string {
   }
   if (!/^[A-Za-z0-9_]*$/.test(str)) {
     throw new Error(
-      '@database/sql restricts postgres identifiers to alphanumeric characers and underscores.',
+      `@database/sql restricts postgres identifiers to alphanumeric characers and underscores. "${str}" is not allowed`,
     );
   }