Fix PostgreSQL visual query follow-ups

src/utils/identifiers.ts

@@ -31,9 +31,10 @@ export function getQuoteChar(
  */
 /** True when identifiers in generated SQL fragments should be double-quoted (PostgreSQL). */
 export function shouldQuoteIdentifiers(
-  driver: string | null | undefined,
+  driver: string | PluginManifest | null | undefined,
 ): boolean {
-  return driver === "postgres";
+  const driverStr = typeof driver === "object" ? driver?.id : driver;
+  return driverStr === "postgres" || driverStr === "postgresql";
 }
 
 // PostgreSQL folds unquoted identifiers to lowercase and only needs quotes for
@@ -50,7 +51,7 @@ const PG_RESERVED = new Set([
  */
 export function formatSqlIdentifier(
   identifier: string,
-  driver: string | null | undefined,
+  driver: string | PluginManifest | null | undefined,
 ): string {
   if (!shouldQuoteIdentifiers(driver)) return identifier;
   if (PG_SAFE_IDENTIFIER.test(identifier) && !PG_RESERVED.has(identifier)) {
@@ -61,7 +62,7 @@ export function formatSqlIdentifier(
 
 export function quoteIdentifier(
   identifier: string,
-  driver: string | null | undefined,
+  driver: string | PluginManifest | null | undefined,
 ): string {
   const quote = getQuoteChar(driver);
   const escaped =
@@ -78,7 +79,7 @@ export function quoteIdentifier(
  */
 export function quoteTableRef(
   table: string,
-  driver: string | null | undefined,
+  driver: string | PluginManifest | null | undefined,
   schema?: string | null,
 ): string {
   if (schema) {

src/utils/visualQuery.ts

@@ -67,7 +67,6 @@ function formatTableRef(
   tableName: string,
   driver: string | null | undefined,
 ): string {
-  if (!driver) return tableName;
   return tableName
     .split('.')
     .map((part) => formatSqlIdentifier(part, driver))
@@ -91,6 +90,13 @@ function formatGeneratedColumnRef(
   return `${alias}.${formatSqlIdentifier(nameParts.join('.'), driver)}`;
 }
 
+function formatAlias(
+  alias: string,
+  driver: string | null | undefined,
+): string {
+  return formatSqlIdentifier(alias, driver);
+}
+
 /**
  * Collects tables and their aliases from nodes
  */
@@ -105,9 +111,13 @@ export function collectTableAliases(nodes: QueryNode[]): Record<string, string>
 /**
  * Generates table list with aliases
  */
-export function generateTableList(nodes: QueryNode[], aliases: Record<string, string>): string[] {
+export function generateTableList(
+  nodes: QueryNode[],
+  aliases: Record<string, string>,
+  driver?: string | null,
+): string[] {
   return nodes.map((node) => {
-    const tableName = node.data.label;
+    const tableName = formatTableRef(node.data.label, driver);
     const alias = aliases[node.id];
     return `${tableName} ${alias}`;
   });
@@ -147,7 +157,7 @@ export function collectSelectedColumns(
             }
 
             if (agg?.alias) {
-              colExpr += ` AS ${agg.alias}`;
+              colExpr += ` AS ${formatAlias(agg.alias, driver)}`;
             }
 
             if (agg?.order !== undefined) {
@@ -157,7 +167,7 @@ export function collectSelectedColumns(
             nonAggregatedCols.push(columnRef);
 
             if (colAlias?.alias) {
-              colExpr += ` AS ${colAlias.alias}`;
+              colExpr += ` AS ${formatAlias(colAlias.alias, driver)}`;
             }
 
             if (colAlias?.order !== undefined) {
@@ -201,10 +211,7 @@ export function generateFromClause(
 ): string {
   if (nodes.length === 0) return '';
 
-  const tableList = nodes.map((node) => {
-    const tableName = formatTableRef(node.data.label, driver);
-    return `${tableName} ${aliases[node.id]}`;
-  });
+  const tableList = generateTableList(nodes, aliases, driver);
 
   if (edges.length === 0) {
     return '\nFROM\n  ' + tableList.join(',\n  ');
@@ -319,13 +326,16 @@ export function generateGroupByClause(
 /**
  * Generates HAVING clause for aggregate conditions
  */
-export function generateHavingClause(conditions: WhereCondition[]): string {
+export function generateHavingClause(
+  conditions: WhereCondition[],
+  driver?: string | null,
+): string {
   const aggregateConditions = conditions.filter((c) => c.isAggregate && c.column && c.value);
 
   if (aggregateConditions.length === 0) return '';
 
   const clauses = aggregateConditions.map((c, idx) => {
-    const condition = `${c.column} ${c.operator} ${c.value}`;
+    const condition = `${formatGeneratedColumnRef(c.column, driver)} ${c.operator} ${c.value}`;
     return idx === 0 ? condition : `${c.logicalOperator} ${condition}`;
   });
 
@@ -377,7 +387,7 @@ export function generateVisualQuerySQL(
   sql += generateFromClause(nodes, edges, aliases, driver);
   sql += generateWhereClause(whereConditions, driver);
   sql += generateGroupByClause(hasAggregation, nonAggregatedCols, groupBy, driver);
-  sql += generateHavingClause(whereConditions);
+  sql += generateHavingClause(whereConditions, driver);
   sql += generateOrderByClause(orderBy, driver);
   sql += generateLimitClause(limit);
 

tests/utils/identifiers.test.ts

@@ -5,6 +5,7 @@ import {
   quoteTableRef,
   formatSqlIdentifier,
 } from '../../src/utils/identifiers';
+import type { PluginManifest } from '../../src/types/plugins';
 
 describe('getQuoteChar', () => {
   it('should return backtick for mysql', () => {
@@ -114,6 +115,17 @@ describe('formatSqlIdentifier', () => {
     expect(formatSqlIdentifier('AccountId', 'postgres')).toBe('"AccountId"');
   });
 
+  it('should quote identifiers for postgresql driver ids', () => {
+    expect(formatSqlIdentifier('AccountId', 'postgresql')).toBe('"AccountId"');
+    expect(formatSqlIdentifier('user', 'postgresql')).toBe('"user"');
+  });
+
+  it('should quote identifiers for PostgreSQL plugin manifests', () => {
+    const manifest = { id: 'postgresql' } as PluginManifest;
+
+    expect(formatSqlIdentifier('AccountId', manifest)).toBe('"AccountId"');
+  });
+
   it('should quote reserved words for postgres', () => {
     expect(formatSqlIdentifier('select', 'postgres')).toBe('"select"');
     expect(formatSqlIdentifier('user', 'postgres')).toBe('"user"');
@@ -138,4 +150,4 @@ describe('formatSqlIdentifier', () => {
     expect(formatSqlIdentifier('users', 'sqlite')).toBe('users');
     expect(formatSqlIdentifier('AccountEventLog', 'sqlite')).toBe('AccountEventLog');
   });
-});
+});

tests/utils/visualQuery.test.ts

@@ -62,6 +62,19 @@ describe('visualQuery utils', () => {
       expect(result).toEqual(['users t1', 'posts t2']);
     });
 
+    it('should quote table names that need it for postgres', () => {
+      const nodes: QueryNode[] = [
+        { id: 'n1', data: { label: 'user', columns: [], selectedColumns: {} } },
+        { id: 'n2', data: { label: 'AccountEventLog', columns: [], selectedColumns: {} } },
+      ];
+      const aliases = { n1: 't1', n2: 't2' };
+
+      expect(generateTableList(nodes, aliases, 'postgres')).toEqual([
+        '"user" t1',
+        '"AccountEventLog" t2',
+      ]);
+    });
+
     it('should return empty array for no nodes', () => {
       expect(generateTableList([], {})).toEqual([]);
     });
@@ -155,6 +168,27 @@ describe('visualQuery utils', () => {
       expect(result.columns[0].expr).toBe('SUM(t1.total) AS total_sum');
     });
 
+    it('should quote aggregation aliases that need it for postgres', () => {
+      const nodes: QueryNode[] = [
+        {
+          id: 'n1',
+          data: {
+            label: 'orders',
+            columns: [{ name: 'total', type: 'DECIMAL' }],
+            selectedColumns: { total: true },
+            columnAggregations: {
+              total: { function: 'SUM', alias: 'Total Count' },
+            },
+          },
+        },
+      ];
+      const aliases = { n1: 't1' };
+
+      const result = collectSelectedColumns(nodes, aliases, 'postgres');
+
+      expect(result.columns[0].expr).toBe('SUM(t1.total) AS "Total Count"');
+    });
+
     it('should handle column aliases without aggregation', () => {
       const nodes: QueryNode[] = [
         {
@@ -177,6 +211,27 @@ describe('visualQuery utils', () => {
       expect(result.nonAggregatedCols).toContain('t1.first_name');
     });
 
+    it('should quote column aliases that need it for postgres', () => {
+      const nodes: QueryNode[] = [
+        {
+          id: 'n1',
+          data: {
+            label: 'users',
+            columns: [{ name: 'first_name', type: 'VARCHAR' }],
+            selectedColumns: { first_name: true },
+            columnAliases: {
+              first_name: { alias: 'Display Name' },
+            },
+          },
+        },
+      ];
+      const aliases = { n1: 't1' };
+
+      const result = collectSelectedColumns(nodes, aliases, 'postgres');
+
+      expect(result.columns[0].expr).toBe('t1.first_name AS "Display Name"');
+    });
+
     it('should handle custom ordering', () => {
       const nodes: QueryNode[] = [
         {
@@ -578,6 +633,16 @@ describe('visualQuery utils', () => {
     it('should return empty string for no aggregate conditions', () => {
       expect(generateHavingClause([])).toBe('');
     });
+
+    it('should quote generated HAVING column references for postgres', () => {
+      const conditions: WhereCondition[] = [
+        { id: '1', column: 't1.AccountId', operator: '>', value: '0', logicalOperator: 'AND', isAggregate: true },
+      ];
+
+      expect(generateHavingClause(conditions, 'postgres')).toBe(
+        '\nHAVING\n  t1."AccountId" > 0',
+      );
+    });
   });
 
   describe('generateOrderByClause', () => {
@@ -728,5 +793,48 @@ describe('visualQuery utils', () => {
       expect(result).toContain('t1."order"');
       expect(result).toContain('FROM\n  "user" t1');
     });
+
+    it('should quote postgres SQL when the driver id is postgresql', () => {
+      const nodes: QueryNode[] = [
+        {
+          id: 'n1',
+          data: {
+            label: 'AccountEventLog',
+            columns: [{ name: 'AccountId', type: 'INT' }],
+            selectedColumns: { AccountId: true },
+          },
+        },
+      ];
+
+      const result = generateVisualQuerySQL(nodes, [], [], [], [], '', 'postgresql');
+
+      expect(result).toContain('t1."AccountId"');
+      expect(result).toContain('FROM\n  "AccountEventLog" t1');
+    });
+
+    it('should generate postgres SQL with quoted HAVING refs and aliases', () => {
+      const nodes: QueryNode[] = [
+        {
+          id: 'n1',
+          data: {
+            label: 'AccountEventLog',
+            columns: [{ name: 'AccountId', type: 'INT' }],
+            selectedColumns: { AccountId: true },
+            columnAggregations: {
+              AccountId: { function: 'COUNT', alias: 'Total Count' },
+            },
+          },
+        },
+      ];
+      const whereConditions: WhereCondition[] = [
+        { id: '1', column: 't1.AccountId', operator: '>', value: '0', logicalOperator: 'AND', isAggregate: true },
+      ];
+
+      const result = generateVisualQuerySQL(nodes, [], whereConditions, [], [], '', 'postgres');
+
+      expect(result).toContain('COUNT(t1."AccountId") AS "Total Count"');
+      expect(result).toContain('FROM\n  "AccountEventLog" t1');
+      expect(result).toContain('HAVING\n  t1."AccountId" > 0');
+    });
   });
 });