[release-12.3.1] Postgresql: Fix variable interpolation logic when the variable has multiple values (#114876)

Postgresql: Fix variable interpolation logic when the variable has multiple values (#114058)

* fix the variable interpolation

* add jest config to grafana-sql

* fix broken tests

* add variable interpolation tests

* lint

* apply fix only to postresql datasource

(cherry picked from commit 0291f6d1e7)

Co-authored-by: ismail simsek <ismailsimsek09@gmail.com>

packages/grafana-sql/jest.config.js

@@ -0,0 +1,5 @@
+const sharedConfig = require('../../jest.config.js');
+module.exports = {
+  ...sharedConfig,
+  rootDir: '../../',
+};

packages/grafana-sql/src/datasource/variable-interpolation.test.ts

@@ -0,0 +1,169 @@
+import { DataSourceInstanceSettings } from '@grafana/data';
+
+import { DB, SQLOptions, SqlQueryModel } from '../types';
+import { makeVariable } from '../utils/testHelpers';
+
+import { SqlDatasource } from './SqlDatasource';
+
+// Minimal test implementation of SqlDatasource
+class TestSqlDatasource extends SqlDatasource {
+  getDB(): DB {
+    return {} as DB;
+  }
+
+  getQueryModel(): SqlQueryModel {
+    return {
+      quoteLiteral: (value: string) => `'${value.replace(/'/g, "''")}'`,
+    } as SqlQueryModel;
+  }
+}
+
+describe('SqlDatasource - Variable Interpolation', () => {
+  const instanceSettings = {
+    jsonData: {
+      defaultProject: 'testproject',
+    },
+  } as unknown as DataSourceInstanceSettings<SQLOptions>;
+
+  let ds: TestSqlDatasource;
+
+  beforeEach(() => {
+    ds = new TestSqlDatasource(instanceSettings);
+  });
+
+  describe('Case 1: Multi-value enabled, single value selected', () => {
+    it('should escape single quotes in string value', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true });
+      // When we apply the general fix for all SQL data sources these should be uncommented
+      // expect(ds.interpolateVariable('value1', variable)).toEqual('value1');
+      // expect(ds.interpolateVariable("O'Brien", variable)).toEqual("O''Brien");
+      expect(ds.interpolateVariable('value1', variable)).toEqual(`'value1'`);
+      expect(ds.interpolateVariable("O'Brien", variable)).toEqual(`'O''Brien'`);
+    });
+
+    it('should handle numeric value', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true });
+      expect(ds.interpolateVariable(42 as unknown as string, variable)).toEqual(42);
+    });
+  });
+
+  describe('Case 2: Multi-value enabled, multiple values selected', () => {
+    it('should return quoted, comma-separated values', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true });
+      expect(ds.interpolateVariable(['value1', 'value2', 'value3'], variable)).toEqual("'value1','value2','value3'");
+    });
+
+    it('should escape single quotes in array values', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true });
+      expect(ds.interpolateVariable(["O'Brien", 'Smith', "D'Angelo"], variable)).toEqual(
+        "'O''Brien','Smith','D''Angelo'"
+      );
+    });
+
+    it('should handle empty array', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true });
+      expect(ds.interpolateVariable([], variable)).toEqual('');
+    });
+  });
+
+  describe('Case 3: Include all enabled, single value selected', () => {
+    it('should escape single quotes in string value', () => {
+      const variable = makeVariable('id1', 'name1', { includeAll: true });
+      // When we apply the general fix for all SQL data sources these should be uncommented
+      // expect(ds.interpolateVariable('value1', variable)).toEqual('value1');
+      // expect(ds.interpolateVariable("O'Brien", variable)).toEqual("O''Brien");
+      expect(ds.interpolateVariable('value1', variable)).toEqual(`'value1'`);
+      expect(ds.interpolateVariable("O'Brien", variable)).toEqual(`'O''Brien'`);
+    });
+
+    it('should handle numeric value', () => {
+      const variable = makeVariable('id1', 'name1', { includeAll: true });
+      expect(ds.interpolateVariable(123 as unknown as string, variable)).toEqual(123);
+    });
+  });
+
+  describe('Case 4: Include all enabled, "All" value selected', () => {
+    it('should handle All option as array', () => {
+      const variable = makeVariable('id1', 'name1', { includeAll: true });
+      expect(ds.interpolateVariable(['value1', 'value2', 'value3'], variable)).toEqual("'value1','value2','value3'");
+    });
+
+    it('should handle All option with special characters', () => {
+      const variable = makeVariable('id1', 'name1', { includeAll: true });
+      expect(ds.interpolateVariable(["test'1", 'test2', "test'3"], variable)).toEqual("'test''1','test2','test''3'");
+    });
+  });
+
+  describe('Case 5: No include all, no multi-value, single value selected', () => {
+    it('should escape single quotes in string value', () => {
+      const variable = makeVariable('id1', 'name1', { multi: false, includeAll: false });
+      expect(ds.interpolateVariable('value1', variable)).toEqual('value1');
+      expect(ds.interpolateVariable("O'Brien", variable)).toEqual("O''Brien");
+    });
+
+    it('should handle numeric value', () => {
+      const variable = makeVariable('id1', 'name1', { multi: false, includeAll: false });
+      expect(ds.interpolateVariable(999 as unknown as string, variable)).toEqual(999);
+    });
+
+    it('should handle empty string', () => {
+      const variable = makeVariable('id1', 'name1', { multi: false, includeAll: false });
+      expect(ds.interpolateVariable('', variable)).toEqual('');
+    });
+  });
+
+  describe('Case 6: Both include all and multi-value enabled, single value selected', () => {
+    it('should escape single quotes in string value', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true, includeAll: true });
+      // When we apply the general fix for all SQL data sources these should be uncommented
+      // expect(ds.interpolateVariable('value1', variable)).toEqual('value1');
+      // expect(ds.interpolateVariable("O'Brien", variable)).toEqual("O''Brien");
+      expect(ds.interpolateVariable('value1', variable)).toEqual(`'value1'`);
+      expect(ds.interpolateVariable("O'Brien", variable)).toEqual(`'O''Brien'`);
+    });
+
+    it('should handle numeric value', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true, includeAll: true });
+      expect(ds.interpolateVariable(456 as unknown as string, variable)).toEqual(456);
+    });
+  });
+
+  describe('Case 7: Both include all and multi-value enabled, "All" value selected', () => {
+    it('should handle All option as array', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true, includeAll: true });
+      expect(ds.interpolateVariable(['value1', 'value2', 'value3'], variable)).toEqual("'value1','value2','value3'");
+    });
+
+    it('should handle All option with mixed values', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true, includeAll: true });
+      expect(ds.interpolateVariable(['alpha', 'beta', 'gamma'], variable)).toEqual("'alpha','beta','gamma'");
+    });
+
+    it('should handle All option with special characters', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true, includeAll: true });
+      expect(ds.interpolateVariable(["it's", "can't", "won't"], variable)).toEqual("'it''s','can''t','won''t'");
+    });
+  });
+
+  describe('Case 8: Both include all and multi-value enabled, multiple values selected', () => {
+    it('should return quoted, comma-separated values', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true, includeAll: true });
+      expect(ds.interpolateVariable(['value1', 'value2'], variable)).toEqual("'value1','value2'");
+    });
+
+    it('should escape single quotes in array values', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true, includeAll: true });
+      expect(ds.interpolateVariable(["O'Brien", "D'Angelo"], variable)).toEqual("'O''Brien','D''Angelo'");
+    });
+
+    it('should handle single item array', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true, includeAll: true });
+      expect(ds.interpolateVariable(['value1'], variable)).toEqual("'value1'");
+    });
+
+    it('should handle array with single quote escaping', () => {
+      const variable = makeVariable('id1', 'name1', { multi: true, includeAll: true });
+      expect(ds.interpolateVariable(['a', "b'c", 'd'], variable)).toEqual("'a','b''c','d'");
+    });
+  });
+});

