Skip to content

Commit 835034c

Browse files
sunchanglongsunchanglong
authored
Merge pull request #35 from GreptimeTeam/feat/sqlrefine
feat: sql refine
2 parents 2c5face + 9264575 commit 835034c

File tree

7 files changed

+128
-176
lines changed

7 files changed

+128
-176
lines changed

‎src/components/configEditor/QuerySettingsConfig.test.tsx‎

Lines changed: 3 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -1,157 +1,16 @@
11
import React from 'react';
2-
import { render, fireEvent } from '@testing-library/react';
2+
import { render } from '@testing-library/react';
33
import { QuerySettingsConfig } from './QuerySettingsConfig';
4-
import allLabels from 'labels';
54

65
describe('QuerySettingsConfig', () => {
76
it('should render', () => {
87
const result = render(
98
<QuerySettingsConfig
10-
connMaxLifetime={'5'}
11-
dialTimeout={'5'}
12-
maxIdleConns={'5'}
13-
maxOpenConns={'5'}
14-
queryTimeout={'5'}
15-
validateSql={true}
16-
onConnMaxIdleConnsChange={() => {}}
17-
onConnMaxLifetimeChange={() => {}}
18-
onConnMaxOpenConnsChange={() => {}}
19-
onDialTimeoutChange={() => {}}
20-
onQueryTimeoutChange={() => {}}
21-
onValidateSqlChange={() => {}}
9+
filterValidationEnabled={true}
10+
onFilterValidationEnabledChange={() => {}}
2211
/>
2312
);
2413
expect(result.container.firstChild).not.toBeNull();
2514
});
2615

27-
it('should call onDialTimeout when changed', () => {
28-
const onDialTimeout = jest.fn();
29-
const result = render(
30-
<QuerySettingsConfig
31-
onConnMaxIdleConnsChange={() => {}}
32-
onConnMaxLifetimeChange={() => {}}
33-
onConnMaxOpenConnsChange={() => {}}
34-
onDialTimeoutChange={onDialTimeout}
35-
onQueryTimeoutChange={() => {}}
36-
onValidateSqlChange={() => {}}
37-
/>
38-
);
39-
expect(result.container.firstChild).not.toBeNull();
40-
41-
const input = result.getByPlaceholderText(allLabels.components.Config.QuerySettingsConfig.dialTimeout.placeholder);
42-
expect(input).toBeInTheDocument();
43-
fireEvent.change(input, { target: { value: '10' } });
44-
fireEvent.blur(input);
45-
expect(onDialTimeout).toHaveBeenCalledTimes(1);
46-
expect(onDialTimeout).toHaveBeenCalledWith(expect.any(Object));
47-
});
48-
49-
it('should call onQueryTimeout when changed', () => {
50-
const onQueryTimeout = jest.fn();
51-
const result = render(
52-
<QuerySettingsConfig
53-
onConnMaxIdleConnsChange={() => {}}
54-
onConnMaxLifetimeChange={() => {}}
55-
onConnMaxOpenConnsChange={() => {}}
56-
onDialTimeoutChange={() => {}}
57-
onQueryTimeoutChange={onQueryTimeout}
58-
onValidateSqlChange={() => {}}
59-
/>
60-
);
61-
expect(result.container.firstChild).not.toBeNull();
62-
63-
const input = result.getByPlaceholderText(allLabels.components.Config.QuerySettingsConfig.queryTimeout.placeholder);
64-
expect(input).toBeInTheDocument();
65-
fireEvent.change(input, { target: { value: '10' } });
66-
fireEvent.blur(input);
67-
expect(onQueryTimeout).toHaveBeenCalledTimes(1);
68-
expect(onQueryTimeout).toHaveBeenCalledWith(expect.any(Object));
69-
});
70-
71-
it('should call onValidateSqlChange when changed', () => {
72-
const onValidateSqlChange = jest.fn();
73-
const result = render(
74-
<QuerySettingsConfig
75-
onConnMaxIdleConnsChange={() => {}}
76-
onConnMaxLifetimeChange={() => {}}
77-
onConnMaxOpenConnsChange={() => {}}
78-
onDialTimeoutChange={() => {}}
79-
onQueryTimeoutChange={() => {}}
80-
onValidateSqlChange={onValidateSqlChange}
81-
/>
82-
);
83-
expect(result.container.firstChild).not.toBeNull();
84-
85-
const input = result.getByRole('checkbox');
86-
expect(input).toBeInTheDocument();
87-
fireEvent.click(input);
88-
expect(onValidateSqlChange).toHaveBeenCalledTimes(1);
89-
expect(onValidateSqlChange).toHaveBeenCalledWith(expect.any(Object));
90-
});
91-
92-
it('should call onConnMaxIdleConnsChange when changed', () => {
93-
const onConnMaxIdleConnsChange = jest.fn();
94-
const result = render(
95-
<QuerySettingsConfig
96-
onConnMaxIdleConnsChange={onConnMaxIdleConnsChange}
97-
onConnMaxLifetimeChange={() => {}}
98-
onConnMaxOpenConnsChange={() => {}}
99-
onDialTimeoutChange={() => {}}
100-
onQueryTimeoutChange={() => {}}
101-
onValidateSqlChange={() => {}}
102-
/>
103-
);
104-
expect(result.container.firstChild).not.toBeNull();
105-
106-
const input = result.getByPlaceholderText(allLabels.components.Config.QuerySettingsConfig.maxIdleConns.placeholder);
107-
expect(input).toBeInTheDocument();
108-
fireEvent.change(input, { target: { value: '10' } });
109-
fireEvent.blur(input);
110-
expect(onConnMaxIdleConnsChange).toHaveBeenCalledTimes(1);
111-
expect(onConnMaxIdleConnsChange).toHaveBeenCalledWith(expect.any(Object));
112-
});
113-
114-
it('should call onConnMaxLifetimeChange when changed', () => {
115-
const onConnMaxLifetimeChange = jest.fn();
116-
const result = render(
117-
<QuerySettingsConfig
118-
onConnMaxIdleConnsChange={() => {}}
119-
onConnMaxLifetimeChange={onConnMaxLifetimeChange}
120-
onConnMaxOpenConnsChange={() => {}}
121-
onDialTimeoutChange={() => {}}
122-
onQueryTimeoutChange={() => {}}
123-
onValidateSqlChange={() => {}}
124-
/>
125-
);
126-
expect(result.container.firstChild).not.toBeNull();
127-
128-
const input = result.getByPlaceholderText(allLabels.components.Config.QuerySettingsConfig.connMaxLifetime.placeholder);
129-
expect(input).toBeInTheDocument();
130-
fireEvent.change(input, { target: { value: '10' } });
131-
fireEvent.blur(input);
132-
expect(onConnMaxLifetimeChange).toHaveBeenCalledTimes(1);
133-
expect(onConnMaxLifetimeChange).toHaveBeenCalledWith(expect.any(Object));
134-
});
135-
136-
it('should call onConnMaxOpenConnsChange when changed', () => {
137-
const onConnMaxOpenConnsChange = jest.fn();
138-
const result = render(
139-
<QuerySettingsConfig
140-
onConnMaxIdleConnsChange={() => {}}
141-
onConnMaxLifetimeChange={() => {}}
142-
onConnMaxOpenConnsChange={onConnMaxOpenConnsChange}
143-
onDialTimeoutChange={() => {}}
144-
onQueryTimeoutChange={() => {}}
145-
onValidateSqlChange={() => {}}
146-
/>
147-
);
148-
expect(result.container.firstChild).not.toBeNull();
149-
150-
const input = result.getByPlaceholderText(allLabels.components.Config.QuerySettingsConfig.maxOpenConns.placeholder);
151-
expect(input).toBeInTheDocument();
152-
fireEvent.change(input, { target: { value: '10' } });
153-
fireEvent.blur(input);
154-
expect(onConnMaxOpenConnsChange).toHaveBeenCalledTimes(1);
155-
expect(onConnMaxOpenConnsChange).toHaveBeenCalledWith(expect.any(Object));
156-
});
15716
});

