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 6 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
Expand Up @@ -10,6 +10,7 @@ const DEFAULT_QUERY_SETTINGS: QuerySettings = {
queryMode: QUERY_MODES.query,
transactionMode: TRANSACTION_MODES.implicit,
timeout: '60',
limitRows: '10000',
statisticsMode: STATISTICS_MODES.none,
tracingLevel: TRACING_LEVELS.detailed,
};
Expand All @@ -28,9 +29,10 @@ describe('getChangedQueryExecutionSettings', () => {
...DEFAULT_QUERY_SETTINGS,
queryMode: QUERY_MODES.data,
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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const DEFAULT_QUERY_SETTINGS: QuerySettings = {
queryMode: QUERY_MODES.query,
transactionMode: TRANSACTION_MODES.implicit,
timeout: '60',
limitRows: '10000',
statisticsMode: STATISTICS_MODES.none,
tracingLevel: TRACING_LEVELS.detailed,
};
Expand All @@ -38,6 +39,7 @@ describe('getChangedQueryExecutionSettingsDescription', () => {
...DEFAULT_QUERY_SETTINGS,
queryMode: QUERY_MODES.pg,
timeout: '63',
limitRows: '100',
};

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@ 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 {
if (content) {
result[settings.title] = content;
}
} else if (currentValue) {
result[settings.title] = 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,14 +1,22 @@
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 {z} from 'zod';

import {useTracingLevelOptionAvailable} from '../../../../store/reducers/capabilities/hooks';
import {
selectQueryAction,
setQueryAction,
} from '../../../../store/reducers/queryActions/queryActions';
import type {QuerySettings} from '../../../../types/store/query';
import type {
QueryMode,
QuerySettings,
StatisticsMode,
TracingLevel,
TransactionMode,
} from '../../../../types/store/query';
import {cn} from '../../../../utils/cn';
import {
useQueryExecutionSettings,
Expand All @@ -24,6 +32,15 @@ import './QuerySettingsDialog.scss';

const b = cn('ydb-query-settings-dialog');

const validationSchema = z.object({
timeout: z.coerce.number().positive().optional().or(z.string().length(0)),
limitRows: z.coerce.number().gt(0).lte(10_000).optional().or(z.string().length(0)),
queryMode: z.custom<QueryMode>(),
transactionMode: z.custom<TransactionMode>(),
statisticsMode: z.custom<StatisticsMode>().optional(),
tracingLevel: z.custom<TracingLevel>().optional(),
});

export function QuerySettingsDialog() {
const dispatch = useTypedDispatch();
const queryAction = useTypedSelector(selectQueryAction);
Expand Down Expand Up @@ -66,8 +83,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(validationSchema),
});

const enableTracingLevel = useTracingLevelOptionAvailable();
Expand All @@ -85,6 +107,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 +127,14 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm
render={({field}) => (
<React.Fragment>
<TextInput
id="timeout"
type="number"
{...field}
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 +155,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 +177,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 +198,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 +209,29 @@ 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}
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;
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"form.transaction-mode": "Уровень изоляции",
"form.statistics-mode": "Режим сбора статистики",
"form.tracing-level": "Tracing level",
"form.limit-rows": "Лимит строк",
"button-done": "Готово",
"button-cancel": "Отменить",
"form.timeout.seconds": "сек",
Expand Down
1 change: 1 addition & 0 deletions src/services/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ export class YdbEmbeddedAPI extends AxiosWrapper {
transaction_mode?: TransactionMode;
timeout?: Timeout;
query_id?: string;
limit_rows?: string;
},
{concurrentId, signal, withRetries}: AxiosOptions = {},
) {
Expand Down
1 change: 1 addition & 0 deletions src/store/reducers/executeQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ export const executeQueryApi = api.injectEndpoints({
querySettings.tracingLevel && enableTracingLevel
? TracingLevelNumber[querySettings.tracingLevel]
: undefined,
limit_rows: querySettings.limitRows || undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

will it be possible to run query without this setting at all?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

now it is running without this setting

or I didnt get your idea

Copy link
Contributor

Choose a reason for hiding this comment

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

Just wanted to double-check that limit_rows may be undefined and nothing breaks!

transaction_mode:
querySettings.transactionMode === 'implicit'
? undefined
Expand Down
1 change: 1 addition & 0 deletions src/types/store/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export interface QuerySettings {
queryMode: QueryMode;
transactionMode: TransactionMode;
timeout?: string;
limitRows?: string;
statisticsMode?: StatisticsMode;
tracingLevel?: TracingLevel;
}
Expand Down
1 change: 1 addition & 0 deletions src/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export const DEFAULT_QUERY_SETTINGS = {
queryMode: QUERY_MODES.query,
transactionMode: TRANSACTION_MODES.implicit,
timeout: '60',
limitRows: '10000',
statisticsMode: STATISTICS_MODES.none,
tracingLevel: TRACING_LEVELS.detailed,
} satisfies QuerySettings;
Expand Down
1 change: 1 addition & 0 deletions src/utils/hooks/useLastQueryExecutionSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export const useLastQueryExecutionSettings = () => {
queryMode: lastStorageSettings.queryMode,
statisticsMode: lastStorageSettings.statisticsMode,
tracingLevel: lastStorageSettings.tracingLevel,
limitRows: lastStorageSettings.limitRows,
timeout: lastStorageSettings.timeout,
}
: undefined;
Expand Down
1 change: 1 addition & 0 deletions src/utils/hooks/useQueryExecutionSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const useQueryExecutionSettings = () => {
timeout: storageSettings.timeout ?? DEFAULT_QUERY_SETTINGS.timeout,
statisticsMode: storageSettings.statisticsMode ?? DEFAULT_QUERY_SETTINGS.statisticsMode,
transactionMode: storageSettings.transactionMode ?? DEFAULT_QUERY_SETTINGS.transactionMode,
limitRows: storageSettings.limitRows ?? DEFAULT_QUERY_SETTINGS.limitRows,
tracingLevel: enableTracingLevel
? storageSettings.tracingLevel
: DEFAULT_QUERY_SETTINGS.tracingLevel,
Expand Down
Loading