Skip to content

Commit f9a337a

Browse files
authored
Merge pull request #817 from Altinity/issue-815
add variables replacement before macros replacement
2 parents 87a2a3c + 820dd5f commit f9a337a

File tree

4 files changed

+77
-1
lines changed

4 files changed

+77
-1
lines changed

pkg/eval/eval_query.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ func (q *EvalQuery) escapeIdentifier(identifier string) string {
291291
}
292292

293293
func (q *EvalQuery) escapeTableIdentifier(identifier string) string {
294-
if regexp.MustCompile(`^[a-zA-Z][0-9a-zA-Z_]+$`).MatchString(identifier) {
294+
if regexp.MustCompile(`^[$a-zA-Z][0-9a-zA-Z_]+$`).MatchString(identifier) {
295295
return identifier
296296
} else {
297297
return "`" + strings.Replace(identifier, "`", "\\`", -1) + "`"

src/datasource/datasource.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,13 @@ export class CHDataSource
803803
},
804804
};
805805

806+
// Apply template variable replacements (these don't require backend processing)
807+
queryData.query = this.templateSrv.replace(
808+
conditionalTest(queryData.query, this.templateSrv),
809+
options.scopedVars,
810+
createContextAwareInterpolation(queryData.query, this.templateSrv.getVariables())
811+
);
812+
806813
// SAFE OPTIMIZATION: Batch createQuery + applyAdhocFilters (reduces 3->2 calls)
807814
const queryResult = await this.resourceClient.createQueryWithAdhoc(queryData, adhocFilters);
808815

src/views/QueryEditor/QueryEditor.tsx

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {useAutocompleteData} from './hooks/useAutocompletionData';
1212
import {initializeQueryDefaults, initializeQueryDefaultsForVariables} from './helpers/initializeQueryDefaults';
1313
import './QueryEditor.css';
1414
import {getAdhocFilters} from './helpers/getAdHocFilters';
15+
import {detectVariableMacroIntersections, createVariableMacroConflictWarning} from './helpers/detectVariableMacroIntersections';
1516

1617
export function QueryEditor(props: QueryEditorProps<CHDataSource, CHQuery, CHDataSourceOptions>): any {
1718
const { datasource, query, onChange, onRunQuery, data } = props;
@@ -41,6 +42,10 @@ export function QueryEditor(props: QueryEditorProps<CHDataSource, CHQuery, CHDat
4142
const adHocFiltersKey = adHocFilters.map(({ key, operator, value }) => `${key}${operator}${value}`).join(',');
4243
const areAdHocFiltersAvailable = !!adHocFilters.length;
4344

45+
// Detect variable/macro name conflicts
46+
const variableMacroConflicts = detectVariableMacroIntersections();
47+
const conflictWarning = createVariableMacroConflictWarning(variableMacroConflicts);
48+
4449
useEffect(() => {
4550
if (props.app !== CoreApp.Explore) {
4651
onChange({ ...initializedQuery, adHocFilters: adHocFilters });
@@ -62,6 +67,16 @@ export function QueryEditor(props: QueryEditorProps<CHDataSource, CHQuery, CHDat
6267
hasAutocompleteError={hasPermissionError}
6368
/>
6469
{error ? <Alert title={error} elevated style={{ marginTop: '5px', marginBottom: '5px' }} /> : null}
70+
{conflictWarning ? (
71+
<Alert
72+
title="Variable/Macro Name Conflict Warning"
73+
severity="warning"
74+
elevated
75+
style={{ marginTop: '5px', marginBottom: '5px' }}
76+
>
77+
{conflictWarning}
78+
</Alert>
79+
) : null}
6580
{editorMode === EditorMode.Builder && (
6681
<QueryBuilder
6782
query={initializedQuery}
@@ -122,6 +137,10 @@ export function QueryEditorVariable(props: QueryEditorProps<CHDataSource, CHQuer
122137
const adHocFiltersKey = adHocFilters.map(({ key, operator, value }) => `${key}${operator}${value}`).join(',');
123138
const areAdHocFiltersAvailable = !!adHocFilters.length;
124139

140+
// Detect variable/macro name conflicts
141+
const variableMacroConflicts = detectVariableMacroIntersections();
142+
const conflictWarning = createVariableMacroConflictWarning(variableMacroConflicts);
143+
125144
useEffect(() => {
126145
if (props.app !== CoreApp.Explore) {
127146
onChange({ ...initializedQuery, adHocFilters: adHocFilters });
@@ -142,6 +161,16 @@ export function QueryEditorVariable(props: QueryEditorProps<CHDataSource, CHQuer
142161
hasAutocompleteError={false}
143162
/>
144163
{error ? <Alert title={error} elevated style={{ marginTop: '5px', marginBottom: '5px' }} /> : null}
164+
{conflictWarning ? (
165+
<Alert
166+
title="Variable/Macro Name Conflict Warning"
167+
severity="warning"
168+
elevated
169+
style={{ marginTop: '5px', marginBottom: '5px' }}
170+
>
171+
{conflictWarning}
172+
</Alert>
173+
) : null}
145174
{editorMode === EditorMode.Builder && (
146175
<QueryBuilder
147176
query={initializedQuery}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { getTemplateSrv } from '@grafana/runtime';
2+
import macros from '../components/QueryTextEditor/editor/constants/macros';
3+
4+
/**
5+
* Detects potential name intersections between Grafana template variables and ClickHouse macros
6+
* Returns an array of conflicting names
7+
*/
8+
export function detectVariableMacroIntersections(): string[] {
9+
const templateSrv = getTemplateSrv();
10+
const variables = templateSrv.getVariables();
11+
const conflicts: string[] = [];
12+
13+
const variableNames = variables.map(variable => `$${variable.name}`);
14+
15+
// Check for intersections with macros
16+
for (const variableName of variableNames) {
17+
if (macros.includes(variableName)) {
18+
conflicts.push(variableName);
19+
}
20+
}
21+
22+
return conflicts;
23+
}
24+
25+
/**
26+
* Creates a warning message for variable/macro name conflicts
27+
*/
28+
export function createVariableMacroConflictWarning(conflicts: string[]): string {
29+
if (conflicts.length === 0) {
30+
return '';
31+
}
32+
33+
const conflictList = conflicts.join(', ');
34+
35+
if (conflicts.length === 1) {
36+
return `Template variable "${conflictList}" has the same name as a ClickHouse macro. This may cause unexpected behavior during query processing.`;
37+
} else {
38+
return `Template variables "${conflictList}" have the same names as ClickHouse macros. This may cause unexpected behavior during query processing.`;
39+
}
40+
}

0 commit comments

Comments
 (0)