‎src/components/configEditor/QuerySettingsConfig.tsx‎

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,38 @@
11
import React, { FormEvent } from 'react';
2-
import { Switch, Input, Field } from '@grafana/ui';
2+
import { Switch, Field } from '@grafana/ui';
33
import { ConfigSection } from 'components/experimental/ConfigSection';
44
import allLabels from 'labels';
55

66
interface QuerySettingsConfigProps {
7-
connMaxLifetime?: string;
8-
dialTimeout?: string;
9-
maxIdleConns?: string;
10-
maxOpenConns?: string;
11-
queryTimeout?: string;
12-
validateSql?: boolean;
13-
onConnMaxIdleConnsChange: (e: FormEvent<HTMLInputElement>) => void;
14-
onConnMaxLifetimeChange: (e: FormEvent<HTMLInputElement>) => void;
15-
onConnMaxOpenConnsChange: (e: FormEvent<HTMLInputElement>) => void;
16-
onDialTimeoutChange: (e: FormEvent<HTMLInputElement>) => void;
17-
onQueryTimeoutChange: (e: FormEvent<HTMLInputElement>) => void;
18-
onValidateSqlChange: (e: FormEvent<HTMLInputElement>) => void;
7+
8+
filterValidationEnabled?: boolean;
9+
10+
onFilterValidationEnabledChange: (e: FormEvent<HTMLInputElement>) => void;
1911
}
2012

