432 fix backward compatibility with queries containing regexp vars (#433)

arturminchukov · web-flow · commit 0a7ff359add9 · 2025-11-05T17:19:28.000+04:00
Related issue: #432 * change regexp var error to warning * fix backward compatibility with queries containing regexp vars * update doc: change all images with regexp vars queries to images with word filter
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## tip
 
+* BUGFIX: fix backward compatibility with queries containing regexp variables. Error in queries with regexp variables has been replaced with a warning with quick fix action. See [#432](https://github.com/VictoriaMetrics/victorialogs-datasource/pull/432).
+
 ## v0.21.3
 
 * BUGFIX: sanitize URLs to prevent potential security issues, properly close gzip reader to prevent resource leaks, fix text filter interpolation for `*` character. See pr [#429](https://github.com/VictoriaMetrics/victorialogs-datasource/pull/429).
diff --git a/src/LogsQL/regExpOperator.test.ts b/src/LogsQL/regExpOperator.test.ts
@@ -139,5 +139,21 @@ describe('regExpOperator', () => {
       const result = replaceRegExpOperatorToOperator('field.name.with.dots:~$variable');
       expect(result).toEqual('field.name.with.dots:$variable');
     });
+
+    it('should replace all regexp vars', () => {
+      const result = replaceRegExpOperatorToOperator('field.name.with.dots1:~$variable field.name.with.dots2:~$variable | field.name.with.dots3:~$variable field.name.with.dots4:~variable field.name.with.dots3:$variable');
+      expect(result).toEqual('field.name.with.dots1:$variable field.name.with.dots2:$variable | field.name.with.dots3:$variable field.name.with.dots4:~variable field.name.with.dots3:$variable');
+    });
+
+    it('should replace all regexp vars with new lines', () => {
+      const result = replaceRegExpOperatorToOperator(`kubernetes.pod_namespace:~"$namespace" kubernetes.pod_name:~"$pod" 
+                                                                          | stats by(kubernetes.pod_name) count() 
+                                                                          | count()`
+      );
+      expect(result).toEqual(`kubernetes.pod_namespace:"$namespace" kubernetes.pod_name:"$pod" 
+                                                                          | stats by(kubernetes.pod_name) count() 
+                                                                          | count()`
+      );
+    });
   });
 });
diff --git a/src/LogsQL/regExpOperator.ts b/src/LogsQL/regExpOperator.ts
@@ -1,12 +1,21 @@
 /**
  * Regular expression for matching variables in a query expression -> filterName:~"$VariableName"
  * */
-const variableRegExp = /(^|\||\s)([^\s:~]+)\s*:\s*~\s*(?:"(\$[A-Za-z0-9_.-]+)"|(\$[A-Za-z0-9_.-]+)|"([^"]+)"|([^\s|]+))(?=\s|\||$)/;
+const variableRegExpPattern = /(^|\||\s)([^\s:~]+)\s*:\s*~\s*(?:"(\$[A-Za-z0-9_.-]+)"|(\$[A-Za-z0-9_.-]+)|"([^"]+)"|([^\s|]+))(?=\s|\||$)/;
 
 export function getQueryExprVariableRegExp(queryExpr: string) {
-  return variableRegExp.exec(queryExpr);
+  return new RegExp(variableRegExpPattern).exec(queryExpr);
 }
 
 export function replaceRegExpOperatorToOperator(queryExpr: string, operator = ':') {
-  return queryExpr.replace(variableRegExp, `$1$2${operator}$4`);
+  return queryExpr.replace(new RegExp(variableRegExpPattern, 'g'), (match, p1, p2, p3, p4) => {
+    // p3 - "$variable"
+    // p4 - $variable
+    const variable = p3 || p4;
+    if (variable) {
+      const value = p3 ? `"${variable}"` : variable;
+      return `${p1}${p2}${operator}${value}`;
+    }
+    return match;
+  });
 }
