Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add rows limit to query settings #1291

Merged
merged 21 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
60b550e
feat: add rows limit to query settings
astandrik Sep 13, 2024
5469b74
fix: review fixes
astandrik Sep 16, 2024
a970b28
Merge branch 'main' into astandrik.add-rows-limit-to-query-settings-1253
astandrik Sep 16, 2024
df16b2d
fix: use zod instead of yup
astandrik Sep 16, 2024
e55c633
fix: review fix
astandrik Sep 16, 2024
b87e848
fix: review fixes
astandrik Sep 16, 2024
f572b1a
fix: revert changes
astandrik Sep 17, 2024
adf896f
Merge branch 'main' into astandrik.add-rows-limit-to-query-settings-1253
astandrik Sep 17, 2024
410a10f
Merge branch 'main' into astandrik.add-rows-limit-to-query-settings-1253
astandrik Sep 17, 2024
56f9e3f
fix: review fixes
astandrik Sep 18, 2024
df1c0bb
fix: long running query test
astandrik Sep 18, 2024
07abc0f
Merge branch 'main' into astandrik.add-rows-limit-to-query-settings-1253
astandrik Sep 18, 2024
cdadfbf
Merge branch 'main' into astandrik.add-rows-limit-to-query-settings-1253
astandrik Sep 18, 2024
c3fe9af
Merge branch 'main' into astandrik.add-rows-limit-to-query-settings-1253
astandrik Sep 18, 2024
dcb0f95
fix: review fixes
astandrik Sep 19, 2024
67b9d0d
Merge branch 'main' into astandrik.add-rows-limit-to-query-settings-1253
astandrik Sep 19, 2024
2c30b28
fix: nanofix
astandrik Sep 19, 2024
470d285
Merge branch 'main' into astandrik.add-rows-limit-to-query-settings-1253
astandrik Sep 19, 2024
f64c011
fix: timeout and limitRows
astandrik Sep 20, 2024
514265d
Merge branch 'main' into astandrik.add-rows-limit-to-query-settings-1253
astandrik Sep 23, 2024
e0d310d
Merge branch 'main' into astandrik.add-rows-limit-to-query-settings-1253
astandrik Sep 23, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"@gravity-ui/table": "^0.5.0",
"@gravity-ui/uikit": "^6.20.1",
"@gravity-ui/websql-autocomplete": "^9.1.0",
"@hookform/resolvers": "^3.9.0",
"@reduxjs/toolkit": "^2.2.3",
"@tanstack/react-table": "^8.19.3",
"axios": "^1.7.3",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
import type {QuerySettings} from '../../../../../types/store/query';
import {
DEFAULT_QUERY_SETTINGS,
QUERY_MODES,
STATISTICS_MODES,
TRACING_LEVELS,
TRANSACTION_MODES,
} from '../../../../../utils/query';

const DEFAULT_QUERY_SETTINGS: QuerySettings = {
queryMode: QUERY_MODES.query,
transactionMode: TRANSACTION_MODES.implicit,
timeout: '60',
statisticsMode: STATISTICS_MODES.none,
tracingLevel: TRACING_LEVELS.detailed,
};

import getChangedQueryExecutionSettings from './getChangedQueryExecutionSettings';

