From 60b550eb54c1b0018d34ec36e9d78f7b8a897790 Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Fri, 13 Sep 2024 12:12:10 +0300 Subject: [PATCH 01/11] feat: add rows limit to query settings --- .../getChangedQueryExecutionSettings.test.ts | 4 +++- ...dQueryExecutionSettingsDescription.test.ts | 3 +++ .../QuerySettingsDialog.scss | 1 + .../QuerySettingsDialog.tsx | 21 +++++++++++++++++++ .../Query/QuerySettingsDialog/constants.ts | 3 +++ .../Query/QuerySettingsDialog/i18n/en.json | 1 + .../Query/QuerySettingsDialog/i18n/ru.json | 1 + src/services/api.ts | 1 + src/store/reducers/executeQuery.ts | 1 + src/types/store/query.ts | 1 + src/utils/constants.ts | 1 + .../hooks/useLastQueryExecutionSettings.ts | 1 + src/utils/hooks/useQueryExecutionSettings.ts | 1 + 13 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts index 3b4d6bd08a..c4254ded7c 100644 --- a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts +++ b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts @@ -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, }; @@ -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', () => { diff --git a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.test.ts b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.test.ts index dec1fa5a8e..c1414d8e3b 100644 --- a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.test.ts +++ b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.test.ts @@ -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, }; @@ -38,6 +39,7 @@ describe('getChangedQueryExecutionSettingsDescription', () => { ...DEFAULT_QUERY_SETTINGS, queryMode: QUERY_MODES.pg, timeout: '63', + limitRows: '100', }; const result = getChangedQueryExecutionSettingsDescription({ @@ -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', }); }); diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.scss b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.scss index 0eae7fc89c..7a2b9ddfe8 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.scss +++ b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.scss @@ -28,6 +28,7 @@ flex: 6; } + &__limit-rows, &__timeout { width: 33.3%; margin-right: var(--g-spacing-2); diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx index 6789c19f21..40733d2519 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx +++ b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx @@ -179,6 +179,27 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm /> + + +
+ ( + + + + )} + /> +
+ { queryMode: lastStorageSettings.queryMode, statisticsMode: lastStorageSettings.statisticsMode, tracingLevel: lastStorageSettings.tracingLevel, + limitRows: lastStorageSettings.limitRows, timeout: lastStorageSettings.timeout, } : undefined; diff --git a/src/utils/hooks/useQueryExecutionSettings.ts b/src/utils/hooks/useQueryExecutionSettings.ts index 1325b67de6..0aeb53769f 100644 --- a/src/utils/hooks/useQueryExecutionSettings.ts +++ b/src/utils/hooks/useQueryExecutionSettings.ts @@ -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, From 5469b74254cead378f2b73fc0a2f15bae9db995a Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Mon, 16 Sep 2024 15:19:41 +0300 Subject: [PATCH 02/11] fix: review fixes --- package-lock.json | 53 +++++++++++++++ package.json | 2 + ...hangedQueryExecutionSettingsDescription.ts | 6 +- .../QuerySettingsDialog.tsx | 64 ++++++++++++++++--- .../QuerySettingsSelect.tsx | 2 + .../Query/QuerySettingsDialog/i18n/en.json | 2 + .../Query/QuerySettingsDialog/i18n/ru.json | 2 + 7 files changed, 119 insertions(+), 12 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5d2ebe8b16..a69f1ee2cc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -22,6 +22,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", @@ -50,6 +51,7 @@ "uuid": "^10.0.0", "web-vitals": "^1.1.2", "ydb-ui-components": "^4.2.0", + "yup": "^1.4.0", "zod": "^3.23.8" }, "devDependencies": { @@ -4084,6 +4086,15 @@ "node": ">=16.0.0" } }, + "node_modules/@hookform/resolvers": { + "version": "3.9.0", + "resolved": "https://registry.npmjs.org/@hookform/resolvers/-/resolvers-3.9.0.tgz", + "integrity": "sha512-bU0Gr4EepJ/EQsH/IwEzYLsT/PEj5C0ynLQ4m+GSHS+xKH4TfSelhluTgOaoc4kA5s7eCsQbM4wvZLzELmWzUg==", + "license": "MIT", + "peerDependencies": { + "react-hook-form": "^7.0.0" + } + }, "node_modules/@humanwhocodes/config-array": { "version": "0.11.8", "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.11.8.tgz", @@ -20437,6 +20448,12 @@ "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.13.1.tgz", "integrity": "sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==" }, + "node_modules/property-expr": { + "version": "2.0.6", + "resolved": "https://registry.npmjs.org/property-expr/-/property-expr-2.0.6.tgz", + "integrity": "sha512-SVtmxhRE/CGkn3eZY1T6pC8Nln6Fr/lu1mKSgRud0eC73whjGfoAogbn78LkD8aFL0zz3bAFerKSnOl7NlErBA==", + "license": "MIT" + }, "node_modules/proxy-addr": { "version": "2.0.7", "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.7.tgz", @@ -25210,6 +25227,12 @@ "integrity": "sha512-eHY7nBftgThBqOyHGVN+l8gF0BucP09fMo0oO/Lb0w1OF80dJv+lDVpXG60WMQvkcxAkNybKsrEIE3ZtKGmPrA==", "dev": true }, + "node_modules/tiny-case": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/tiny-case/-/tiny-case-1.0.3.tgz", + "integrity": "sha512-Eet/eeMhkO6TX8mnUteS9zgPbUMQa4I6Kkp5ORiBD5476/m+PIRiumP5tmh5ioJpH7k51Kehawy2UDfsnxxY8Q==", + "license": "MIT" + }, "node_modules/tiny-invariant": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/tiny-invariant/-/tiny-invariant-1.2.0.tgz", @@ -25261,6 +25284,12 @@ "node": ">=0.6" } }, + "node_modules/toposort": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/toposort/-/toposort-2.0.2.tgz", + "integrity": "sha512-0a5EOkAUp8D4moMi2W8ZF8jcga7BgZd91O/yabJCFY8az+XSzeGyTKs0Aoo897iV1Nj6guFq8orWDS96z91oGg==", + "license": "MIT" + }, "node_modules/tough-cookie": { "version": "4.1.4", "resolved": "https://registry.npmjs.org/tough-cookie/-/tough-cookie-4.1.4.tgz", @@ -26912,6 +26941,30 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/yup": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/yup/-/yup-1.4.0.tgz", + "integrity": "sha512-wPbgkJRCqIf+OHyiTBQoJiP5PFuAXaWiJK6AmYkzQAh5/c2K9hzSApBZG5wV9KoKSePF7sAxmNSvh/13YHkFDg==", + "license": "MIT", + "dependencies": { + "property-expr": "^2.0.5", + "tiny-case": "^1.0.3", + "toposort": "^2.0.2", + "type-fest": "^2.19.0" + } + }, + "node_modules/yup/node_modules/type-fest": { + "version": "2.19.0", + "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-2.19.0.tgz", + "integrity": "sha512-RAH822pAdBgcNMAfWnCBU3CFZcfZ/i1eZjwFU/dsLKumyuuP3niueg2UAukXYF0E2AAoc82ZSSf9J0WQBinzHA==", + "license": "(MIT OR CC0-1.0)", + "engines": { + "node": ">=12.20" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/zod": { "version": "3.23.8", "resolved": "https://registry.npmjs.org/zod/-/zod-3.23.8.tgz", diff --git a/package.json b/package.json index 830c4599c1..8ff0afd905 100644 --- a/package.json +++ b/package.json @@ -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", @@ -52,6 +53,7 @@ "uuid": "^10.0.0", "web-vitals": "^1.1.2", "ydb-ui-components": "^4.2.0", + "yup": "^1.4.0", "zod": "^3.23.8" }, "scripts": { diff --git a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts index cbd97c29b7..8c0e8ba702 100644 --- a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts +++ b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts @@ -21,8 +21,10 @@ export default function getChangedQueryExecutionSettingsDescription({ const content = settings.options.find((option) => option.value === currentValue) ?.content as string; - result[settings.title] = content; - } else { + if (content) { + result[settings.title] = content; + } + } else if (currentValue) { result[settings.title] = currentValue; } }); diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx index 40733d2519..b0a85aad8d 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx +++ b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx @@ -1,14 +1,22 @@ import React from 'react'; import {Dialog, Link as ExternalLink, Flex, TextInput} from '@gravity-ui/uikit'; +import {yupResolver} from '@hookform/resolvers/yup'; import {Controller, useForm} from 'react-hook-form'; +import * as yup from 'yup'; 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, @@ -24,6 +32,27 @@ import './QuerySettingsDialog.scss'; const b = cn('ydb-query-settings-dialog'); +const validationSchema = yup.object().shape({ + timeout: yup.string().test('is-within-range', i18n('form.validation.timeout'), (value) => { + if (!value) { + return true; + } + const num = Number(value); + return !isNaN(num) && num > 0; + }), + limitRows: yup.string().test('is-within-range', i18n('form.validation.limitRows'), (value) => { + if (!value) { + return true; + } + const num = Number(value); + return !isNaN(num) && num > 0 && num <= 100000; + }), + queryMode: yup.mixed().required(), + transactionMode: yup.mixed().required(), + statisticsMode: yup.mixed(), + tracingLevel: yup.mixed(), +}); + export function QuerySettingsDialog() { const dispatch = useTypedDispatch(); const queryAction = useTypedSelector(selectQueryAction); @@ -66,8 +95,13 @@ interface QuerySettingsFormProps { } function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsFormProps) { - const {control, handleSubmit} = useForm({ + const { + control, + handleSubmit, + formState: {errors}, + } = useForm({ defaultValues: initialValues, + resolver: yupResolver(validationSchema), }); const enableTracingLevel = useTracingLevelOptionAvailable(); @@ -85,6 +119,7 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm control={control} render={({field}) => ( ( {i18n('form.timeout.seconds')} @@ -128,6 +167,7 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm control={control} render={({field}) => ( ( ( ( - - - + )} /> diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsSelect.tsx b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsSelect.tsx index 9d5ab4967c..d0430b0d5a 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsSelect.tsx +++ b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsSelect.tsx @@ -23,6 +23,7 @@ type SelectType = QueryMode | TransactionMode | StatisticsMode | TracingLevel; type QuerySettingSelectOption = SelectOption & {isDefault?: boolean}; interface QuerySettingsSelectProps { + id?: string; setting: T; settingOptions: QuerySettingSelectOption[]; onUpdateSetting: (mode: T) => void; @@ -32,6 +33,7 @@ export function QuerySettingsSelect(props: QuerySettingsSe return (
+ id={props.id} options={props.settingOptions} value={[props.setting]} onUpdate={(value) => { diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json index 5cd37302f7..55b7c23986 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json +++ b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json @@ -9,6 +9,8 @@ "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" } diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json index 4686592dca..ca93e2d755 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json +++ b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json @@ -9,6 +9,8 @@ "button-done": "Готово", "button-cancel": "Отменить", "form.timeout.seconds": "сек", + "form.validation.timeout": "Таймаут должен быть положительным", + "form.validation.limitRows": "Лимит строк должен быть между 1 и 100000", "description.default": " (default)", "docs": "Документация" } From df16b2d0b9a74c445b8da702c8788313c008a3e0 Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Mon, 16 Sep 2024 17:00:12 +0300 Subject: [PATCH 03/11] fix: use zod instead of yup --- package-lock.json | 43 ---------------- package.json | 1 - .../QuerySettingsDialog.tsx | 50 +++++++++++-------- 3 files changed, 28 insertions(+), 66 deletions(-) diff --git a/package-lock.json b/package-lock.json index ba20004c86..73dcec6ce5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -51,7 +51,6 @@ "uuid": "^10.0.0", "web-vitals": "^1.1.2", "ydb-ui-components": "^4.2.0", - "yup": "^1.4.0", "zod": "^3.23.8" }, "devDependencies": { @@ -20450,12 +20449,6 @@ "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.13.1.tgz", "integrity": "sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==" }, - "node_modules/property-expr": { - "version": "2.0.6", - "resolved": "https://registry.npmjs.org/property-expr/-/property-expr-2.0.6.tgz", - "integrity": "sha512-SVtmxhRE/CGkn3eZY1T6pC8Nln6Fr/lu1mKSgRud0eC73whjGfoAogbn78LkD8aFL0zz3bAFerKSnOl7NlErBA==", - "license": "MIT" - }, "node_modules/proxy-addr": { "version": "2.0.7", "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.7.tgz", @@ -25229,12 +25222,6 @@ "integrity": "sha512-eHY7nBftgThBqOyHGVN+l8gF0BucP09fMo0oO/Lb0w1OF80dJv+lDVpXG60WMQvkcxAkNybKsrEIE3ZtKGmPrA==", "dev": true }, - "node_modules/tiny-case": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/tiny-case/-/tiny-case-1.0.3.tgz", - "integrity": "sha512-Eet/eeMhkO6TX8mnUteS9zgPbUMQa4I6Kkp5ORiBD5476/m+PIRiumP5tmh5ioJpH7k51Kehawy2UDfsnxxY8Q==", - "license": "MIT" - }, "node_modules/tiny-invariant": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/tiny-invariant/-/tiny-invariant-1.2.0.tgz", @@ -25286,12 +25273,6 @@ "node": ">=0.6" } }, - "node_modules/toposort": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/toposort/-/toposort-2.0.2.tgz", - "integrity": "sha512-0a5EOkAUp8D4moMi2W8ZF8jcga7BgZd91O/yabJCFY8az+XSzeGyTKs0Aoo897iV1Nj6guFq8orWDS96z91oGg==", - "license": "MIT" - }, "node_modules/tough-cookie": { "version": "4.1.4", "resolved": "https://registry.npmjs.org/tough-cookie/-/tough-cookie-4.1.4.tgz", @@ -26943,30 +26924,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/yup": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/yup/-/yup-1.4.0.tgz", - "integrity": "sha512-wPbgkJRCqIf+OHyiTBQoJiP5PFuAXaWiJK6AmYkzQAh5/c2K9hzSApBZG5wV9KoKSePF7sAxmNSvh/13YHkFDg==", - "license": "MIT", - "dependencies": { - "property-expr": "^2.0.5", - "tiny-case": "^1.0.3", - "toposort": "^2.0.2", - "type-fest": "^2.19.0" - } - }, - "node_modules/yup/node_modules/type-fest": { - "version": "2.19.0", - "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-2.19.0.tgz", - "integrity": "sha512-RAH822pAdBgcNMAfWnCBU3CFZcfZ/i1eZjwFU/dsLKumyuuP3niueg2UAukXYF0E2AAoc82ZSSf9J0WQBinzHA==", - "license": "(MIT OR CC0-1.0)", - "engines": { - "node": ">=12.20" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/zod": { "version": "3.23.8", "resolved": "https://registry.npmjs.org/zod/-/zod-3.23.8.tgz", diff --git a/package.json b/package.json index b7514b577c..ea352f7bba 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,6 @@ "uuid": "^10.0.0", "web-vitals": "^1.1.2", "ydb-ui-components": "^4.2.0", - "yup": "^1.4.0", "zod": "^3.23.8" }, "scripts": { diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx index b0a85aad8d..59262b1157 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx +++ b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx @@ -1,9 +1,9 @@ import React from 'react'; import {Dialog, Link as ExternalLink, Flex, TextInput} from '@gravity-ui/uikit'; -import {yupResolver} from '@hookform/resolvers/yup'; +import {zodResolver} from '@hookform/resolvers/zod'; import {Controller, useForm} from 'react-hook-form'; -import * as yup from 'yup'; +import {z} from 'zod'; import {useTracingLevelOptionAvailable} from '../../../../store/reducers/capabilities/hooks'; import { @@ -32,25 +32,31 @@ import './QuerySettingsDialog.scss'; const b = cn('ydb-query-settings-dialog'); -const validationSchema = yup.object().shape({ - timeout: yup.string().test('is-within-range', i18n('form.validation.timeout'), (value) => { - if (!value) { - return true; - } - const num = Number(value); - return !isNaN(num) && num > 0; - }), - limitRows: yup.string().test('is-within-range', i18n('form.validation.limitRows'), (value) => { - if (!value) { - return true; - } - const num = Number(value); - return !isNaN(num) && num > 0 && num <= 100000; - }), - queryMode: yup.mixed().required(), - transactionMode: yup.mixed().required(), - statisticsMode: yup.mixed(), - tracingLevel: yup.mixed(), +const validationSchema = z.object({ + timeout: z.string().refine( + (value) => { + if (!value) { + return true; + } + const num = Number(value); + return !isNaN(num) && num > 0; + }, + {message: i18n('form.validation.timeout')}, + ), + limitRows: z.string().refine( + (value) => { + if (!value) { + return true; + } + const num = Number(value); + return !isNaN(num) && num > 0 && num <= 100000; + }, + {message: i18n('form.validation.limitRows')}, + ), + queryMode: z.custom(), + transactionMode: z.custom(), + statisticsMode: z.custom().optional(), + tracingLevel: z.custom().optional(), }); export function QuerySettingsDialog() { @@ -101,7 +107,7 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm formState: {errors}, } = useForm({ defaultValues: initialValues, - resolver: yupResolver(validationSchema), + resolver: zodResolver(validationSchema), }); const enableTracingLevel = useTracingLevelOptionAvailable(); From e55c633222dcb21d87ca76f97d93d351c43535d7 Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Mon, 16 Sep 2024 18:23:21 +0300 Subject: [PATCH 04/11] fix: review fix --- .../QuerySettingsDialog.tsx | 22 ++----------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx index 59262b1157..ec50e426b7 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx +++ b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx @@ -33,26 +33,8 @@ import './QuerySettingsDialog.scss'; const b = cn('ydb-query-settings-dialog'); const validationSchema = z.object({ - timeout: z.string().refine( - (value) => { - if (!value) { - return true; - } - const num = Number(value); - return !isNaN(num) && num > 0; - }, - {message: i18n('form.validation.timeout')}, - ), - limitRows: z.string().refine( - (value) => { - if (!value) { - return true; - } - const num = Number(value); - return !isNaN(num) && num > 0 && num <= 100000; - }, - {message: i18n('form.validation.limitRows')}, - ), + 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(), transactionMode: z.custom(), statisticsMode: z.custom().optional(), From b87e848659c80eaa53c77c4c394c802679aacf68 Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Mon, 16 Sep 2024 19:25:07 +0300 Subject: [PATCH 05/11] fix: review fixes --- .../utils/getChangedQueryExecutionSettingsDescription.ts | 7 ++++--- .../Tenant/Query/QuerySettingsDialog/i18n/en.json | 2 -- .../Tenant/Query/QuerySettingsDialog/i18n/ru.json | 2 -- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts index 8c0e8ba702..20b86c55b8 100644 --- a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts +++ b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts @@ -15,11 +15,12 @@ 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; if (content) { result[settings.title] = content; diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json index 55b7c23986..5cd37302f7 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json +++ b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json @@ -9,8 +9,6 @@ "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" } diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json index ca93e2d755..4686592dca 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json +++ b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json @@ -9,8 +9,6 @@ "button-done": "Готово", "button-cancel": "Отменить", "form.timeout.seconds": "сек", - "form.validation.timeout": "Таймаут должен быть положительным", - "form.validation.limitRows": "Лимит строк должен быть между 1 и 100000", "description.default": " (default)", "docs": "Документация" } From f572b1a449c3654f83307ecfcd291dfc626f2763 Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Tue, 17 Sep 2024 11:56:55 +0300 Subject: [PATCH 06/11] fix: revert changes --- .../QuerySettingsDialog.tsx | 23 +++++++++++++++++-- .../Query/QuerySettingsDialog/i18n/en.json | 2 ++ .../Query/QuerySettingsDialog/i18n/ru.json | 2 ++ src/services/api.ts | 2 +- src/store/reducers/executeQuery.ts | 4 +++- 5 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx index ec50e426b7..21bf153f10 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx +++ b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx @@ -33,8 +33,27 @@ 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)), + timeout: z.string().refine( + (value) => { + if (!value) { + return true; + } + const num = Number(value); + return !isNaN(num) && num > 0; + }, + {message: i18n('form.validation.timeout')}, + ), + limitRows: z.string().refine( + (value) => { + if (!value) { + return true; + } + const num = Number(value); + return !isNaN(num) && num > 0 && num <= 100000; + }, + {message: i18n('form.validation.limitRows')}, + ), + queryMode: z.custom(), transactionMode: z.custom(), statisticsMode: z.custom().optional(), diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json index 5cd37302f7..55b7c23986 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json +++ b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/en.json @@ -9,6 +9,8 @@ "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" } diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json index 4686592dca..ca93e2d755 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json +++ b/src/containers/Tenant/Query/QuerySettingsDialog/i18n/ru.json @@ -9,6 +9,8 @@ "button-done": "Готово", "button-cancel": "Отменить", "form.timeout.seconds": "сек", + "form.validation.timeout": "Таймаут должен быть положительным", + "form.validation.limitRows": "Лимит строк должен быть между 1 и 100000", "description.default": " (default)", "docs": "Документация" } diff --git a/src/services/api.ts b/src/services/api.ts index 1ef43b00d0..72e7c6dfa8 100644 --- a/src/services/api.ts +++ b/src/services/api.ts @@ -543,7 +543,7 @@ export class YdbEmbeddedAPI extends AxiosWrapper { transaction_mode?: TransactionMode; timeout?: Timeout; query_id?: string; - limit_rows?: string; + limit_rows?: number; }, {concurrentId, signal, withRetries}: AxiosOptions = {}, ) { diff --git a/src/store/reducers/executeQuery.ts b/src/store/reducers/executeQuery.ts index b24d6888c3..c197ae858a 100644 --- a/src/store/reducers/executeQuery.ts +++ b/src/store/reducers/executeQuery.ts @@ -233,7 +233,9 @@ export const executeQueryApi = api.injectEndpoints({ querySettings.tracingLevel && enableTracingLevel ? TracingLevelNumber[querySettings.tracingLevel] : undefined, - limit_rows: querySettings.limitRows || undefined, + limit_rows: isNumeric(querySettings.limitRows) + ? Number(querySettings.limitRows) + : undefined, transaction_mode: querySettings.transactionMode === 'implicit' ? undefined From 56f9e3fde77ff66f5dd671211939aa050db597ad Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Wed, 18 Sep 2024 15:06:33 +0300 Subject: [PATCH 07/11] fix: review fixes --- .../getChangedQueryExecutionSettings.test.ts | 17 +++----- ...dQueryExecutionSettingsDescription.test.ts | 17 +++----- ...hangedQueryExecutionSettingsDescription.ts | 2 +- .../QuerySettingsDialog.tsx | 42 +++---------------- src/services/settings.ts | 3 +- src/types/store/query.ts | 12 ++---- src/utils/constants.ts | 12 ------ src/utils/hooks/useChangedQuerySettings.ts | 7 +--- .../hooks/useLastQueryExecutionSettings.ts | 10 +---- src/utils/hooks/useQueryExecutionSettings.ts | 13 +++--- src/utils/query.ts | 27 ++++++++++++ 11 files changed, 58 insertions(+), 104 deletions(-) diff --git a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts index c4254ded7c..b27880cf16 100644 --- a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts +++ b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts @@ -1,20 +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', - limitRows: '10000', - statisticsMode: STATISTICS_MODES.none, - tracingLevel: TRACING_LEVELS.detailed, -}; - import getChangedQueryExecutionSettings from './getChangedQueryExecutionSettings'; describe('getChangedQueryExecutionSettings', () => { @@ -28,8 +20,8 @@ describe('getChangedQueryExecutionSettings', () => { const currentSettings: QuerySettings = { ...DEFAULT_QUERY_SETTINGS, queryMode: QUERY_MODES.data, - timeout: '30', - limitRows: '100', + timeout: 30, + limitRows: 100, }; const result = getChangedQueryExecutionSettings(currentSettings, DEFAULT_QUERY_SETTINGS); expect(result).toEqual(['queryMode', 'timeout', 'limitRows']); @@ -39,7 +31,8 @@ describe('getChangedQueryExecutionSettings', () => { 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, }; diff --git a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.test.ts b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.test.ts index c1414d8e3b..d8e23f1269 100644 --- a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.test.ts +++ b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.test.ts @@ -1,5 +1,6 @@ import type {QuerySettings} from '../../../../../types/store/query'; import { + DEFAULT_QUERY_SETTINGS, QUERY_MODES, QUERY_MODES_TITLES, STATISTICS_MODES, @@ -13,15 +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', - limitRows: '10000', - 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}; @@ -38,8 +30,8 @@ describe('getChangedQueryExecutionSettingsDescription', () => { const currentSettings: QuerySettings = { ...DEFAULT_QUERY_SETTINGS, queryMode: QUERY_MODES.pg, - timeout: '63', - limitRows: '100', + timeout: 63, + limitRows: 100, }; const result = getChangedQueryExecutionSettingsDescription({ @@ -61,7 +53,8 @@ describe('getChangedQueryExecutionSettingsDescription', () => { 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, }; diff --git a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts index 20b86c55b8..7ed4f987a1 100644 --- a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts +++ b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription.ts @@ -26,7 +26,7 @@ export default function getChangedQueryExecutionSettingsDescription({ result[settings.title] = content; } } else if (currentValue) { - result[settings.title] = currentValue; + result[settings.title] = String(currentValue); } }); diff --git a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx index 21bf153f10..a34fe13a81 100644 --- a/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx +++ b/src/containers/Tenant/Query/QuerySettingsDialog/QuerySettingsDialog.tsx @@ -3,26 +3,20 @@ 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 { - QueryMode, - QuerySettings, - StatisticsMode, - TracingLevel, - TransactionMode, -} from '../../../../types/store/query'; +import type {QuerySettings} from '../../../../types/store/query'; import {cn} from '../../../../utils/cn'; import { useQueryExecutionSettings, useTypedDispatch, useTypedSelector, } from '../../../../utils/hooks'; +import {querySettingsValidationSchema} from '../../../../utils/query'; import {QuerySettingsSelect} from './QuerySettingsSelect'; import {QUERY_SETTINGS_FIELD_SETTINGS} from './constants'; @@ -32,34 +26,6 @@ import './QuerySettingsDialog.scss'; const b = cn('ydb-query-settings-dialog'); -const validationSchema = z.object({ - timeout: z.string().refine( - (value) => { - if (!value) { - return true; - } - const num = Number(value); - return !isNaN(num) && num > 0; - }, - {message: i18n('form.validation.timeout')}, - ), - limitRows: z.string().refine( - (value) => { - if (!value) { - return true; - } - const num = Number(value); - return !isNaN(num) && num > 0 && num <= 100000; - }, - {message: i18n('form.validation.limitRows')}, - ), - - queryMode: z.custom(), - transactionMode: z.custom(), - statisticsMode: z.custom().optional(), - tracingLevel: z.custom().optional(), -}); - export function QuerySettingsDialog() { const dispatch = useTypedDispatch(); const queryAction = useTypedSelector(selectQueryAction); @@ -108,7 +74,7 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm formState: {errors}, } = useForm({ defaultValues: initialValues, - resolver: zodResolver(validationSchema), + resolver: zodResolver(querySettingsValidationSchema), }); const enableTracingLevel = useTracingLevelOptionAvailable(); @@ -149,6 +115,7 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm id="timeout" type="number" {...field} + value={field.value?.toString()} className={b('timeout')} placeholder="60" validationState={errors.timeout ? 'invalid' : undefined} @@ -241,6 +208,7 @@ function QuerySettingsForm({initialValues, onSubmit, onClose}: QuerySettingsForm id="limitRows" type="number" {...field} + value={field.value?.toString()} className={b('limit-rows')} placeholder="10000" validationState={errors.limitRows ? 'invalid' : undefined} diff --git a/src/services/settings.ts b/src/services/settings.ts index feed25697c..2221004156 100644 --- a/src/services/settings.ts +++ b/src/services/settings.ts @@ -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, @@ -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; diff --git a/src/types/store/query.ts b/src/types/store/query.ts index 857be698b5..51845561db 100644 --- a/src/types/store/query.ts +++ b/src/types/store/query.ts @@ -1,3 +1,5 @@ +import type {z} from 'zod'; + import type { QUERY_ACTIONS, QUERY_MODES, @@ -5,6 +7,7 @@ import type { STATISTICS_MODES, TRACING_LEVELS, TRANSACTION_MODES, + querySettingsValidationSchema, } from '../../utils/query'; import type {IResponseError, NetworkError} from '../api/error'; import type { @@ -37,14 +40,7 @@ export interface QueryRequestParams { query: string; } -export interface QuerySettings { - queryMode: QueryMode; - transactionMode: TransactionMode; - timeout?: string; - limitRows?: string; - statisticsMode?: StatisticsMode; - tracingLevel?: TracingLevel; -} +export type QuerySettings = z.infer; export type QueryErrorResponse = IResponseError; export type QueryError = NetworkError | QueryErrorResponse; diff --git a/src/utils/constants.ts b/src/utils/constants.ts index d9c0033198..a5dc07c150 100644 --- a/src/utils/constants.ts +++ b/src/utils/constants.ts @@ -2,9 +2,6 @@ import DataTable from '@gravity-ui/react-data-table'; import type {Settings} from '@gravity-ui/react-data-table'; import {EType} from '../types/api/tablet'; -import type {QuerySettings} from '../types/store/query'; - -import {QUERY_MODES, STATISTICS_MODES, TRACING_LEVELS, TRANSACTION_MODES} from './query'; const SECOND = 1000; @@ -122,15 +119,6 @@ export const TENANT_OVERVIEW_TABLES_SETTINGS = { dynamicRender: false, } as const; -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; - export const QUERY_EXECUTION_SETTINGS_KEY = 'queryExecutionSettings'; export const LAST_QUERY_EXECUTION_SETTINGS_KEY = 'last_query_execution_settings'; export const QUERY_SETTINGS_BANNER_LAST_CLOSED_KEY = 'querySettingsBannerLastClosed'; diff --git a/src/utils/hooks/useChangedQuerySettings.ts b/src/utils/hooks/useChangedQuerySettings.ts index 869fc0814d..b6e05d2e21 100644 --- a/src/utils/hooks/useChangedQuerySettings.ts +++ b/src/utils/hooks/useChangedQuerySettings.ts @@ -1,10 +1,7 @@ import getChangedQueryExecutionSettings from '../../containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings'; import getChangedQueryExecutionSettingsDescription from '../../containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettingsDescription'; -import { - DEFAULT_QUERY_SETTINGS, - QUERY_SETTINGS_BANNER_LAST_CLOSED_KEY, - WEEK_IN_SECONDS, -} from '../constants'; +import {QUERY_SETTINGS_BANNER_LAST_CLOSED_KEY, WEEK_IN_SECONDS} from '../constants'; +import {DEFAULT_QUERY_SETTINGS} from '../query'; import {useLastQueryExecutionSettings} from './useLastQueryExecutionSettings'; import {useQueryExecutionSettings} from './useQueryExecutionSettings'; diff --git a/src/utils/hooks/useLastQueryExecutionSettings.ts b/src/utils/hooks/useLastQueryExecutionSettings.ts index 0e3face8e8..7c3e08042e 100644 --- a/src/utils/hooks/useLastQueryExecutionSettings.ts +++ b/src/utils/hooks/useLastQueryExecutionSettings.ts @@ -1,5 +1,6 @@ import type {QuerySettings} from '../../types/store/query'; import {LAST_QUERY_EXECUTION_SETTINGS_KEY} from '../constants'; +import {querySettingsValidationSchema} from '../query'; import {useSetting} from './useSetting'; @@ -9,14 +10,7 @@ export const useLastQueryExecutionSettings = () => { ); const lastSettings: QuerySettings | undefined = lastStorageSettings - ? { - transactionMode: lastStorageSettings.transactionMode, - queryMode: lastStorageSettings.queryMode, - statisticsMode: lastStorageSettings.statisticsMode, - tracingLevel: lastStorageSettings.tracingLevel, - limitRows: lastStorageSettings.limitRows, - timeout: lastStorageSettings.timeout, - } + ? querySettingsValidationSchema.parse(lastStorageSettings) : undefined; return [lastSettings, setLastSettings] as const; diff --git a/src/utils/hooks/useQueryExecutionSettings.ts b/src/utils/hooks/useQueryExecutionSettings.ts index 0aeb53769f..b5516673be 100644 --- a/src/utils/hooks/useQueryExecutionSettings.ts +++ b/src/utils/hooks/useQueryExecutionSettings.ts @@ -1,6 +1,7 @@ import {useTracingLevelOptionAvailable} from '../../store/reducers/capabilities/hooks'; import type {QuerySettings} from '../../types/store/query'; -import {DEFAULT_QUERY_SETTINGS, QUERY_EXECUTION_SETTINGS_KEY} from '../constants'; +import {QUERY_EXECUTION_SETTINGS_KEY} from '../constants'; +import {DEFAULT_QUERY_SETTINGS, querySettingsValidationSchema} from '../query'; import {useSetting} from './useSetting'; @@ -8,14 +9,12 @@ export const useQueryExecutionSettings = () => { const enableTracingLevel = useTracingLevelOptionAvailable(); const [storageSettings, setSettings] = useSetting(QUERY_EXECUTION_SETTINGS_KEY); + const validatedSettings = querySettingsValidationSchema.parse(storageSettings); + const settings: QuerySettings = { - queryMode: storageSettings.queryMode ?? DEFAULT_QUERY_SETTINGS.queryMode, - 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, + ...validatedSettings, tracingLevel: enableTracingLevel - ? storageSettings.tracingLevel + ? validatedSettings.tracingLevel : DEFAULT_QUERY_SETTINGS.tracingLevel, }; diff --git a/src/utils/query.ts b/src/utils/query.ts index b0949da617..87280c2197 100644 --- a/src/utils/query.ts +++ b/src/utils/query.ts @@ -1,3 +1,5 @@ +import {z} from 'zod'; + import {YQLType} from '../types'; import type { AnyExecuteResponse, @@ -330,3 +332,28 @@ export const parseQueryErrorToString = (error: unknown) => { return parsedError?.error?.message; }; + +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, +}; + +export const querySettingsValidationSchema = z.object({ + timeout: z.preprocess( + (val) => (val === '' ? undefined : val), + z.coerce.number().positive().default(DEFAULT_QUERY_SETTINGS.timeout), + ), + limitRows: z.preprocess( + (val) => (val === '' ? undefined : val), + + z.coerce.number().gt(0).lte(10_000).default(DEFAULT_QUERY_SETTINGS.limitRows), + ), + queryMode: z.custom().default(DEFAULT_QUERY_SETTINGS.queryMode), + transactionMode: z.custom().default(DEFAULT_QUERY_SETTINGS.transactionMode), + statisticsMode: z.custom().default(DEFAULT_QUERY_SETTINGS.statisticsMode), + tracingLevel: z.custom().default(DEFAULT_QUERY_SETTINGS.tracingLevel), +}); From df1c0bbc7cca41323331353d3f9c99e40dd0889f Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Wed, 18 Sep 2024 16:05:50 +0300 Subject: [PATCH 08/11] fix: long running query test --- tests/suites/tenant/queryEditor/constants.ts | 31 ++------------------ 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/tests/suites/tenant/queryEditor/constants.ts b/tests/suites/tenant/queryEditor/constants.ts index 3238b159c0..be9ca373c5 100644 --- a/tests/suites/tenant/queryEditor/constants.ts +++ b/tests/suites/tenant/queryEditor/constants.ts @@ -1,32 +1,7 @@ // Long running query for tests // May cause Memory exceed on real database -export const longRunningQuery = ` -PRAGMA TablePathPrefix(""); +const simpleQuery = 'SELECT 1;'; -SELECT COUNT(*) AS total_count -FROM ( - SELECT 1 AS dummy - FROM - (SELECT 1 AS d UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL - SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1) AS t1 - CROSS JOIN - (SELECT 1 AS d UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL - SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1) AS t2 - CROSS JOIN - (SELECT 1 AS d UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL - SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1) AS t3 - CROSS JOIN - (SELECT 1 AS d UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL - SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1) AS t4 - CROSS JOIN - (SELECT 1 AS d UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL - SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1) AS t5 - CROSS JOIN - (SELECT 1 AS d UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL - SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1) AS t6 - CROSS JOIN - (SELECT 1 AS d UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL - SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1) AS t7 -) AS large_table - `; +// 400 is pretty enough +export const longRunningQuery = new Array(400).fill(simpleQuery).join(''); From dcb0f95d4a085375ac5a465d2b662ee9d7ea075a Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Thu, 19 Sep 2024 13:12:55 +0300 Subject: [PATCH 09/11] fix: review fixes --- .../getChangedQueryExecutionSettings.test.ts | 4 +-- .../utils/getChangedQueryExecutionSettings.ts | 5 ++- .../hooks/useLastQueryExecutionSettings.ts | 9 +++-- src/utils/query.ts | 34 +++++++++++++++---- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts index b27880cf16..086a7b1bad 100644 --- a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts +++ b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.test.ts @@ -21,10 +21,10 @@ describe('getChangedQueryExecutionSettings', () => { ...DEFAULT_QUERY_SETTINGS, queryMode: QUERY_MODES.data, timeout: 30, - limitRows: 100, + limitRows: undefined, }; const result = getChangedQueryExecutionSettings(currentSettings, DEFAULT_QUERY_SETTINGS); - expect(result).toEqual(['queryMode', 'timeout', 'limitRows']); + expect(result).toEqual(['queryMode', 'timeout']); }); it('should return all keys if all settings have changed', () => { diff --git a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.ts b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.ts index a456a2e5b2..a0b2929523 100644 --- a/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.ts +++ b/src/containers/Tenant/Query/QueryEditorControls/utils/getChangedQueryExecutionSettings.ts @@ -8,6 +8,9 @@ export default function getChangedQueryExecutionSettings( const defaultMap = new Map(Object.entries(defaultSettings)); return Array.from(currentMap.keys()).filter( - (key) => currentMap.get(key) !== defaultMap.get(key), + (key) => + currentMap.has(key) && + currentMap.get(key) !== undefined && + currentMap.get(key) !== defaultMap.get(key), ) as (keyof QuerySettings)[]; } diff --git a/src/utils/hooks/useLastQueryExecutionSettings.ts b/src/utils/hooks/useLastQueryExecutionSettings.ts index 7c3e08042e..b69e537949 100644 --- a/src/utils/hooks/useLastQueryExecutionSettings.ts +++ b/src/utils/hooks/useLastQueryExecutionSettings.ts @@ -8,10 +8,13 @@ export const useLastQueryExecutionSettings = () => { const [lastStorageSettings, setLastSettings] = useSetting( LAST_QUERY_EXECUTION_SETTINGS_KEY, ); + let lastSettings: QuerySettings | undefined; - const lastSettings: QuerySettings | undefined = lastStorageSettings - ? querySettingsValidationSchema.parse(lastStorageSettings) - : undefined; + try { + lastSettings = querySettingsValidationSchema.parse(lastStorageSettings); + } catch (error) { + lastSettings = undefined; + } return [lastSettings, setLastSettings] as const; }; diff --git a/src/utils/query.ts b/src/utils/query.ts index 87280c2197..48fbc9be3d 100644 --- a/src/utils/query.ts +++ b/src/utils/query.ts @@ -342,18 +342,38 @@ export const DEFAULT_QUERY_SETTINGS = { tracingLevel: TRACING_LEVELS.detailed, }; +export const queryModeSchema = z.nativeEnum(QUERY_MODES); +export const transactionModeSchema = z.nativeEnum(TRANSACTION_MODES); +export const statisticsModeSchema = z.nativeEnum(STATISTICS_MODES); +export const tracingLevelSchema = z.nativeEnum(TRACING_LEVELS); export const querySettingsValidationSchema = z.object({ timeout: z.preprocess( (val) => (val === '' ? undefined : val), - z.coerce.number().positive().default(DEFAULT_QUERY_SETTINGS.timeout), + z.coerce.number().positive().or(z.undefined()), ), limitRows: z.preprocess( (val) => (val === '' ? undefined : val), - - z.coerce.number().gt(0).lte(10_000).default(DEFAULT_QUERY_SETTINGS.limitRows), + z.coerce.number().gt(0).lte(10_000).or(z.undefined()), ), - queryMode: z.custom().default(DEFAULT_QUERY_SETTINGS.queryMode), - transactionMode: z.custom().default(DEFAULT_QUERY_SETTINGS.transactionMode), - statisticsMode: z.custom().default(DEFAULT_QUERY_SETTINGS.statisticsMode), - tracingLevel: z.custom().default(DEFAULT_QUERY_SETTINGS.tracingLevel), + queryMode: queryModeSchema, + transactionMode: transactionModeSchema, + statisticsMode: statisticsModeSchema, + tracingLevel: tracingLevelSchema, }); + +export const querySettingsRestoreSchema = z + .object({ + timeout: z.preprocess( + (val) => (val === '' ? undefined : val), + z.coerce.number().positive().catch(DEFAULT_QUERY_SETTINGS.timeout), + ), + limitRows: z.preprocess( + (val) => (val === '' ? undefined : val), + z.coerce.number().gt(0).lte(10_000).catch(DEFAULT_QUERY_SETTINGS.limitRows), + ), + queryMode: queryModeSchema.catch(DEFAULT_QUERY_SETTINGS.queryMode), + transactionMode: transactionModeSchema.catch(DEFAULT_QUERY_SETTINGS.transactionMode), + statisticsMode: statisticsModeSchema.catch(DEFAULT_QUERY_SETTINGS.statisticsMode), + tracingLevel: tracingLevelSchema.catch(DEFAULT_QUERY_SETTINGS.tracingLevel), + }) + .catch(DEFAULT_QUERY_SETTINGS); From 2c30b28311069092630ad5178cb1cbc68d1f7543 Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Thu, 19 Sep 2024 13:35:12 +0300 Subject: [PATCH 10/11] fix: nanofix --- src/utils/hooks/useQueryExecutionSettings.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/hooks/useQueryExecutionSettings.ts b/src/utils/hooks/useQueryExecutionSettings.ts index b5516673be..4b4dce6c2a 100644 --- a/src/utils/hooks/useQueryExecutionSettings.ts +++ b/src/utils/hooks/useQueryExecutionSettings.ts @@ -1,7 +1,7 @@ import {useTracingLevelOptionAvailable} from '../../store/reducers/capabilities/hooks'; import type {QuerySettings} from '../../types/store/query'; import {QUERY_EXECUTION_SETTINGS_KEY} from '../constants'; -import {DEFAULT_QUERY_SETTINGS, querySettingsValidationSchema} from '../query'; +import {DEFAULT_QUERY_SETTINGS, querySettingsRestoreSchema} from '../query'; import {useSetting} from './useSetting'; @@ -9,7 +9,7 @@ export const useQueryExecutionSettings = () => { const enableTracingLevel = useTracingLevelOptionAvailable(); const [storageSettings, setSettings] = useSetting(QUERY_EXECUTION_SETTINGS_KEY); - const validatedSettings = querySettingsValidationSchema.parse(storageSettings); + const validatedSettings = querySettingsRestoreSchema.parse(storageSettings); const settings: QuerySettings = { ...validatedSettings, From f64c01102fbd650c945f660788c5031d42c95047 Mon Sep 17 00:00:00 2001 From: Anton Standrik Date: Fri, 20 Sep 2024 15:32:28 +0300 Subject: [PATCH 11/11] fix: timeout and limitRows --- src/utils/query.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/query.ts b/src/utils/query.ts index 01a38554ae..4da0f40441 100644 --- a/src/utils/query.ts +++ b/src/utils/query.ts @@ -366,11 +366,11 @@ export const querySettingsRestoreSchema = z .object({ timeout: z.preprocess( (val) => (val === '' ? undefined : val), - z.coerce.number().positive().catch(DEFAULT_QUERY_SETTINGS.timeout), + z.coerce.number().positive().optional().catch(DEFAULT_QUERY_SETTINGS.timeout), ), limitRows: z.preprocess( (val) => (val === '' ? undefined : val), - z.coerce.number().gt(0).lte(10_000).catch(DEFAULT_QUERY_SETTINGS.limitRows), + z.coerce.number().gt(0).lte(10_000).optional().catch(DEFAULT_QUERY_SETTINGS.limitRows), ), queryMode: queryModeSchema.catch(DEFAULT_QUERY_SETTINGS.queryMode), transactionMode: transactionModeSchema.catch(DEFAULT_QUERY_SETTINGS.transactionMode),