From 2d676ae660dad1f72926b63ee1f95cd69953b16a Mon Sep 17 00:00:00 2001 From: Angelelz Date: Tue, 12 Dec 2023 22:25:47 -0500 Subject: [PATCH 1/2] [Pg] FIX: correct string escaping for empty PgArrays --- drizzle-orm/src/pg-core/utils/array.ts | 7 ++- drizzle-orm/tests/makePgArray.test.ts | 26 +++++++- drizzle-orm/tests/parsePgArray.test.ts | 2 +- integration-tests/tests/pg.test.ts | 70 ++++++++++++++++----- integration-tests/tests/postgres.js.test.ts | 40 ++++++++++++ 5 files changed, 125 insertions(+), 20 deletions(-) diff --git a/drizzle-orm/src/pg-core/utils/array.ts b/drizzle-orm/src/pg-core/utils/array.ts index fef4d3fb2..5b6fa0f18 100644 --- a/drizzle-orm/src/pg-core/utils/array.ts +++ b/drizzle-orm/src/pg-core/utils/array.ts @@ -85,8 +85,11 @@ export function makePgArray(array: any[]): string { return makePgArray(item); } - if (typeof item === 'string' && item.includes(',')) { - return `"${item.replace(/"/g, '\\"')}"`; + if (typeof item === 'string') { + if (item.includes(',')) { + return `"${item.replace(/"/g, '\\"')}"`; + } + if (item === '') return '""'; } return `${item}`; diff --git a/drizzle-orm/tests/makePgArray.test.ts b/drizzle-orm/tests/makePgArray.test.ts index e37363a59..ed7185e33 100644 --- a/drizzle-orm/tests/makePgArray.test.ts +++ b/drizzle-orm/tests/makePgArray.test.ts @@ -47,7 +47,7 @@ describe.concurrent('makePgArray', () => { it('parses array with empty values', ({ expect }) => { const input = ['1', '', '3']; const output = table.a.mapToDriverValue(input); - expect(output).toEqual('{1,,3}'); + expect(output).toEqual('{1,"",3}'); }); it('parses array with empty nested values', ({ expect }) => { @@ -57,7 +57,7 @@ describe.concurrent('makePgArray', () => { ['7', '8', '9'], ]; const output = table.b.mapToDriverValue(input); - expect(output).toEqual('{{1,2,3},{,5,6},{7,8,9}}'); + expect(output).toEqual('{{1,2,3},{"",5,6},{7,8,9}}'); }); it('parses empty array', ({ expect }) => { @@ -117,4 +117,26 @@ describe.concurrent('makePgArray', () => { '{{1,"two \\"and a half\\", three",3},{four,"five \\"and a half\\", six",6},{seven,eight,nine}}', ); }); + + it('parses an array with null values', ({ expect }) => { + const input = ['1', null, '3']; + const output = table.a.mapToDriverValue(input); + expect(output).toEqual('{1,null,3}'); + }); + + it('parses an array with null values in nested arrays', ({ expect }) => { + const input = [ + ['1', '2', '3'], + [null, '5', '6'], + ['7', '8', '9'], + ]; + const output = table.b.mapToDriverValue(input); + expect(output).toEqual('{{1,2,3},{null,5,6},{7,8,9}}'); + }); + + it('parses string array with empty strings', ({ expect }) => { + const input = ['1', '', '3']; + const output = table.a.mapToDriverValue(input); + expect(output).toEqual('{1,"",3}'); + }); }); diff --git a/drizzle-orm/tests/parsePgArray.test.ts b/drizzle-orm/tests/parsePgArray.test.ts index 0b0abf4d2..c89da8f95 100644 --- a/drizzle-orm/tests/parsePgArray.test.ts +++ b/drizzle-orm/tests/parsePgArray.test.ts @@ -45,7 +45,7 @@ describe.concurrent('parsePgArray', () => { }); it('parses array with empty values', ({ expect }) => { - const input = '{1,,3}'; + const input = '{1,"",3}'; const output = table.a.mapFromDriverValue(input); expect(output).toEqual(['1', '', '3']); }); diff --git a/integration-tests/tests/pg.test.ts b/integration-tests/tests/pg.test.ts index 3c1b1c9d7..0c05c8bfd 100644 --- a/integration-tests/tests/pg.test.ts +++ b/integration-tests/tests/pg.test.ts @@ -9,25 +9,25 @@ import { arrayContains, arrayOverlaps, asc, + avg, + avgDistinct, + count, + countDistinct, eq, gt, gte, inArray, lt, + max, + min, name, placeholder, type SQL, sql, type SQLWrapper, - TransactionRollbackError, - count, - countDistinct, - avg, - avgDistinct, sum, sumDistinct, - max, - min, + TransactionRollbackError, } from 'drizzle-orm'; import { drizzle, type NodePgDatabase } from 'drizzle-orm/node-postgres'; import { migrate } from 'drizzle-orm/node-postgres/migrator'; @@ -50,6 +50,7 @@ import { macaddr, macaddr8, type PgColumn, + pgEnum, pgMaterializedView, pgTable, pgTableCreator, @@ -64,7 +65,6 @@ import { uniqueKeyName, uuid as pgUuid, varchar, - pgEnum, } from 'drizzle-orm/pg-core'; import getPort from 'get-port'; import pg from 'pg'; @@ -149,7 +149,7 @@ const aggregateTable = pgTable('aggregate_table', { a: integer('a'), b: integer('b'), c: integer('c'), - nullOnly: integer('null_only') + nullOnly: integer('null_only'), }); interface Context { @@ -523,7 +523,7 @@ test.serial('select distinct', async (t) => { const usersDistinctTable = pgTable('users_distinct', { id: integer('id').notNull(), name: text('name').notNull(), - age: integer('age').notNull() + age: integer('age').notNull(), }); await db.execute(sql`drop table if exists ${usersDistinctTable}`); @@ -534,7 +534,7 @@ test.serial('select distinct', async (t) => { { id: 1, name: 'John', age: 24 }, { id: 2, name: 'John', age: 25 }, { id: 1, name: 'Jane', age: 24 }, - { id: 1, name: 'Jane', age: 26 } + { id: 1, name: 'Jane', age: 26 }, ]); const users1 = await db.selectDistinct().from(usersDistinctTable).orderBy( usersDistinctTable.id, @@ -547,8 +547,8 @@ test.serial('select distinct', async (t) => { usersDistinctTable, ).orderBy(usersDistinctTable.name); const users4 = await db.selectDistinctOn([usersDistinctTable.id, usersDistinctTable.age]).from( - usersDistinctTable - ).orderBy(usersDistinctTable.id, usersDistinctTable.age) + usersDistinctTable, + ).orderBy(usersDistinctTable.id, usersDistinctTable.age); await db.execute(sql`drop table ${usersDistinctTable}`); @@ -556,7 +556,7 @@ test.serial('select distinct', async (t) => { { id: 1, name: 'Jane', age: 24 }, { id: 1, name: 'Jane', age: 26 }, { id: 1, name: 'John', age: 24 }, - { id: 2, name: 'John', age: 25 } + { id: 2, name: 'John', age: 25 }, ]); t.deepEqual(users2.length, 2); @@ -570,7 +570,7 @@ test.serial('select distinct', async (t) => { t.deepEqual(users4, [ { id: 1, name: 'John', age: 24 }, { id: 1, name: 'Jane', age: 26 }, - { id: 2, name: 'John', age: 25 } + { id: 2, name: 'John', age: 25 }, ]); }); @@ -3276,3 +3276,43 @@ test.serial('aggregate function: min', async (t) => { t.deepEqual(result1[0]?.value, 10); t.deepEqual(result2[0]?.value, null); }); + +test.serial('array mapping and parsing', async (t) => { + const { db } = t.context; + + const arrays = pgTable('arrays_tests', { + id: serial('id').primaryKey(), + tags: text('tags').array(), + nested: text('nested').array().array(), + numbers: integer('numbers').notNull().array(), + }); + + db.execute(sql`drop table if exists ${arrays}`); + db.execute(sql` + create table ${arrays} ( + id serial primary key, + tags text[], + nested text[][], + numbers integer[] + ) + `); + + // TODO support possoble null values + // @ts-ignore + await db.insert(arrays).values({ + tags: ['', 'b', 'c'], + nested: [['1', ''], ['3', '4']], + numbers: [1, 2, 3], + }); + + const result = await db.select().from(arrays); + + t.deepEqual(result, [{ + id: 1, + tags: ['', 'b', 'c'], + nested: [['1', ''], ['3', '4']], + numbers: [1, 2, 3], + }]); + + await db.execute(sql`drop table ${arrays}`); +}); diff --git a/integration-tests/tests/postgres.js.test.ts b/integration-tests/tests/postgres.js.test.ts index a1f979f60..58f14f8ce 100644 --- a/integration-tests/tests/postgres.js.test.ts +++ b/integration-tests/tests/postgres.js.test.ts @@ -2162,3 +2162,43 @@ test.serial('array operators', async (t) => { t.deepEqual(overlaps, [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }, { id: 5 }]); t.deepEqual(withSubQuery, [{ id: 1 }, { id: 3 }, { id: 5 }]); }); + +test.serial('array mapping and parsing', async (t) => { + const { db } = t.context; + + const arrays = pgTable('arrays_tests', { + id: serial('id').primaryKey(), + tags: text('tags').array(), + nested: text('nested').array().array(), + numbers: integer('numbers').notNull().array(), + }); + + db.execute(sql`drop table if exists ${arrays}`); + db.execute(sql` + create table ${arrays} ( + id serial primary key, + tags text[], + nested text[][], + numbers integer[] + ) + `); + + // TODO support possoble null values + // @ts-ignore + await db.insert(arrays).values({ + tags: ['', 'b', 'c'], + nested: [['1', ''], ['3', '4']], + numbers: [1, 2, 3], + }); + + const result = await db.select().from(arrays); + + t.deepEqual(result, [{ + id: 1, + tags: ['', 'b', 'c'], + nested: [['1', ''], ['3', '4']], + numbers: [1, 2, 3], + }]); + + await db.execute(sql`drop table ${arrays}`); +}); From 5c557abfc84457a0e117d6725dcc9419f583c43c Mon Sep 17 00:00:00 2001 From: Angelelz Date: Tue, 12 Dec 2023 23:28:31 -0500 Subject: [PATCH 2/2] [Pg] Improve string escaping and fixed ava tests --- drizzle-orm/src/pg-core/utils/array.ts | 5 +-- drizzle-orm/tests/makePgArray.test.ts | 36 ++++++++++++--------- drizzle-typebox/package.json | 2 +- drizzle-valibot/package.json | 2 +- drizzle-zod/package.json | 2 +- integration-tests/package.json | 2 +- integration-tests/tests/pg.test.ts | 6 ++-- integration-tests/tests/postgres.js.test.ts | 6 ++-- integration-tests/vitest.config.ts | 9 +++++- 9 files changed, 38 insertions(+), 32 deletions(-) diff --git a/drizzle-orm/src/pg-core/utils/array.ts b/drizzle-orm/src/pg-core/utils/array.ts index 5b6fa0f18..0bae7054e 100644 --- a/drizzle-orm/src/pg-core/utils/array.ts +++ b/drizzle-orm/src/pg-core/utils/array.ts @@ -86,10 +86,7 @@ export function makePgArray(array: any[]): string { } if (typeof item === 'string') { - if (item.includes(',')) { - return `"${item.replace(/"/g, '\\"')}"`; - } - if (item === '') return '""'; + return `"${item.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`; } return `${item}`; diff --git a/drizzle-orm/tests/makePgArray.test.ts b/drizzle-orm/tests/makePgArray.test.ts index ed7185e33..b7b27d7a5 100644 --- a/drizzle-orm/tests/makePgArray.test.ts +++ b/drizzle-orm/tests/makePgArray.test.ts @@ -16,7 +16,7 @@ describe.concurrent('makePgArray', () => { it('parses simple 1D array', ({ expect }) => { const input = ['1', '2', '3']; const output = table.a.mapToDriverValue(input); - expect(output).toEqual('{1,2,3}'); + expect(output).toEqual('{"1","2","3"}'); }); it('parses simple 2D array', ({ expect }) => { @@ -26,13 +26,13 @@ describe.concurrent('makePgArray', () => { ['7', '8', '9'], ]; const output = table.b.mapToDriverValue(input); - expect(output).toEqual('{{1,2,3},{4,5,6},{7,8,9}}'); + expect(output).toEqual('{{"1","2","3"},{"4","5","6"},{"7","8","9"}}'); }); it('parses array with quoted values', ({ expect }) => { const input = ['1', '2,3', '4']; const output = table.a.mapToDriverValue(input); - expect(output).toEqual('{1,"2,3",4}'); + expect(output).toEqual('{"1","2,3","4"}'); }); it('parses array with nested quoted values', ({ expect }) => { @@ -41,13 +41,13 @@ describe.concurrent('makePgArray', () => { ['5', '6,7', '8'], ]; const output = table.b.mapToDriverValue(input); - expect(output).toEqual('{{1,"2,3",4},{5,"6,7",8}}'); + expect(output).toEqual('{{"1","2,3","4"},{"5","6,7","8"}}'); }); it('parses array with empty values', ({ expect }) => { const input = ['1', '', '3']; const output = table.a.mapToDriverValue(input); - expect(output).toEqual('{1,"",3}'); + expect(output).toEqual('{"1","","3"}'); }); it('parses array with empty nested values', ({ expect }) => { @@ -57,7 +57,7 @@ describe.concurrent('makePgArray', () => { ['7', '8', '9'], ]; const output = table.b.mapToDriverValue(input); - expect(output).toEqual('{{1,2,3},{"",5,6},{7,8,9}}'); + expect(output).toEqual('{{"1","2","3"},{"","5","6"},{"7","8","9"}}'); }); it('parses empty array', ({ expect }) => { @@ -75,25 +75,25 @@ describe.concurrent('makePgArray', () => { it('parses single-level array with strings', ({ expect }) => { const input = ['one', 'two', 'three']; const output = table.a.mapToDriverValue(input); - expect(output).toEqual('{one,two,three}'); + expect(output).toEqual('{"one","two","three"}'); }); it('parses single-level array with mixed values', ({ expect }) => { const input = ['1', 'two', '3']; const output = table.a.mapToDriverValue(input); - expect(output).toEqual('{1,two,3}'); + expect(output).toEqual('{"1","two","3"}'); }); it('parses single-level array with commas inside quotes', ({ expect }) => { const input = ['1', 'two, three', '4']; const output = table.a.mapToDriverValue(input); - expect(output).toEqual('{1,"two, three",4}'); + expect(output).toEqual('{"1","two, three","4"}'); }); it('parses single-level array with escaped quotes inside quotes', ({ expect }) => { const input = ['1', 'two "three", four', '5']; const output = table.a.mapToDriverValue(input); - expect(output).toEqual('{1,"two \\"three\\", four",5}'); + expect(output).toEqual('{"1","two \\"three\\", four","5"}'); }); it('parses two-dimensional array with strings', ({ expect }) => { @@ -103,7 +103,7 @@ describe.concurrent('makePgArray', () => { ['seven', 'eight', 'nine'], ]; const output = table.b.mapToDriverValue(input); - expect(output).toEqual('{{one,two,three},{four,five,six},{seven,eight,nine}}'); + expect(output).toEqual('{{"one","two","three"},{"four","five","six"},{"seven","eight","nine"}}'); }); it('parses two-dimensional array with mixed values and escaped quotes', ({ expect }) => { @@ -114,14 +114,14 @@ describe.concurrent('makePgArray', () => { ]; const output = table.b.mapToDriverValue(input); expect(output).toEqual( - '{{1,"two \\"and a half\\", three",3},{four,"five \\"and a half\\", six",6},{seven,eight,nine}}', + '{{"1","two \\"and a half\\", three","3"},{"four","five \\"and a half\\", six","6"},{"seven","eight","nine"}}', ); }); it('parses an array with null values', ({ expect }) => { const input = ['1', null, '3']; const output = table.a.mapToDriverValue(input); - expect(output).toEqual('{1,null,3}'); + expect(output).toEqual('{"1",null,"3"}'); }); it('parses an array with null values in nested arrays', ({ expect }) => { @@ -131,12 +131,18 @@ describe.concurrent('makePgArray', () => { ['7', '8', '9'], ]; const output = table.b.mapToDriverValue(input); - expect(output).toEqual('{{1,2,3},{null,5,6},{7,8,9}}'); + expect(output).toEqual('{{"1","2","3"},{null,"5","6"},{"7","8","9"}}'); }); it('parses string array with empty strings', ({ expect }) => { const input = ['1', '', '3']; const output = table.a.mapToDriverValue(input); - expect(output).toEqual('{1,"",3}'); + expect(output).toEqual('{"1","","3"}'); + }); + + it('parses string array with backlash strings', ({ expect }) => { + const input = ['1', '\n', '3\\']; + const output = table.a.mapToDriverValue(input); + expect(output).toEqual('{"1","\n","3\\\\"}'); }); }); diff --git a/drizzle-typebox/package.json b/drizzle-typebox/package.json index cdeee64f9..a0fc7e28d 100644 --- a/drizzle-typebox/package.json +++ b/drizzle-typebox/package.json @@ -9,7 +9,7 @@ "test:types": "cd tests && tsc", "pack": "(cd dist && npm pack --pack-destination ..) && rm -f package.tgz && mv *.tgz package.tgz", "publish": "npm publish package.tgz", - "test": "ava tests" + "test": "NODE_OPTIONS='--loader=tsx --no-warnings' ava" }, "exports": { ".": { diff --git a/drizzle-valibot/package.json b/drizzle-valibot/package.json index 278e95a11..86d074934 100644 --- a/drizzle-valibot/package.json +++ b/drizzle-valibot/package.json @@ -9,7 +9,7 @@ "test:types": "cd tests && tsc", "pack": "(cd dist && npm pack --pack-destination ..) && rm -f package.tgz && mv *.tgz package.tgz", "publish": "npm publish package.tgz", - "test": "ava tests" + "test": "NODE_OPTIONS='--loader=tsx --no-warnings' ava" }, "exports": { ".": { diff --git a/drizzle-zod/package.json b/drizzle-zod/package.json index df6e47325..b5d21582f 100644 --- a/drizzle-zod/package.json +++ b/drizzle-zod/package.json @@ -9,7 +9,7 @@ "test:types": "cd tests && tsc", "pack": "(cd dist && npm pack --pack-destination ..) && rm -f package.tgz && mv *.tgz package.tgz", "publish": "npm publish package.tgz", - "test": "ava tests" + "test": "NODE_OPTIONS='--loader=tsx --no-warnings' ava" }, "exports": { ".": { diff --git a/integration-tests/package.json b/integration-tests/package.json index 745d1cbe0..ab3ffde7a 100644 --- a/integration-tests/package.json +++ b/integration-tests/package.json @@ -5,7 +5,7 @@ "type": "module", "scripts": { "test:types": "tsc", - "test": "ava tests --timeout=60s --serial && pnpm test:esm && pnpm test:rqb", + "test": "NODE_OPTIONS='--loader=tsx --no-warnings' ava --timeout=60s --serial && pnpm test:esm && pnpm test:rqb", "test:rqb": "vitest run --no-threads", "test:esm": "node tests/imports.test.mjs && node tests/imports.test.cjs" }, diff --git a/integration-tests/tests/pg.test.ts b/integration-tests/tests/pg.test.ts index 0c05c8bfd..f3f03c051 100644 --- a/integration-tests/tests/pg.test.ts +++ b/integration-tests/tests/pg.test.ts @@ -3297,11 +3297,9 @@ test.serial('array mapping and parsing', async (t) => { ) `); - // TODO support possoble null values - // @ts-ignore await db.insert(arrays).values({ tags: ['', 'b', 'c'], - nested: [['1', ''], ['3', '4']], + nested: [['1', ''], ['3', '\\a']], numbers: [1, 2, 3], }); @@ -3310,7 +3308,7 @@ test.serial('array mapping and parsing', async (t) => { t.deepEqual(result, [{ id: 1, tags: ['', 'b', 'c'], - nested: [['1', ''], ['3', '4']], + nested: [['1', ''], ['3', '\\a']], numbers: [1, 2, 3], }]); diff --git a/integration-tests/tests/postgres.js.test.ts b/integration-tests/tests/postgres.js.test.ts index 58f14f8ce..2d13717b7 100644 --- a/integration-tests/tests/postgres.js.test.ts +++ b/integration-tests/tests/postgres.js.test.ts @@ -2183,11 +2183,9 @@ test.serial('array mapping and parsing', async (t) => { ) `); - // TODO support possoble null values - // @ts-ignore await db.insert(arrays).values({ tags: ['', 'b', 'c'], - nested: [['1', ''], ['3', '4']], + nested: [['1', ''], ['3', '\\a']], numbers: [1, 2, 3], }); @@ -2196,7 +2194,7 @@ test.serial('array mapping and parsing', async (t) => { t.deepEqual(result, [{ id: 1, tags: ['', 'b', 'c'], - nested: [['1', ''], ['3', '4']], + nested: [['1', ''], ['3', '\\a']], numbers: [1, 2, 3], }]); diff --git a/integration-tests/vitest.config.ts b/integration-tests/vitest.config.ts index 261754895..12b7a1eb3 100644 --- a/integration-tests/vitest.config.ts +++ b/integration-tests/vitest.config.ts @@ -1,10 +1,17 @@ +import 'dotenv/config'; import { viteCommonjs } from '@originjs/vite-plugin-commonjs'; import tsconfigPaths from 'vite-tsconfig-paths'; import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { - include: ['tests/relational/**/*.test.ts', 'tests/libsql-batch.test.ts', 'tests/d1-batch.test.ts', 'tests/replicas/**/*', 'tests/imports/**/*'], + include: [ + 'tests/relational/**/*.test.ts', + 'tests/libsql-batch.test.ts', + 'tests/d1-batch.test.ts', + 'tests/replicas/**/*', + 'tests/imports/**/*', + ], exclude: [ ...(process.env.SKIP_PLANETSCALE_TESTS ? ['tests/relational/mysql.planetscale.test.ts'] : []), 'tests/relational/vercel.test.ts',