diff --git a/src/components/QueryEditor/QueryEditor.tsx b/src/components/QueryEditor/QueryEditor.tsx
@@ -26,7 +26,7 @@ import { changeEditorMode, getQueryWithDefaults } from "./state";
 const QueryEditor = React.memo<VictoriaLogsQueryEditorProps>((props) => {
   const styles = useStyles2(getStyles);
 
-  const { onChange, onRunQuery: runQuery, data, app, queries, datasource, range: timeRange } = props;
+  const { onChange, onRunQuery, data, app, queries, datasource, range: timeRange } = props;
   const [dataIsStale, setDataIsStale] = useState(false);
   const [parseModalOpen, setParseModalOpen] = useState(false);
 
@@ -62,13 +62,6 @@ const QueryEditor = React.memo<VictoriaLogsQueryEditorProps>((props) => {
     onChange(query);
   };
 
-  const onRunQuery = useCallback(() => {
-    if(varRegExp) {
-      return;
-    }
-    runQuery();
-  }, [runQuery, varRegExp]);
-
   useEffect(() => {
     // grafana with a version below 12 doesn't support subscribe function on store
     if ('subscribe' in store) {
@@ -125,7 +118,7 @@ const QueryEditor = React.memo<VictoriaLogsQueryEditorProps>((props) => {
           ) : (
             <QueryCodeEditor {...props} query={query} onChange={onChangeInternal} showExplain={true}/>
           )}
-          {varRegExp && (<QueryEditorVariableRegexpError regExp={varRegExp} />)}
+          {varRegExp && (<QueryEditorVariableRegexpError regExp={varRegExp} query={query} onChange={onChange}/>)}
           <QueryEditorOptions
             query={query}
             onChange={onChange}
diff --git a/src/components/QueryEditor/QueryEditorVariableRegexpError.tsx b/src/components/QueryEditor/QueryEditorVariableRegexpError.tsx
@@ -1,24 +1,32 @@
 import { css } from "@emotion/css";
-import React from 'react';
+import React, { useCallback } from 'react';
 
 import { GrafanaTheme2 } from "@grafana/data";
-import { Badge, useTheme2 } from "@grafana/ui";
+import { Badge, TextLink, useTheme2 } from "@grafana/ui";
 
 import { replaceRegExpOperatorToOperator } from "../../LogsQL/regExpOperator";
-
+import { Query } from "../../types";
 
 interface Props {
   regExp: string;
+  query: Query;
+  onChange: (query: Query) => void;
 }
 
-const QueryEditorVariableRegexpError = ({ regExp }: Props) => {
+const QueryEditorVariableRegexpError = ({ regExp, query, onChange }: Props) => {
   const theme = useTheme2();
   const styles = getStyles(theme);
   const fixedFilter = replaceRegExpOperatorToOperator(regExp);
+  const onApply = useCallback((e: React.MouseEvent) => {
+    e.preventDefault();
+    const queryExpr = replaceRegExpOperatorToOperator(query.expr);
+    onChange({ ...query, expr: queryExpr });
+  }, [onChange, query])
 
   const text = (
     <div>
-      Regexp operator `~` cannot be used with variables in `{regExp}`. Use exact match operator `:` (e.g. `{fixedFilter}`) or use non variable in regexp instead.
+      Regexp operator `~` cannot be used with variables in `{regExp}`. Use word filter operator `:` (e.g. `{fixedFilter}`)
+      or use non variable in regexp instead. <TextLink onClick={onApply} href={""}>Apply fix</TextLink>
     </div>
   )
 
diff --git a/src/datasource.ts b/src/datasource.ts
@@ -40,6 +40,7 @@ import { escapeLabelValueInSelector } from "./languageUtils";
 import LogsQlLanguageProvider from "./language_provider";
 import { LOGS_VOLUME_BARS, queryLogsVolume } from "./logsVolumeLegacy";
 import { addLabelToQuery, addSortPipeToQuery, queryHasFilter, removeLabelFromQuery } from "./modifyQuery";
+import { removeDoubleQuotesAroundVar } from "./parsing";
 import { replaceOperatorWithIn, returnVariables } from "./parsingUtils";
 import { storeKeys } from "./store/constants";
 import store from "./store/store";
@@ -286,14 +287,21 @@ export class VictoriaLogsDatasource
     return Array.isArray(value) ? value.includes(VARIABLE_ALL_VALUE) : false;
   }
 
-  replaceOperatorsToInForMultiQueryVariables(expr: string,) {
+  replaceAllOption(queryExpr: string, variableName: string, regExpAllValue: string): string {
+    queryExpr = queryExpr.replace(`~"$${variableName}"`, `~"${regExpAllValue}"` || '~".*"');
+    queryExpr = queryExpr.replace(`$${variableName}`, '*');
+    return queryExpr;
+  }
+
+  replaceOperatorsToInForMultiQueryVariables(expr: string) {
     const variables = this.templateSrv.getVariables();
     const fieldValuesVariables = variables.filter(v => v.type === 'query' && v.query.type === 'fieldValue' && v.multi || this.isAllOption(v)) as QueryVariableModel[];
     let result = expr;
     for (let variable of fieldValuesVariables) {
+      result = removeDoubleQuotesAroundVar(result, variable.name);
       result = replaceOperatorWithIn(result, variable.name);
       if (this.isAllOption(variable)) {
-        result = result.replace(`$${variable.name}`, '*');
+        result = this.replaceAllOption(result, variable.name, variable.allValue || '.*');
       }
     }
     return result;
diff --git a/src/img/panel_logs.png b/src/img/panel_logs.png
diff --git a/src/img/panel_stat.png b/src/img/panel_stat.png
diff --git a/src/img/panel_table.png b/src/img/panel_table.png
diff --git a/src/img/panel_time_series.png b/src/img/panel_time_series.png
diff --git a/src/parsing.test.ts b/src/parsing.test.ts
@@ -0,0 +1,51 @@
+import { removeDoubleQuotesAroundVar } from './parsing';
+
+describe('removeDoubleQuotesAroundVar', () => {
+  it('should remove double quotes around the variable name', () => {
+    const query = `"text" and "$var"`;
+    const result = removeDoubleQuotesAroundVar(query, 'var');
+    expect(result).toBe('"text" and $var');
+  });
+
+  it('should not remove double quotes if the variable name does not match', () => {
+    const query = `"text" and "$otherVar"`;
+    const result = removeDoubleQuotesAroundVar(query, 'var');
+    expect(result).toBe(`"text" and "$otherVar"`);
+  });
+
+  it('should not modify variables that already have no quotes', () => {
+    const query = `"text" and $var`;
+    const result = removeDoubleQuotesAroundVar(query, 'var');
+    expect(result).toBe(`"text" and $var`);
+  });
+
+  it('should not remove double quotes if preceded by a tilde (~)', () => {
+    const query = `"text" and ~"$var"`;
+    const result = removeDoubleQuotesAroundVar(query, 'var');
+    expect(result).toBe(`"text" and ~"$var"`);
+  });
+
+  it('should handle multiple occurrences of the same variable', () => {
+    const query = `"$var" and "$var"`;
+    const result = removeDoubleQuotesAroundVar(query, 'var');
+    expect(result).toBe(`$var and $var`);
+  });
+
+  it('should leave the string unmodified if no matching variable is found', () => {
+    const query = `"text" and "$differentVar"`;
+    const result = removeDoubleQuotesAroundVar(query, 'var');
+    expect(result).toBe(`"text" and "$differentVar"`);
+  });
+
+  it('should handle an empty query string gracefully', () => {
+    const query = ``;
+    const result = removeDoubleQuotesAroundVar(query, 'var');
+    expect(result).toBe(``);
+  });
+
+  it('should handle an empty variable name gracefully', () => {
+    const query = `"$var" and "$otherVar"`;
+    const result = removeDoubleQuotesAroundVar(query, '');
+    expect(result).toBe(`"$var" and "$otherVar"`);
+  });
+});
diff --git a/src/parsing.ts b/src/parsing.ts
@@ -7,3 +7,10 @@ export function handleQuotes(string: string) {
   }
   return string.replace(/`/g, '');
 }
+
+// remove double quotes around the variable name if it's not a regex selector `~`
+export function removeDoubleQuotesAroundVar(queryExpr: string, variableName: string): string {
+  const regex = new RegExp(`(?<!~)"\\$${variableName}"`, 'g');
+  queryExpr = queryExpr.replace(regex, `$${variableName}`);
+  return queryExpr;
+}
