Parse queries in Slonik

.changeset/shy-oranges-beg.md

@@ -0,0 +1,7 @@
+---
+"@slonik/pg-driver": minor
+"@slonik/sql-tag": minor
+"slonik": minor
+---
+
+changing from extended queries to parsing queries

cspell.yml

@@ -15,4 +15,5 @@ words:
   - gajus
   - kuizinas
   - roarr
-  - plpgsql
+  - plpgsql
+  - pgsql

packages/pg-driver/src/factories/createPgDriverFactory.ts

@@ -111,8 +111,6 @@ const createClientConfiguration = (
     options: connectionOptions.options,
     password: connectionOptions.password,
     port: connectionOptions.port,
-    // @ts-expect-error - https://github.com/brianc/node-postgres/pull/3214
-    queryMode: 'extended',
     ssl: false,
     user: connectionOptions.username,
   };

packages/slonik/package.json

@@ -12,6 +12,7 @@
     "@slonik/utilities": "^45.6.0",
     "get-stack-trace": "^3.1.1",
     "iso8601-duration": "^1.3.0",
+    "pgsql-parser": "^13.16.0",
     "postgres-interval": "^4.0.2",
     "roarr": "^7.21.1",
     "serialize-error": "^8.0.0"
@@ -26,7 +27,6 @@
     "eslint-config-canonical": "^42.8.1",
     "expect-type": "^0.15.0",
     "get-port": "^5.1.1",
-    "nyc": "^15.1.0",
     "sinon": "^15.1.0",
     "ts-node": "^10.9.1",
     "typescript": "^5.4.5",
@@ -47,26 +47,6 @@
   "license": "BSD-3-Clause",
   "main": "./dist/index.js",
   "name": "slonik",
-  "nyc": {
-    "all": true,
-    "exclude": [
-      "src/bin",
-      "src/queries/*.ts",
-      "**/*.d.ts"
-    ],
-    "include": [
-      "src/**/*.ts"
-    ],
-    "reporter": [
-      "html",
-      "text-summary"
-    ],
-    "require": [
-      "ts-node/register/transpile-only"
-    ],
-    "silent": true,
-    "sourceMap": false
-  },
   "peerDependencies": {
     "zod": "^3"
   },
@@ -80,7 +60,7 @@
     "lint:cspell": "cspell . --no-progress --gitignore",
     "lint:eslint": "eslint --cache ./src",
     "lint:tsc": "tsc --noEmit",
-    "test": "nyc ava --verbose --serial"
+    "test": "ava --verbose --serial"
   },
   "types": "./dist/index.d.ts",
   "version": "45.6.0"

packages/slonik/src/factories/createQueryValidator.test.ts

@@ -0,0 +1,51 @@
+import { createQueryValidator } from './createQueryValidator';
+import { createSqlTag, stripSlonikPlaceholders } from '@slonik/sql-tag';
+import test from 'ava';
+
+const sql = createSqlTag();
+
+test('allows to pass a query with a single statement', async (t) => {
+  const query = sql.fragment`SELECT 1`;
+
+  const assertValidQuery = createQueryValidator();
+
+  await t.notThrowsAsync(async () => {
+    await assertValidQuery(query);
+  });
+});
+
+test('allows to pass a query with a single statement (with parameters)', async (t) => {
+  const query = sql.fragment`SELECT ${1}`;
+
+  const assertValidQuery = createQueryValidator();
+
+  await t.notThrowsAsync(async () => {
+    await assertValidQuery({
+      ...query,
+      sql: stripSlonikPlaceholders(query.sql),
+    });
+  });
+});
+
+test('allows to pass a query with comments', async (t) => {
+  const query = sql.fragment`
+    -- Hello, World!
+    SELECT 1
+  `;
+
+  const assertValidQuery = createQueryValidator();
+
+  await t.notThrowsAsync(async () => {
+    await assertValidQuery(query);
+  });
+});
+
+test('throws an error if a query contains multiple statements', async (t) => {
+  const query = sql.fragment`SELECT 1; SELECT 2`;
+
+  const assertValidQuery = createQueryValidator();
+
+  await t.throwsAsync(async () => {
+    await assertValidQuery(query);
+  });
+});

packages/slonik/src/factories/createQueryValidator.ts