public/app/plugins/datasource/grafana-postgresql-datasource/datasource.test.ts

@@ -714,7 +714,7 @@ describe('PostgreSQLDatasource', () => {
       it('should return a quoted value', () => {
         const { ds, variable } = setupTestContext({});
         variable.multi = true;
-        expect(ds.interpolateVariable('abc', variable)).toEqual("'abc'");
+        expect(ds.interpolateVariable('abc', variable)).toEqual('abc');
       });
     });
 
@@ -722,8 +722,8 @@ describe('PostgreSQLDatasource', () => {
       it('should return a quoted value', () => {
         const { ds, variable } = setupTestContext({});
         variable.multi = true;
-        expect(ds.interpolateVariable("a'bc", variable)).toEqual("'a''bc'");
-        expect(ds.interpolateVariable("a'b'c", variable)).toEqual("'a''b''c'");
+        expect(ds.interpolateVariable("a'bc", variable)).toEqual("a''bc");
+        expect(ds.interpolateVariable("a'b'c", variable)).toEqual("a''b''c");
       });
     });
 
@@ -731,7 +731,7 @@ describe('PostgreSQLDatasource', () => {
       it('should return a quoted value', () => {
         const { ds, variable } = setupTestContext({});
         variable.includeAll = true;
-        expect(ds.interpolateVariable('abc', variable)).toEqual("'abc'");
+        expect(ds.interpolateVariable('abc', variable)).toEqual('abc');
       });
     });
   });

public/app/plugins/datasource/grafana-postgresql-datasource/datasource.ts

@@ -1,6 +1,6 @@
 import { v4 as uuidv4 } from 'uuid';
 
-import { DataSourceInstanceSettings, ScopedVars } from '@grafana/data';
+import { DataSourceInstanceSettings, ScopedVars, VariableWithMultiSupport } from '@grafana/data';
 import { LanguageDefinition } from '@grafana/plugin-ui';
 import { TemplateSrv } from '@grafana/runtime';
 import {
@@ -31,6 +31,28 @@ export class PostgresDatasource extends SqlDatasource {
     return new PostgresQueryModel(target, templateSrv, scopedVars);
   }
 
+  interpolateVariable = (value: string | string[] | number, variable: VariableWithMultiSupport) => {
+    if (typeof value === 'string') {
+      // For single string values, just escape quotes (don't add outer quotes)
+      // The quotes are provided by the query template: WHERE x = '$var'
+      // We only escape internal single quotes: O'Brien -> O''Brien
+      return String(value).replace(/'/g, "''");
+    }
+
+    if (typeof value === 'number') {
+      return value;
+    }
+
+    if (Array.isArray(value)) {
+      // For arrays, quote each value individually and join with comma
+      // Used in: WHERE x IN ($var) -> WHERE x IN ('val1','val2','val3')
+      const quotedValues = value.map((v) => this.getQueryModel().quoteLiteral(v));
+      return quotedValues.join(',');
+    }
+
+    return value;
+  };
+
   async getVersion(): Promise<string> {
     const value = await this.runSql<{ version: number }>(getVersion());
     const results = value.fields.version?.values;