describe('getChangedQueryExecutionSettings', () => {
Expand All @@ -27,17 +20,19 @@ describe('getChangedQueryExecutionSettings', () => {
const currentSettings: QuerySettings = {
...DEFAULT_QUERY_SETTINGS,
queryMode: QUERY_MODES.data,
timeout: '30',
timeout: 30,
limitRows: 100,
};
const result = getChangedQueryExecutionSettings(currentSettings, DEFAULT_QUERY_SETTINGS);
expect(result).toEqual(['queryMode', 'timeout']);
expect(result).toEqual(['queryMode', 'timeout', 'limitRows']);
});

it('should return all keys if all settings have changed', () => {
const currentSettings: QuerySettings = {
queryMode: QUERY_MODES.data,
transactionMode: TRANSACTION_MODES.onlinero,
timeout: '90',
timeout: 90,
limitRows: DEFAULT_QUERY_SETTINGS.limitRows,
statisticsMode: STATISTICS_MODES.basic,
tracingLevel: TRACING_LEVELS.basic,
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {QuerySettings} from '../../../../../types/store/query';
import {
DEFAULT_QUERY_SETTINGS,
QUERY_MODES,
QUERY_MODES_TITLES,
STATISTICS_MODES,
Expand All @@ -13,14 +14,6 @@ import {QUERY_SETTINGS_FIELD_SETTINGS} from '../../QuerySettingsDialog/constants

import getChangedQueryExecutionSettingsDescription from './getChangedQueryExecutionSettingsDescription';

const DEFAULT_QUERY_SETTINGS: QuerySettings = {
queryMode: QUERY_MODES.query,
transactionMode: TRANSACTION_MODES.implicit,
timeout: '60',
statisticsMode: STATISTICS_MODES.none,
tracingLevel: TRACING_LEVELS.detailed,
};

describe('getChangedQueryExecutionSettingsDescription', () => {
it('should return an empty object if no settings changed', () => {
const currentSettings: QuerySettings = {...DEFAULT_QUERY_SETTINGS};
Expand All @@ -37,7 +30,8 @@ describe('getChangedQueryExecutionSettingsDescription', () => {
const currentSettings: QuerySettings = {
...DEFAULT_QUERY_SETTINGS,
queryMode: QUERY_MODES.pg,
timeout: '63',
timeout: 63,
limitRows: 100,
};

const result = getChangedQueryExecutionSettingsDescription({
Expand All @@ -51,14 +45,16 @@ describe('getChangedQueryExecutionSettingsDescription', () => {
(option) => option.value === QUERY_MODES.pg,
)?.content,
[QUERY_SETTINGS_FIELD_SETTINGS.timeout.title]: '63',
[QUERY_SETTINGS_FIELD_SETTINGS.limitRows.title]: '100',
});
});

it('should return the correct description for all changed settings', () => {
const currentSettings: QuerySettings = {
queryMode: QUERY_MODES.data,
transactionMode: TRANSACTION_MODES.snapshot,
timeout: '120',
timeout: 120,
limitRows: DEFAULT_QUERY_SETTINGS.limitRows,
statisticsMode: STATISTICS_MODES.profile,
tracingLevel: TRACING_LEVELS.diagnostic,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,18 @@ export default function getChangedQueryExecutionSettingsDescription({

keys.forEach((key) => {
const settings = QUERY_SETTINGS_FIELD_SETTINGS[key];
const currentValue = currentSettings[key] as string;
const currentValue = currentSettings[key];

if ('options' in settings) {
const content = settings.options.find((option) => option.value === currentValue)
?.content as string;
const content = settings.options.find(
(option) => option.value === currentValue,
)?.content;

result[settings.title] = content;
} else {
result[settings.title] = currentValue;
if (content) {
result[settings.title] = content;
}
} else if (currentValue) {
result[settings.title] = String(currentValue);
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
flex: 6;
}

&__limit-rows,
&__timeout {
width: 33.3%;
margin-right: var(--g-spacing-2);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';

import {Dialog, Link as ExternalLink, Flex, TextInput} from '@gravity-ui/uikit';
import {zodResolver} from '@hookform/resolvers/zod';
import {Controller, useForm} from 'react-hook-form';

import {useTracingLevelOptionAvailable} from '../../../../store/reducers/capabilities/hooks';
Expand All @@ -15,6 +16,7 @@ import {
useTypedDispatch,
useTypedSelector,
} from '../../../../utils/hooks';
import {querySettingsValidationSchema} from '../../../../utils/query';

import {QuerySettingsSelect} from './QuerySettingsSelect';
import {QUERY_SETTINGS_FIELD_SETTINGS} from './constants';
Expand Down Expand Up @@ -66,8 +68,13 @@ interface QuerySettingsFormProps {
}

function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsFormProps) {
const {control, handleSubmit} = useForm<QuerySettings>({
const {
control,
handleSubmit,
formState: {errors},
} = useForm<QuerySettings>({
defaultValues: initialValues,
resolver: zodResolver(querySettingsValidationSchema),
});

const enableTracingLevel = useTracingLevelOptionAvailable();
Expand All @@ -85,6 +92,7 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm
control={control}
render={({field}) => (
<QuerySettingsSelect
id="queryMode"
setting={field.value}
onUpdateSetting={field.onChange}
settingOptions={QUERY_SETTINGS_FIELD_SETTINGS.queryMode.options}
Expand All @@ -104,10 +112,15 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm
render={({field}) => (
<React.Fragment>
<TextInput
id="timeout"
type="number"
{...field}
value={field.value?.toString()}
className={b('timeout')}
placeholder="60"
validationState={errors.timeout ? 'invalid' : undefined}
errorMessage={errors.timeout?.message}
errorPlacement="inside"
/>
<span className={b('timeout-suffix')}>
{i18n('form.timeout.seconds')}
Expand All @@ -128,6 +141,7 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm
control={control}
render={({field}) => (
<QuerySettingsSelect
id="tracingLevel"
setting={field.value}
onUpdateSetting={field.onChange}
settingOptions={
Expand All @@ -149,6 +163,7 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm
control={control}
render={({field}) => (
<QuerySettingsSelect
id="transactionMode"
setting={field.value}
onUpdateSetting={field.onChange}
settingOptions={
Expand All @@ -169,6 +184,7 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm
control={control}
render={({field}) => (
<QuerySettingsSelect
id="statisticsMode"
setting={field.value}
onUpdateSetting={field.onChange}
settingOptions={
Expand All @@ -179,6 +195,30 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm
/>
</div>
</Flex>
<Flex direction="row" alignItems="flex-start" className={b('dialog-row')}>
<label htmlFor="limitRows" className={b('field-title')}>
{QUERY_SETTINGS_FIELD_SETTINGS.limitRows.title}
</label>
<div className={b('control-wrapper')}>
<Controller
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add validation in Controller?

name="limitRows"
control={control}
render={({field}) => (
<TextInput
id="limitRows"
type="number"
{...field}
value={field.value?.toString()}
className={b('limit-rows')}
placeholder="10000"
validationState={errors.limitRows ? 'invalid' : undefined}
errorMessage={errors.limitRows?.message}
errorPlacement="inside"
/>
)}
/>
</div>
</Flex>
</Dialog.Body>
<Dialog.Footer
textButtonApply={i18n('button-done')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type SelectType = QueryMode | TransactionMode | StatisticsMode | TracingLevel;
type QuerySettingSelectOption<T> = SelectOption<T> & {isDefault?: boolean};

interface QuerySettingsSelectProps<T extends SelectType> {
id?: string;
setting: T;
settingOptions: QuerySettingSelectOption<T>[];
onUpdateSetting: (mode: T) => void;
Expand All @@ -32,6 +33,7 @@ export function QuerySettingsSelect<T extends SelectType>(props: QuerySettingsSe
return (
<div className={b('selector')}>
<Select<T>
id={props.id}
options={props.settingOptions}
value={[props.setting]}
onUpdate={(value) => {
Expand Down
3 changes: 3 additions & 0 deletions src/containers/Tenant/Query/QuerySettingsDialog/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,7 @@ export const QUERY_SETTINGS_FIELD_SETTINGS = {
timeout: {
title: formI18n('form.timeout'),
},
limitRows: {
title: formI18n('form.limit-rows'),
},
} as const;
3 changes: 3 additions & 0 deletions src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
"form.transaction-mode": "Transaction mode",
"form.statistics-mode": "Statistics collection mode",
"form.tracing-level": "Tracing level",
"form.limit-rows": "Limit rows",
"button-done": "Save",
"button-cancel": "Cancel",
"form.timeout.seconds": "sec",
"form.validation.timeout": "Must be positive",
"form.validation.limitRows": "Must be between 1 and 100000",
"description.default": " (default)",
"docs": "Documentation"
}
3 changes: 3 additions & 0 deletions src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
"form.transaction-mode": "Уровень изоляции",
"form.statistics-mode": "Режим сбора статистики",
"form.tracing-level": "Tracing level",
"form.limit-rows": "Лимит строк",
"button-done": "Готово",
"button-cancel": "Отменить",
"form.timeout.seconds": "сек",
"form.validation.timeout": "Таймаут должен быть положительным",
"form.validation.limitRows": "Лимит строк должен быть между 1 и 100000",
"description.default": " (default)",
"docs": "Документация"
}
1 change: 1 addition & 0 deletions src/services/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,7 @@ export class YdbEmbeddedAPI extends AxiosWrapper {
transaction_mode?: TransactionMode;
timeout?: Timeout;
query_id?: string;
limit_rows?: number;
},
{concurrentId, signal, withRetries}: AxiosOptions = {},
) {
Expand Down
3 changes: 1 addition & 2 deletions src/services/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
AUTOCOMPLETE_ON_ENTER,
AUTO_REFRESH_INTERVAL,
BINARY_DATA_IN_PLAIN_TEXT_DISPLAY,
DEFAULT_QUERY_SETTINGS,
ENABLE_AUTOCOMPLETE,
INVERTED_DISKS_KEY,
IS_HOTKEYS_HELP_HIDDEN_KEY,
Expand All @@ -23,7 +22,7 @@ import {
USE_CLUSTER_BALANCER_AS_BACKEND_KEY,
USE_PAGINATED_TABLES_KEY,
} from '../utils/constants';
import {QUERY_ACTIONS} from '../utils/query';
import {DEFAULT_QUERY_SETTINGS, QUERY_ACTIONS} from '../utils/query';
import {parseJson} from '../utils/utils';

export type SettingsObject = Record<string, unknown>;
Expand Down
3 changes: 3 additions & 0 deletions src/store/reducers/executeQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,9 @@ export const executeQueryApi = api.injectEndpoints({
querySettings.tracingLevel && enableTracingLevel
? TracingLevelNumber[querySettings.tracingLevel]
: undefined,
limit_rows: isNumeric(querySettings.limitRows)
? Number(querySettings.limitRows)
: undefined,
transaction_mode:
querySettings.transactionMode === 'implicit'
? undefined
Expand Down
11 changes: 4 additions & 7 deletions src/types/store/query.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import type {z} from 'zod';

import type {
QUERY_ACTIONS,
QUERY_MODES,
QUERY_SYNTAX,
STATISTICS_MODES,
TRACING_LEVELS,
TRANSACTION_MODES,
querySettingsValidationSchema,
} from '../../utils/query';
import type {IResponseError, NetworkError} from '../api/error';
import type {
Expand Down Expand Up @@ -37,13 +40,7 @@ export interface QueryRequestParams {
query: string;
}

export interface QuerySettings {
queryMode: QueryMode;
transactionMode: TransactionMode;
timeout?: string;
statisticsMode?: StatisticsMode;
tracingLevel?: TracingLevel;
}
export type QuerySettings = z.infer<typeof querySettingsValidationSchema>;

export type QueryErrorResponse = IResponseError<QueryErrorResponseData>;
export type QueryError = NetworkError | QueryErrorResponse;
Expand Down
Loading
Loading