fixes for alias handling (#156)

* fixes for alias handling

* merge main

Author: egan8888

meerkat-browser/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@devrev/meerkat-browser",
-  "version": "0.0.100",
+  "version": "0.0.101",
   "dependencies": {
     "tslib": "^2.3.0",
     "@devrev/meerkat-core": "*",

meerkat-core/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@devrev/meerkat-core",
-  "version": "0.0.100",
+  "version": "0.0.101",
   "dependencies": {
     "tslib": "^2.3.0"
   },

meerkat-core/src/cube-measure-transformer/cube-measure-transformer.ts

@@ -34,9 +34,13 @@ export const cubeMeasureToSQLSelectString = (
     if (i > 0) {
       base += ', ';
     }
+
+    // Note that tableSchemaName is not necessarily the same as tableSchema.name
+    // since tableSchema might be the merged tableSchema.
     let meerkatReplacedSqlString = meerkatPlaceholderReplacer(
       measureSchema.sql,
-      tableSchema.name
+      tableSchemaName,
+      tableSchema
     );
 
     /**

meerkat-core/src/get-final-base-sql/get-final-base-sql.spec.ts

@@ -44,7 +44,7 @@ describe('get final base sql', () => {
       getQueryOutput,
     });
     expect(result).toEqual(
-      'SELECT *, amount AS orders__amount, status AS orders__status FROM (select * from orders WHERE  ((orders.status IS NOT NULL))) AS orders'
+      'SELECT amount AS orders__amount, status AS orders__status, * FROM (select * from orders WHERE  ((orders.status IS NOT NULL))) AS orders'
     );
   });
   it('should not return measures in the projected base sql when filter param not passed', async () => {
@@ -62,7 +62,7 @@ describe('get final base sql', () => {
       getQueryOutput,
     });
     expect(result).toEqual(
-      'SELECT *, amount AS orders__amount, status AS orders__status FROM (select * from orders WHERE TRUE) AS orders'
+      'SELECT amount AS orders__amount, status AS orders__status, * FROM (select * from orders WHERE TRUE) AS orders'
     );
   });
 
@@ -110,7 +110,7 @@ describe('get final base sql', () => {
       getQueryOutput,
     });
     expect(result).toEqual(
-      'SELECT *, amount AS "order amount", status AS "order status" FROM (select * from orders WHERE  ((orders.status IS NOT NULL))) AS orders'
+      'SELECT amount AS "order amount", status AS "order status", * FROM (select * from orders WHERE  ((orders.status IS NOT NULL))) AS orders'
     );
   });
 });

meerkat-core/src/get-wrapped-base-query-with-projections/get-aliased-columns-from-filters.ts

@@ -116,40 +116,42 @@ const getFilterProjections = ({
 };
 
 export const getAliasedColumnsFromFilters = ({
-  baseSql,
   meerkatFilters,
   tableSchema,
   aliasedColumnSet,
   query,
 }: {
   meerkatFilters?: MeerkatQueryFilter[];
   tableSchema: TableSchema;
-  baseSql: string;
   aliasedColumnSet: Set<string>;
   query: Query;
 }) => {
-  let sql = baseSql;
+  const parts: string[] = [];
   const { measures } = query;
   meerkatFilters?.forEach((filter) => {
     if ('and' in filter) {
       // Traverse through the passed 'and' filters
-      sql += getAliasedColumnsFromFilters({
-        baseSql: '',
+      const sql = getAliasedColumnsFromFilters({
         meerkatFilters: filter.and,
         tableSchema,
         aliasedColumnSet,
         query,
       });
+      if (sql) {
+        parts.push(sql);
+      }
     }
     if ('or' in filter) {
       // Traverse through the passed 'or' filters
-      sql += getAliasedColumnsFromFilters({
-        baseSql: '',
+      const sql = getAliasedColumnsFromFilters({
         tableSchema,
         meerkatFilters: filter.or,
         aliasedColumnSet,
         query,
       });
+      if (sql) {
+        parts.push(sql);
+      }
     }
     if ('member' in filter) {
       const {
@@ -170,8 +172,11 @@ export const getAliasedColumnsFromFilters = ({
         aliasedColumnSet.add(aliasKey);
       }
       // Add the alias key to the set. So we have a reference to all the previously selected members.
-      sql += `, ${memberSql}`;
+      const sql = memberSql;
+      if (sql) {
+        parts.push(sql);
+      }
     }
   });
-  return sql;
+  return parts.join(', ');
 };

meerkat-core/src/get-wrapped-base-query-with-projections/get-wrapped-base-query-with-projections.ts

@@ -1,9 +1,7 @@
 import { getSelectReplacedSql } from '../cube-measure-transformer/cube-measure-transformer';
 import { Query, TableSchema } from '../types/cube-types';
 import { getAliasedColumnsFromFilters } from './get-aliased-columns-from-filters';
-import {
-  getProjectionClause
-} from './get-projection-clause';
+import { getProjectionClause } from './get-projection-clause';
 
 interface GetWrappedBaseQueryWithProjectionsParams {
   baseQuery: string;
@@ -32,19 +30,17 @@ export const getWrappedBaseQueryWithProjections = ({
 
   const aliasFromFilters = getAliasedColumnsFromFilters({
     aliasedColumnSet,
-    baseSql: 'SELECT *',
     // setting measures to empty array, since we don't want to project measures present in the filters in the base query
     tableSchema: tableSchema,
     query,
     meerkatFilters: query.filters,
   });
 
-  const formattedMemberProjection = memberProjections
-    ? `, ${memberProjections}`
-    : '';
-
-  const finalAliasedColumnsClause =
-    aliasFromFilters + formattedMemberProjection;
+  const parts = [aliasFromFilters, memberProjections].filter(
+    (part) => part !== ''
+  );
+  parts.push('*');
+  const finalAliasedColumnsClause = 'SELECT ' + parts.join(', ');
 
   const sqlWithFilterProjects = getSelectReplacedSql(
     newBaseSql,

meerkat-core/src/utils/meerkat-placeholder-replacer.spec.ts

@@ -1,41 +1,70 @@
+import { TableSchema } from '../types/cube-types';
 import { meerkatPlaceholderReplacer } from './meerkat-placeholder-replacer';
 
-describe("meerkatPlaceholderReplacer", () => {
-    it("should not replace placeholders with tableName if placeholder pattern doesnt end in .", () => {
-        const sql = "SELECT * FROM {MEERKAT}fieldName";
-        const tableName = "customers";
-        expect(meerkatPlaceholderReplacer(sql, tableName)).toEqual("SELECT * FROM {MEERKAT}fieldName");
-    });
-
-    it("should replace multiple placeholders in the SQL query", () => {
-        const sql = "SELECT {MEERKAT}.a, {MEERKAT}.b FROM orders";
-        const tableName = "orders";
-        expect(meerkatPlaceholderReplacer(sql, tableName)).toEqual('SELECT orders__a, orders__b FROM orders');
-    });
-
-    it("should be case sensitive", () => {
-        const sql = "SELECT {meerkat}.a FROM orders";
-        const tableName = "orders";
-        expect(meerkatPlaceholderReplacer(sql, tableName)).toEqual('SELECT {meerkat}.a FROM orders');
-    });
-
-    it("should replace the correct match", () => {
-        const sql = "SELECT {MEERKAT.{MEERKAT}.}.a FROM customers";
-        const tableName = "customers";
-        expect(meerkatPlaceholderReplacer(sql, tableName)).toEqual("SELECT {MEERKAT.customers__}.a FROM customers");
-    });
-
-    it("should handle empty SQL queries", () => {
-        const sql = "";
-        const tableName = "customers";
-        expect(meerkatPlaceholderReplacer(sql, tableName)).toEqual('');
-    });
-
-    it("should handle SQL queries without placeholders", () => {
-        const sql = "SELECT * FROM customers.";
-        const tableName = "orders";
-        expect(meerkatPlaceholderReplacer(sql, tableName)).toEqual('SELECT * FROM customers.');
-    });
-
-    
+describe('meerkatPlaceholderReplacer', () => {
+  let tableSchema: TableSchema;
+  beforeEach(() => {
+    tableSchema = {
+      name: 'test',
+      sql: 'test',
+      measures: [
+        { name: 'measure1', sql: 'COUNT(*)', type: 'number', alias: 'alias1' },
+      ],
+      dimensions: [],
+    };
+  });
+
+  it('should not replace placeholders with tableName if placeholder pattern doesnt end in .', () => {
+    const sql = 'SELECT * FROM {MEERKAT}fieldName';
+    const tableName = 'customers';
+    expect(meerkatPlaceholderReplacer(sql, tableName, tableSchema)).toEqual(
+      'SELECT * FROM {MEERKAT}fieldName'
+    );
+  });
+
+  it('should replace multiple placeholders in the SQL query', () => {
+    const sql = 'SELECT {MEERKAT}.a, {MEERKAT}.b FROM orders';
+    const tableName = 'orders';
+    expect(meerkatPlaceholderReplacer(sql, tableName, tableSchema)).toEqual(
+      'SELECT orders__a, orders__b FROM orders'
+    );
+  });
+
+  it('should be case sensitive', () => {
+    const sql = 'SELECT {meerkat}.a FROM orders';
+    const tableName = 'orders';
+    expect(meerkatPlaceholderReplacer(sql, tableName, tableSchema)).toEqual(
+      'SELECT {meerkat}.a FROM orders'
+    );
+  });
+
+  it('should replace the correct match', () => {
+    const sql = 'SELECT {MEERKAT.{MEERKAT}.a}.a FROM customers';
+    const tableName = 'customers';
+    expect(meerkatPlaceholderReplacer(sql, tableName, tableSchema)).toEqual(
+      'SELECT {MEERKAT.customers__a}.a FROM customers'
+    );
+  });
+
+  it('should handle empty SQL queries', () => {
+    const sql = '';
+    const tableName = 'customers';
+    expect(meerkatPlaceholderReplacer(sql, tableName, tableSchema)).toEqual('');
+  });
+
+  it('should handle SQL queries without placeholders', () => {
+    const sql = 'SELECT * FROM customers.';
+    const tableName = 'orders';
+    expect(meerkatPlaceholderReplacer(sql, tableName, tableSchema)).toEqual(
+      'SELECT * FROM customers.'
+    );
+  });
+
+  it('should replace placeholders with alias if provided', () => {
+    const sql = 'SELECT {MEERKAT}.measure1 FROM orders';
+    const tableName = 'orders';
+    expect(meerkatPlaceholderReplacer(sql, tableName, tableSchema)).toEqual(
+      'SELECT "alias1" FROM orders'
+    );
+  });
 });

meerkat-core/src/utils/meerkat-placeholder-replacer.ts

@@ -1,9 +1,19 @@
-import { MEERKAT_OUTPUT_DELIMITER } from '../member-formatters/constants';
+import { getAliasFromSchema, getNamespacedKey } from '../member-formatters';
+import { TableSchema } from '../types/cube-types';
 
-export const meerkatPlaceholderReplacer = (sql: string, tableName: string) => {
-  const tableNameEncapsulationRegEx = /\{MEERKAT\}\./g;
-  return sql.replace(
-    tableNameEncapsulationRegEx,
-    tableName + MEERKAT_OUTPUT_DELIMITER
-  );
+export const meerkatPlaceholderReplacer = (
+  sql: string,
+  originalTableName: string,
+  tableSchema: TableSchema
+) => {
+  const tableNameEncapsulationRegEx = /\{MEERKAT\}\.([a-zA-Z_][a-zA-Z0-9_]*)/g;
+  return sql.replace(tableNameEncapsulationRegEx, (_, columnName) => {
+    return getAliasFromSchema({
+      name: getNamespacedKey(originalTableName, columnName),
+      tableSchema,
+      aliasContext: {
+        isAstIdentifier: false,
+      },
+    });
+  });
 };

meerkat-node/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@devrev/meerkat-node",
-  "version": "0.0.100",
+  "version": "0.0.101",
   "dependencies": {
     "@swc/helpers": "~0.5.0",
     "@devrev/meerkat-core": "*",

meerkat-node/src/__tests__/aliases.spec.ts

@@ -92,7 +92,7 @@ describe('cube-to-sql', () => {
     const sql = await cubeQueryToSQL({ query, tableSchemas: [TABLE_SCHEMA] });
     console.info(`SQL for Simple Cube Query: `, sql);
     expect(sql).toBe(
-      `SELECT SUM(order_amount) AS "Total Order Amount" ,   "Customer ID" FROM (SELECT *, customer_id AS "Customer ID" FROM (select * from orders) AS orders) AS orders GROUP BY "Customer ID"`
+      `SELECT SUM(order_amount) AS "Total Order Amount" ,   "Customer ID" FROM (SELECT customer_id AS "Customer ID", * FROM (select * from orders) AS orders) AS orders GROUP BY "Customer ID"`
     );
     const output = await duckdbExec(sql);
     expect(output.length).toEqual(7);
@@ -118,7 +118,7 @@ describe('cube-to-sql', () => {
     const sql = await cubeQueryToSQL({ query, tableSchemas: [TABLE_SCHEMA] });
     console.info(`SQL for Simple Cube Query: `, sql);
     expect(sql).toBe(
-      `SELECT SUM(order_amount) AS "Total Order Amount" ,   "Customer ID" FROM (SELECT *, customer_id AS "Customer ID" FROM (select * from orders) AS orders) AS orders WHERE ("Customer ID" = '2') GROUP BY "Customer ID" HAVING ("Total Order Amount" = 100)`
+      `SELECT SUM(order_amount) AS "Total Order Amount" ,   "Customer ID" FROM (SELECT customer_id AS "Customer ID", * FROM (select * from orders) AS orders) AS orders WHERE ("Customer ID" = '2') GROUP BY "Customer ID" HAVING ("Total Order Amount" = 100)`
     );
     const output = await duckdbExec(sql);
     expect(output).toEqual([
@@ -147,7 +147,7 @@ describe('cube-to-sql', () => {
     const sql = await cubeQueryToSQL({ query, tableSchemas: [TABLE_SCHEMA] });
     console.info(`SQL for Simple Cube Query: `, sql);
     expect(sql).toBe(
-      `SELECT SUM(order_amount) AS "Total Order Amount" ,   "Customer ID" FROM (SELECT *, customer_id AS "Customer ID" FROM (select * from orders) AS orders) AS orders WHERE ("Customer ID" = '2') GROUP BY "Customer ID" ORDER BY "Total Order Amount" DESC`
+      `SELECT SUM(order_amount) AS "Total Order Amount" ,   "Customer ID" FROM (SELECT customer_id AS "Customer ID", * FROM (select * from orders) AS orders) AS orders WHERE ("Customer ID" = '2') GROUP BY "Customer ID" ORDER BY "Total Order Amount" DESC`
     );
     const output = await duckdbExec(sql);
     expect(output).toEqual([
@@ -157,4 +157,45 @@ describe('cube-to-sql', () => {
       },
     ]);
   });
+
+  it('Should handle alias that conflicts with another column', async () => {
+    const tableSchemaWithConflict = {
+      ...TABLE_SCHEMA,
+      dimensions: [
+        {
+          name: 'customer_id',
+          sql: 'customer_id',
+          type: 'string',
+          alias: 'order_id', // This conflicts with the orderr_id dimension
+        },
+      ],
+    };
+
+    const query: Query = {
+      measures: ['orders.total_order_amount'],
+      filters: [
+        {
+          member: 'orders.customer_id',
+          operator: 'equals',
+          values: ['2'],
+        },
+      ],
+      dimensions: ['orders.customer_id'],
+    };
+    const sql = await cubeQueryToSQL({
+      query,
+      tableSchemas: [tableSchemaWithConflict],
+    });
+    console.info(`SQL for Simple Cube Query: `, sql);
+    expect(sql).toBe(
+      `SELECT SUM(order_amount) AS "Total Order Amount" ,   "order_id" FROM (SELECT customer_id AS "order_id", * FROM (select * from orders) AS orders) AS orders WHERE (order_id = '2') GROUP BY order_id`
+    );
+    const output = await duckdbExec(sql);
+    expect(output).toEqual([
+      {
+        order_id: '2',
+        'Total Order Amount': 100,
+      },
+    ]);
+  });
 });