2113
export const QuerySettingsConfig = (props: QuerySettingsConfigProps) => {
2214
const {
23-
connMaxLifetime,
24-
dialTimeout,
25-
maxIdleConns,
26-
maxOpenConns,
27-
queryTimeout,
28-
validateSql,
29-
onConnMaxIdleConnsChange,
30-
onConnMaxLifetimeChange,
31-
onConnMaxOpenConnsChange,
32-
onDialTimeoutChange,
33-
onQueryTimeoutChange,
34-
onValidateSqlChange,
15+
// connMaxLifetime,
16+
// dialTimeout,
17+
// maxIdleConns,
18+
// maxOpenConns,
19+
// queryTimeout,
20+
// validateSql,
21+
filterValidationEnabled,
22+
// onConnMaxIdleConnsChange,
23+
// onConnMaxLifetimeChange,
24+
// onConnMaxOpenConnsChange,
25+
// onDialTimeoutChange,
26+
// onQueryTimeoutChange,
27+
// onValidateSqlChange,
28+
onFilterValidationEnabledChange,
3529
} = props;
3630

3731
const labels = allLabels.components.Config.QuerySettingsConfig;
3832

3933
return (
4034
<ConfigSection title={labels.title}>
41-
<Field label={labels.dialTimeout.label} description={labels.dialTimeout.tooltip}>
35+
{/* <Field label={labels.dialTimeout.label} description={labels.dialTimeout.tooltip}>
4236
<Input
4337
name={labels.dialTimeout.name}
4438
width={40}
@@ -101,6 +95,10 @@ export const QuerySettingsConfig = (props: QuerySettingsConfigProps) => {
10195
10296
<Field label={labels.validateSql.label} description={labels.validateSql.tooltip}>
10397
<Switch className="gf-form" value={validateSql || false} onChange={onValidateSqlChange} role="checkbox" />
98+
</Field> */}
99+
100+
<Field label="Query Builder Filter Validation" description="Enable validation to require at least one non-default time range condition">
101+
<Switch className="gf-form" value={filterValidationEnabled || false} onChange={onFilterValidationEnabledChange} role="checkbox" />
104102
</Field>
105103
</ConfigSection>
106104
);

‎src/components/queryBuilder/FilterEditor.tsx‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useState } from 'react';
1+
import React, { useEffect, useState } from 'react';
22
import { SelectableValue } from '@grafana/data';
33
import { Button, HorizontalGroup, InlineFormLabel, Input, MultiSelect, RadioButtonGroup, Select } from '@grafana/ui';
44
import { Filter, FilterOperator, TableColumn, NullFilter } from 'types/queryBuilder';
@@ -7,6 +7,7 @@ import labels from 'labels';
77
import { styles } from 'styles';
88
import { Datasource } from 'data/CHDatasource';
99
import useUniqueMapKeys from 'hooks/useUniqueMapKeys';
10+
import { validateUserFilters } from './filterValidator';
1011

