dbeaver/pro#8298 fix: preserve query param order, fix incorrect repla…#4164
dbeaver/pro#8298 fix: preserve query param order, fix incorrect repla…#4164sergeyteleshev merged 5 commits intodevelfrom
Conversation
…cement, add tooltip
There was a problem hiding this comment.
Pull request overview
This PR updates the SQL editor query-parameters confirmation dialog to preserve the original parameter order, improve parameter token replacement in the query preview, and expose tooltips for parameters via PropertiesTable.
Changes:
- Preserve query parameter order and de-duplicate parameter names when building the confirmation dialog state.
- Replace parameter tokens in the query preview using regex-based matching with escaping.
- Add an
orderedoption toPropertiesTableto allow preserving caller-provided ordering (and use it for query parameters), and populatedescriptionfor tooltips.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| webapp/packages/plugin-sql-editor/src/renderQueryParamsForConfirmation.tsx | Adds ordered parameter rendering + regex-based token replacement + tooltip description wiring. |
| webapp/packages/plugin-sql-editor/src/QueryDataSource.ts | Preserves parameter order from the server event and passes it through to the confirmation renderer. |
| webapp/packages/core-blocks/src/PropertiesTable/PropertiesTable.tsx | Adds ordered prop to optionally skip default sorting by displayName. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| function replaceQueryToken(query: string, pattern: string, value: string): string { | ||
| return query.replace(new RegExp(pattern, 'g'), value); |
There was a problem hiding this comment.
replaceQueryToken uses String.prototype.replace with a string replacement. If value contains $ sequences (e.g. $&, $1), JS will treat them as replacement tokens and the preview query will be corrupted. Use a replacer function (e.g. replace(regex, () => value)) or escape $ in the replacement to ensure literal insertion.
| return query.replace(new RegExp(pattern, 'g'), value); | |
| return query.replace(new RegExp(pattern, 'g'), () => value); |
| } | ||
|
|
||
| function getNamedParameterPattern(escapedName: string): string { | ||
| return `:${escapedName}\\b`; |
There was a problem hiding this comment.
getNamedParameterPattern will match :name even when it appears as the second colon in PostgreSQL-style casts (e.g. ::int contains :int). This can cause unintended substitutions in the query preview when a parameter name matches a cast type. Consider using a pattern that avoids :: (e.g. negative lookbehind (?<!:) before : or an equivalent non-lookbehind alternative).
| return `:${escapedName}\\b`; | |
| return `(?<!:):${escapedName}\\b`; |
| className?: string; | ||
| staticProperties?: boolean; | ||
| filterable?: boolean; | ||
| ordered?: boolean; |
There was a problem hiding this comment.
orderByName / sortByName and it should be enabled by default
| const parameterValues = new Map<string, string | null | undefined>(); | ||
|
|
||
| for (const parameter of queryParamsEvent.parameters) { | ||
| if (!parameter?.name || parameterValues.has(parameter.name)) { | ||
| continue; | ||
| } | ||
|
|
||
| parameterValues.set(parameter.name, parameter.value); | ||
| } | ||
|
|
||
| const paramNames = Array.from(parameterValues.keys()); |
There was a problem hiding this comment.
i think you can just use queryParamsEvent.parameters as is, without parameterValues map
There was a problem hiding this comment.
I can't, because the params array has duplicates like:
parameters: [{name: "b"}, {name: "col2"}, {name: "a"}, {name: "col2"}]
…te sorting logic
#4164) * dbeaver/pro#8298 fix: preserve query param order, fix incorrect replacement, add tooltip * dbeaver/pro#8298 refactor: rename ordered prop to sortByName and update sorting logic --------- Co-authored-by: Daria Marutkina <125263541+dariamarutkina@users.noreply.github.com> Co-authored-by: sergeyteleshev <iamsergeyteleshev@gmail.com>
…4196) * dbeaver/pro#8187 Adds right click for the cell menu * supports multiple pin/unpin columns * applies selection for the features: duplicate, delete, revert, set to NULL * fixes pin column for 1 column selected * renames to multiple pin/unpin columns * reverts sandwich * adds multiple add row action * dbeaver/pro#8601 TE + Git fixes (#4193) * dbeaver/pro#8601 Fix event handlers on RM server * dbeaver/pro#8601 Fix RM node icons * dbeaver/pro#8601 Fix RM project obtain * dbeaver/pro#8592 support new headers (#4190) * dbeaver/pro#7657 avoid blicking by debounce loading state (#4174) * dbeaver/pro#7657 avoid blicking by debounce loading state * dbeaver/pro#7657 use existing hooks * dbeaver/pro#7657 keep the old behaviour --------- Co-authored-by: Evgenia <139753579+EvgeniaBzzz@users.noreply.github.com> * dbeaver/pro#8298 fix: preserve query param order, fix incorrect repla… (#4164) * dbeaver/pro#8298 fix: preserve query param order, fix incorrect replacement, add tooltip * dbeaver/pro#8298 refactor: rename ordered prop to sortByName and update sorting logic --------- Co-authored-by: Daria Marutkina <125263541+dariamarutkina@users.noreply.github.com> Co-authored-by: sergeyteleshev <iamsergeyteleshev@gmail.com> * fix: on selected cell menu click opens menu * fix: duplicates on right click 1 row at time * removes selection on cell click + removes menu sandwich for selected multiple cells * value panel, upload, download features available only for 1 selected cell * adds translations for the multiple rows/columns actions --------- Co-authored-by: Serge Rider <serge@jkiss.org> Co-authored-by: Alexander Skoblikov <aleksandr.skoblikov@dbeaver.com> Co-authored-by: alex <48489896+devnaumov@users.noreply.github.com> Co-authored-by: Evgenia <139753579+EvgeniaBzzz@users.noreply.github.com> Co-authored-by: Sychev Andrey <44414066+SychevAndrey@users.noreply.github.com> Co-authored-by: Daria Marutkina <125263541+dariamarutkina@users.noreply.github.com> Co-authored-by: mr-anton-t <42037741+mr-anton-t@users.noreply.github.com>
…cement, add tooltip