Fix Query Builder FilterEditor on Alert Rules page

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Fixed `IN` operator escaping the entire string (specifically with `Nullable(String)`), also added `FixedString(N)` (#830)
+- Fixed query builder filter editor on alert rules page (#828)
+
 ## 4.0.7
 
 - Upgrade dependencies

cspell.config.json

@@ -10,6 +10,8 @@
     "mage_output_file.go"
   ],
   "words": [
+    "lowcardinality",
+    "fixedstring",
     "singlequote",
     "concats",
     "Milli",
@@ -77,4 +79,4 @@
     "goproxy",
     "Nonproxy"
   ]
-}
+}

src/components/queryBuilder/FilterEditor.test.tsx

@@ -192,6 +192,28 @@ describe('FilterEditor', () => {
       // type key into the mapKey input
       await userEvent.type(result!.getAllByRole('combobox')[1], 'http.status_code');
       await userEvent.keyboard('{Enter}');
+
+      result.rerender(
+        <FilterEditor
+          allColumns={[{ name: 'SpanAttributes', type: 'Map(String, String)', picklistValues: [] }]}
+          filter={{
+            key: 'SpanAttributes',
+            type: 'Map(String, String)',
+            mapKey: 'http.status_code',
+            value: '',
+            operator: FilterOperator.Equals,
+            condition: 'AND',
+            filterType: 'custom',
+          }}
+          index={0}
+          onFilterChange={onFilterChange}
+          removeFilter={() => {}}
+          datasource={mockDatasource}
+          database=''
+          table=''
+        />
+      );
+
       // type value into the input
       await userEvent.type(result!.getByTestId('query-builder-filters-single-string-value-input'), '200');
       result!.getByTestId('query-builder-filters-single-string-value-input').blur();

src/components/queryBuilder/FilterEditor.tsx

@@ -328,12 +328,12 @@ export const FilterEditor = (props: {
     onFilterChange(index, newFilter);
   };
   const onFilterMapKeyChange = (mapKey: string) => {
-    const newFilter: Filter = filter;
+    const newFilter: Filter = { ...filter };
     newFilter.mapKey = mapKey;
     onFilterChange(index, newFilter);
   };
   const onFilterOperatorChange = (operator: FilterOperator) => {
-    let newFilter: Filter = filter;
+    const newFilter: Filter = { ...filter };
     newFilter.operator = operator;
     if (utils.isMultiFilter(newFilter)) {
       if (!Array.isArray(newFilter.value)) {
@@ -343,7 +343,7 @@ export const FilterEditor = (props: {
     onFilterChange(index, newFilter);
   };
   const onFilterConditionChange = (condition: 'AND' | 'OR') => {
-    let newFilter: Filter = filter;
+    const newFilter: Filter = { ...filter };
     newFilter.condition = condition;
     onFilterChange(index, newFilter);
   };

src/data/sqlGenerator.test.ts

@@ -530,6 +530,21 @@ describe('getLimit', () => {
   });
 });
 
+describe('is*Type', () => {
+  it.each<{ input: string, expected: boolean }>([
+    { input: 'String', expected: true },
+    { input: 'Nullable(String)', expected: true },
+    { input: 'LowCardinality(Nullable(String))', expected: true },
+    { input: 'FixedString(1)', expected: true },
+    { input: 'LowCardinality(Nullable(FixedString(1)))', expected: true },
+    { input: 'LowCardinality(FixedString(1))', expected: true },
+    { input: 'Nullable(FixedString(1))', expected: true },
+    { input: 'Array(String)', expected: false },
+  ])('$input isStringType $expected', (c) => {
+    expect(_testExports.isStringType(c.input)).toEqual(c.expected);
+  });
+});
+
 describe('getFilters', () => {
   it('returns empty filter array', () => {
     const options = {} as QueryBuilderOptions;
@@ -538,6 +553,24 @@ describe('getFilters', () => {
     expect(sql).toEqual(expectedSql);
   });
 
+  it('returns correct IN clause for escaped and unescaped values', () => {
+    const options = {
+      filters: [
+        {
+          condition: 'AND',
+          filterType: 'custom',
+          key: 'col',
+          operator: FilterOperator.In,
+          type: 'string',
+          value: '1, (2), 3, some string, \'another string\', someFunction(123), "column reference"'.split(',')
+        }
+      ]
+    } as QueryBuilderOptions;
+    const sql = _testExports.getFilters(options);
+    const expectedSql = `( col IN ('1', (2), '3', 'some string', 'another string', someFunction(123), "column reference") )`;
+    expect(sql).toEqual(expectedSql);
+  });
+
   it('returns complex filter array', () => {
     const options = {
       columns: [{ name: 'hinted', hint: ColumnHint.Time }],

src/data/sqlGenerator.ts

@@ -742,12 +742,24 @@ const getFilters = (options: QueryBuilderOptions): string => {
   return concatQueryParts(builtFilters);
 };
 
+const stripTypeModifiers = (type: string): string => {
+    return type.toLowerCase().
+      replace(/\(/g, '').
+      replace(/\)/g, '').
+      replace(/nullable/g, '').
+      replace(/lowcardinality/g, '');
+
+}
 const isBooleanType = (type: string): boolean => (type?.toLowerCase().startsWith('boolean'));
 const numberTypes = ['int', 'float', 'decimal'];
 const isNumberType = (type: string): boolean => numberTypes.some(t => type?.toLowerCase().includes(t));
 const isDateType = (type: string): boolean => type?.toLowerCase().startsWith('date') || type?.toLowerCase().startsWith('nullable(date');
 // const isDateTimeType = (type: string): boolean => type?.toLowerCase().startsWith('datetime') || type?.toLowerCase().startsWith('nullable(datetime');
-const isStringType = (type: string): boolean => type.toLowerCase() === 'string' && !(isBooleanType(type) || isNumberType(type) || isDateType(type));
+const isStringType = (type: string): boolean => {
+  type = stripTypeModifiers(type.toLowerCase());
+  return (type === 'string' || type.startsWith('fixedstring'))
+  && !(isBooleanType(type) || isNumberType(type) || isDateType(type));
+}
 const isNullFilter = (operator: FilterOperator): boolean => operator === FilterOperator.IsNull || operator === FilterOperator.IsNotNull;
 const isBooleanFilter = (type: string): boolean => isBooleanType(type);
 const isNumberFilter = (type: string): boolean => isNumberType(type);
@@ -778,4 +790,5 @@ export const _testExports = {
   getOrderBy,
   getLimit,
   getFilters,
+  isStringType,
 };