Merge pull request #648 from jvalue/641-sql-queries

[BUG] Handle column names that include double quotes

libs/execution/src/lib/types/io-types/table.ts

@@ -8,6 +8,7 @@ import { strict as assert } from 'assert';
 import {
   IOType,
   type InternalValueRepresentation,
+  type TextValuetype,
   type ValueType,
 } from '@jvalue/jayvee-language-server';
 
@@ -137,41 +138,47 @@ export class Table implements IOTypeImplementation<IOType.TABLE> {
     return `DROP TABLE IF EXISTS "${tableName}";`;
   }
 
-  generateInsertValuesStatement(tableName: string): string {
+  generateInsertValuesStatement(
+    tableName: string,
+    text: TextValuetype,
+  ): string {
     const valueRepresentationVisitor = new SQLValueRepresentationVisitor();
 
-    const columnNames = [...this.columns.keys()];
+    const columns = [...this.columns.entries()];
     const formattedRowValues: string[] = [];
     for (let rowIndex = 0; rowIndex < this.numberOfRows; ++rowIndex) {
       const rowValues: string[] = [];
-      for (const columnName of columnNames) {
-        const column = this.columns.get(columnName);
-        const entry = column?.values[rowIndex];
+      for (const [, column] of columns) {
+        const entry = column.values[rowIndex];
         assert(entry !== undefined);
-        const formattedValue = column?.valueType.acceptVisitor(
+        const formattedValue = column.valueType.acceptVisitor(
           valueRepresentationVisitor,
         )(entry);
-        assert(formattedValue !== undefined);
         rowValues.push(formattedValue);
       }
       formattedRowValues.push(`(${rowValues.join(',')})`);
     }
 
-    const formattedColumns = columnNames.map((c) => `"${c}"`).join(',');
+    const formattedColumns = columns
+      .map(([colName]) => {
+        return text.acceptVisitor(valueRepresentationVisitor)(colName);
+      })
+      .join(',');
 
     return `INSERT INTO "${tableName}" (${formattedColumns}) VALUES ${formattedRowValues.join(
       ', ',
     )}`;
   }
 
-  generateCreateTableStatement(tableName: string): string {
+  generateCreateTableStatement(tableName: string, text: TextValuetype): string {
     const columnTypeVisitor = new SQLColumnTypeVisitor();
+    const valueRepresentationVisitor = new SQLValueRepresentationVisitor();
 
     const columns = [...this.columns.entries()];
     const columnStatements = columns.map(([columnName, column]) => {
-      return `"${columnName}" ${column.valueType.acceptVisitor(
-        columnTypeVisitor,
-      )}`;
+      return `${text.acceptVisitor(valueRepresentationVisitor)(
+        columnName,
+      )} ${column.valueType.acceptVisitor(columnTypeVisitor)}`;
     });
 
     return `CREATE TABLE IF NOT EXISTS "${tableName}" (${columnStatements.join(

libs/extensions/rdbms/exec/src/lib/postgres-loader-executor.spec.ts

@@ -134,11 +134,11 @@ describe('Validation of PostgresLoaderExecutor', () => {
       );
       expect(databaseQueryMock).nthCalledWith(
         2,
-        `CREATE TABLE IF NOT EXISTS "Test" ("Column1" text,"Column2" real);`,
+        `CREATE TABLE IF NOT EXISTS "Test" ('Column1' text,'Column2' real);`,
       );
       expect(databaseQueryMock).nthCalledWith(
         3,
-        `INSERT INTO "Test" ("Column1","Column2") VALUES ('value 1',20.2)`,
+        `INSERT INTO "Test" ('Column1','Column2') VALUES ('value 1',20.2)`,
       );
       expect(databaseEndMock).toBeCalledTimes(1);
     }

libs/extensions/rdbms/exec/src/lib/postgres-loader-executor.ts

@@ -74,11 +74,21 @@ export class PostgresLoaderExecutor extends AbstractBlockExecutor<
       );
       await client.query(Table.generateDropTableStatement(table));
       context.logger.logDebug(`Creating table "${table}"`);
-      await client.query(input.generateCreateTableStatement(table));
+      await client.query(
+        input.generateCreateTableStatement(
+          table,
+          context.valueTypeProvider.Primitives.Text,
+        ),
+      );
       context.logger.logDebug(
         `Inserting ${input.getNumberOfRows()} row(s) into table "${table}"`,
       );
-      await client.query(input.generateInsertValuesStatement(table));
+      await client.query(
+        input.generateInsertValuesStatement(
+          table,
+          context.valueTypeProvider.Primitives.Text,
+        ),
+      );
 
       context.logger.logDebug(
         `The data was successfully loaded into the database`,

libs/extensions/rdbms/exec/src/lib/sqlite-loader-executor.spec.ts

@@ -157,18 +157,118 @@ describe('Validation of SQLiteLoaderExecutor', () => {
       );
       expect(databaseRunMock).nthCalledWith(
         2,
-        `CREATE TABLE IF NOT EXISTS "Test" ("Column1" text,"Column2" real);`,
+        `CREATE TABLE IF NOT EXISTS "Test" ('Column1' text,'Column2' real);`,
         expect.any(Function),
       );
       expect(databaseRunMock).nthCalledWith(
         3,
-        `INSERT INTO "Test" ("Column1","Column2") VALUES ('value 1',20.2)`,
+        `INSERT INTO "Test" ('Column1','Column2') VALUES ('value 1',20.2)`,
         expect.any(Function),
       );
       expect(databaseCloseMock).toBeCalledTimes(1);
     }
   });
 
+  it('should diagnose no error on column names with quotes', async () => {
+    mockDatabaseDefault();
+    databaseRunMock.mockImplementation(
+      (sql: string, callback: SqliteRunCallbackType) => {
+        callback(
+          {
+            lastID: 0,
+            changes: 0,
+          } as sqlite3.RunResult,
+          null,
+        );
+        return this;
+      },
+    );
+    const text = readJvTestAsset('valid-sqlite-loader.jv');
+
+    const inputTable = constructTable(
+      [
+        {
+          columnName: '"ColumnWithDoubleQuotes"',
+          column: {
+            values: ['value 1'],
+            valueType: services.ValueTypeProvider.Primitives.Text,
+          },
+        },
+        {
+          columnName: "'ColumnWithSingleQuotes'",
+          column: {
+            values: [20.2],
+            valueType: services.ValueTypeProvider.Primitives.Decimal,
+          },
+        },
+      ],
+      1,
+    );
+    const result = await parseAndExecuteExecutor(text, inputTable);
+
+    expect(R.isErr(result)).toEqual(false);
+    if (R.isOk(result)) {
+      expect(result.right.ioType).toEqual(IOType.NONE);
+      expect(databaseRunMock).toBeCalledTimes(3);
+      expect(databaseRunMock).nthCalledWith(
+        1,
+        'DROP TABLE IF EXISTS "Test";',
+        expect.any(Function),
+      );
+      expect(databaseRunMock).nthCalledWith(
+        2,
+        `CREATE TABLE IF NOT EXISTS "Test" ('"ColumnWithDoubleQuotes"' text,'''ColumnWithSingleQuotes''' real);`,
+        expect.any(Function),
+      );
+      expect(databaseRunMock).nthCalledWith(
+        3,
+        `INSERT INTO "Test" ('"ColumnWithDoubleQuotes"','''ColumnWithSingleQuotes''') VALUES ('value 1',20.2)`,
+        expect.any(Function),
+      );
+      expect(databaseCloseMock).toBeCalledTimes(1);
+    }
+  });
+
+  it('should diagnose error on query error', async () => {
+    mockDatabaseDefault();
+    databaseRunMock.mockImplementation(
+      (sql: string, callback: SqliteRunCallbackType) => {
+        callback(
+          {
+            lastID: 0,
+            changes: 0,
+          } as sqlite3.RunResult,
+          new Error('mock error message'),
+        );
+        return this;
+      },
+    );
+    const text = readJvTestAsset('valid-sqlite-loader.jv');
+
+    const inputTable = constructTable(
+      [
+        {
+          columnName: 'Column1',
+          column: {
+            values: ['value 1'],
+            valueType: services.ValueTypeProvider.Primitives.Text,
+          },
+        },
+      ],
+      1,
+    );
+    const result = await parseAndExecuteExecutor(text, inputTable);
+
+    expect(R.isOk(result)).toEqual(false);
+    if (R.isErr(result)) {
+      expect(result.left.message).toEqual(
+        'Could not write to sqlite database: mock error message',
+      );
+      expect(databaseRunMock).toBeCalledTimes(1);
+      expect(databaseCloseMock).toBeCalledTimes(1);
+    }
+  });
+
   it('should diagnose error on sqlite database open error', async () => {
     databaseMock.mockImplementation(() => {
       throw new Error('File not found');

libs/extensions/rdbms/exec/src/lib/sqlite-loader-executor.ts

@@ -57,11 +57,23 @@ export class SQLiteLoaderExecutor extends AbstractBlockExecutor<
       }
 
       context.logger.logDebug(`Creating table "${table}"`);
-      await this.runQuery(db, input.generateCreateTableStatement(table));
+      await this.runQuery(
+        db,
+        input.generateCreateTableStatement(
+          table,
+          context.valueTypeProvider.Primitives.Text,
+        ),
+      );
       context.logger.logDebug(
         `Inserting ${input.getNumberOfRows()} row(s) into table "${table}"`,
       );
-      await this.runQuery(db, input.generateInsertValuesStatement(table));
+      await this.runQuery(
+        db,
+        input.generateInsertValuesStatement(
+          table,
+          context.valueTypeProvider.Primitives.Text,
+        ),
+      );
 
       context.logger.logDebug(
         `The data was successfully loaded into the database`,
@@ -84,9 +96,15 @@ export class SQLiteLoaderExecutor extends AbstractBlockExecutor<
     query: string,
   ): Promise<sqlite3.RunResult> {
     return new Promise((resolve, reject) => {
-      db.run(query, (result: sqlite3.RunResult, error: Error | null) =>
-        error ? reject(error) : resolve(result),
-      );
+      db.run(query, (result: sqlite3.RunResult, error: Error | null) => {
+        if (error) {
+          reject(error);
+        } else if (result instanceof Error) {
+          reject(result);
+        } else {
+          resolve(result);
+        }
+      });
     });
   }
 }

package.json

@@ -9,6 +9,7 @@
     "lint": "nx run-many --target lint --max-warnings 0",
     "test": "nx run-many --target test",
     "generate": "nx run language-server:generate",
+    "reproduce": "nx run interpreter:run -d repro.jv -dg exhaustive",
     "example:cars": "nx run interpreter:run -d example/cars.jv -dg peek",
     "example:gtfs": "nx run interpreter:run -d example/gtfs-static.jv -dg peek",
     "example:gtfs-rt": "nx run interpreter:run -d example/gtfs-rt.jv -dg peek",