1112
const boolValues: Array<SelectableValue<boolean>> = [
1213
{ value: true, label: 'True' },
@@ -418,20 +419,38 @@ export const FiltersEditor = (props: {
418419
}) => {
419420
const { filters = [], onFiltersChange, allColumns: fieldsList = [], datasource, database, table } = props;
420421
const { label, tooltip, addLabel } = labels.components.FilterEditor;
422+
423+
// Add validation state
424+
const [validationError, setValidationError] = useState<string | undefined>();
425+
const isFilterValidationEnabled = datasource.settings.jsonData.filterValidationEnabled || false;
421426
const addFilter = () => {
422427
onFiltersChange([...filters, { ...defaultNewFilter }]);
423428
};
429+
424430
const removeFilter = (index: number) => {
425431
const newFilters = [...filters];
426432
newFilters.splice(index, 1);
427433
onFiltersChange(newFilters);
428434
};
435+
429436
const onFilterChange = (index: number, filter: Filter) => {
430437
const newFilters = [...filters];
431438
newFilters[index] = filter;
432439
onFiltersChange(newFilters);
433440
};
434441

442+
useEffect(() => {
443+
if (!isFilterValidationEnabled) {
444+
return;
445+
}
446+
const validation = validateUserFilters(filters);
447+
if (validation.isValid) {
448+
setValidationError(undefined);
449+
} else {
450+
setValidationError(validation.error);
451+
}
452+
}, [filters, isFilterValidationEnabled]);
453+
435454
return (
436455
<>
437456
{filters.length === 0 && (
@@ -451,6 +470,9 @@ export const FiltersEditor = (props: {
451470
</Button>
452471
</div>
453472
)}
473+
474+
475+
454476
{filters.map((filter, index) => {
455477
return (
456478
<div className="gf-form" key={index}>
@@ -489,6 +511,13 @@ export const FiltersEditor = (props: {
489511
</Button>
490512
</div>
491513
)}
514+
{/* Display validation error */}
515+
{validationError && (
516+
<div className="gf-form" style={{ color: 'orange', fontSize: '12px' }}>
517+
<div className={`width-8 ${styles.Common.firstLabel}`}></div>
518+
<div>⚠️ {validationError}</div>
519+
</div>
520+
)}
492521
</>
493522
);
494523
};

‎src/components/queryBuilder/filterValidator.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { Filter, FilterOperator, ColumnHint } from 'types/queryBuilder';
2+
3+
export interface FilterValidationResult {
4+
isValid: boolean;
5+
error?: string;
6+
}
7+
8+
/**
9+
* Validates if there's at least one user-added filter (excluding default time range filters)
10+
*/
11+
export const validateUserFilters = (filters: Filter[]): FilterValidationResult => {
12+
// Count user-added filters (excluding default time range filters)
13+
let hasDefaultTimeRangeFilter = false;
14+
const userFilters = filters.filter(filter => {
15+
if (!filter.key && !filter.hint) {
16+
return false;
17+
}
18+
19+
// Exclude default time range filters that are automatically added
20+
const isDefaultTimeRange = filter.hint === ColumnHint.Time
21+
if (!hasDefaultTimeRangeFilter && isDefaultTimeRange) {
22+
return false
23+
}
24+
hasDefaultTimeRangeFilter = true;
25+
return true;
26+
});
27+
28+
if (userFilters.length === 0) {
29+
return {
30+
isValid: false,
31+
error: 'At least one non-default time range condition is required'
32+
};
33+
}
34+
35+
return { isValid: true };
36+
};
37+
38+
/**
39+
* Checks if a specific filter is a default time range filter
40+
*/
41+
export const isDefaultTimeRangeFilter = (filter: Filter): boolean => {
42+
return filter.hint === ColumnHint.Time &&
43+
filter.operator === FilterOperator.WithInGrafanaTimeRange &&
44+
filter.id === 'timeRange';
45+
};
46+
47+
/**
48+
* Gets only user-added filters (excluding default time range filters)
49+
*/
50+
export const getUserFilters = (filters: Filter[]): Filter[] => {
51+
return filters.filter(filter => !isDefaultTimeRangeFilter(filter));
52+
};

0 commit comments

Comments
 (0)