@@ -0,0 +1,46 @@
+import { type Query } from '../types';
+import { InputSyntaxError, InvalidInputError } from '@slonik/errors';
+import { parseAsync } from 'pgsql-parser';
+
+export const createQueryValidator = () => {
+  const queries: Record<string, number> = {};
+
+  return async (query: Query) => {
+    let stats = queries[query.sql];
+
+    if (stats === undefined) {
+      let ast;
+
+      try {
+        ast = await parseAsync(query.sql);
+      } catch (error) {
+        throw new InputSyntaxError(error, query);
+      }
+
+      stats = ast.length;
+
+      // eslint-disable-next-line require-atomic-updates
+      queries[query.sql] = stats;
+    }
+
+    if (stats === 1) {
+      return;
+    }
+
+    if (stats === 0) {
+      throw new InputSyntaxError(
+        new InvalidInputError('Expected query to be provided'),
+        query,
+      );
+    }
+
+    if (stats > 1) {
+      throw new InputSyntaxError(
+        new InvalidInputError(
+          'Must not use multiple statements in a single query.',
+        ),
+        query,
+      );
+    }
+  };
+};

packages/slonik/src/helpers.test/createIntegrationTests.ts

@@ -139,6 +139,23 @@ export const createIntegrationTests = (
     await pool.end();
   });
 
+  // We have to test this scenario because we are using a custom query parser.
+  test('produces syntax error', async (t) => {
+    const pool = await createPool(t.context.dsn, {
+      driverFactory,
+    });
+
+    const error = await t.throwsAsync(
+      pool.query(sql.unsafe`
+        SELECT 1 foo bar baz
+      `),
+    );
+
+    t.true(error instanceof InputSyntaxError);
+  });
+
+  // It is important that we cover queries with and without parameters.
+  // That's because in the extended mode, only statements with parameters would trigger the error.
   test('produces error if multiple statements are passed as the query input (without parameters)', async (t) => {
     const pool = await createPool(t.context.dsn, {
       driverFactory,
@@ -150,11 +167,9 @@ export const createIntegrationTests = (
       `),
     );
 
-    t.true(error instanceof InvalidInputError);
+    t.true(error instanceof InputSyntaxError);
   });
 
-  // The difference between this test and the previous one is that this one is expected to fail before the query is executed.
-  // In case of pg driver, that is because of the { queryMode: 'extended' } setting.
   test('produces error if multiple statements are passed as the query input (with parameters)', async (t) => {
     const pool = await createPool(t.context.dsn, {
       driverFactory,

packages/slonik/src/routines/executeQuery.ts

@@ -1,6 +1,7 @@
 import { TRANSACTION_ROLLBACK_ERROR_PREFIX } from '../constants';
 import { transactionContext } from '../contexts/transactionContext';
 import { type ConnectionPoolClient } from '../factories/createConnectionPool';
+import { createQueryValidator } from '../factories/createQueryValidator';
 import { getPoolClientState } from '../state';
 import {
   type ClientConfiguration,
@@ -26,11 +27,14 @@ import {
 import {
   type PrimitiveValueExpression,
   type QuerySqlToken,
+  stripSlonikPlaceholders,
 } from '@slonik/sql-tag';
 import { defer, generateUid } from '@slonik/utilities';
 import { getStackTrace } from 'get-stack-trace';
 import { serializeError } from 'serialize-error';
 
+const assertValidQuery = createQueryValidator();
+
 type GenericQueryResult = StreamResult | QueryResult<QueryResultRow>;
 
 export type ExecutionRoutine = (
@@ -166,11 +170,12 @@ export const executeQuery = async (
   });
 
   const originalQuery = {
-    // See comments in `formatSlonikPlaceholder` for more information.
-    sql: query.sql.replaceAll('$slonik_', '$'),
+    sql: stripSlonikPlaceholders(query.sql),
     values: query.values,
   };
 
+  await assertValidQuery(originalQuery);
+
   let actualQuery = {
     ...originalQuery,
   };

packages/sql-tag/src/index.ts

@@ -37,3 +37,4 @@ export {
   type ValueExpression,
 } from './types';
 export { isSqlToken } from './utilities/isSqlToken';
+export { stripSlonikPlaceholders } from './utilities/stripSlonikPlaceholders';

packages/sql-tag/src/utilities/stripSlonikPlaceholders.ts

@@ -0,0 +1,6 @@
+/**
+ * See comments in `formatSlonikPlaceholder` for more information.
+ */
+export const stripSlonikPlaceholders = (sql: string) => {
+  return sql.replaceAll('$slonik_', '$');
+};