Skip to content

Fix query tool auto execution after session restore #10055

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.

Sign up for GitHub

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

Open
dev-hari-prasad wants to merge 7 commits into
base: master
Choose a base branch
from
+37 −11
Open

Fix query tool auto execution after session restore #10055

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0d9c38d
Fix query auto execution on session restore
dev-hari-prasad Jun 9, 2026
29433f2
Clean up QueryToolComponent state and URL handling
dev-hari-prasad Jun 10, 2026
aefee3f
Update QueryToolComponent.jsx
dev-hari-prasad Jun 10, 2026
b8a2a4e
Refactoring: Clean up state initialization and URL checks
dev-hari-prasad Jun 10, 2026
a0a4ee8
Fix restore execution and continue smooth Query Tool promotion
dev-hari-prasad Jun 18, 2026
93bab32
Merge branch 'master' into bug/prevent-query-auto-execution
dev-hari-prasad Jun 18, 2026
798ecef
Fix toolUrl persistence and clean up Query Tool restore logic
dev-hari-prasad Sep 5, 2026
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
48 changes: 37 additions & 11 deletions web/pgadmin/tools/sqleditor/static/js/components/QueryToolComponent.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ const FIXED_PREF = {
export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedNodeInfo, qtPanelDocker, qtPanelId, eventBusObj}) {
const containerRef = React.useRef(null);
const preferencesStore = usePreferences();
const isQueryTool = params.is_query_tool === true || params.is_query_tool === 'true';
const [qtState, setQtState] = useState({
preferences: {
browser: preferencesStore.getPreferencesForModule('browser'),
Expand All @@ -138,7 +139,8 @@ export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedN
params: {
...params,
title: _.unescape(params.title),
is_query_tool: params.is_query_tool == 'true',
is_query_tool: isQueryTool,
restore: params.restore === true || params.restore === 'true',
node_name: retrieveNodeName(selectedNodeInfo),
dbname: _.unescape(params.database_name) || getDatabaseLabel(selectedNodeInfo),
server_cursor: preferencesStore.getPreferencesForModule('sqleditor').server_cursor === true,
Expand All @@ -154,7 +156,7 @@ export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedN
bgcolor: params.bgcolor,
conn_title: getTitle(
pgAdmin, null, selectedNodeInfo, true, _.unescape(params.server_name), _.unescape(params.database_name) || getDatabaseLabel(selectedNodeInfo),
_.unescape(params.role) || _.unescape(params.user), params.is_query_tool == 'true'),
_.unescape(params.role) || _.unescape(params.user), isQueryTool),
server_name: _.unescape(params.server_name),
database_name: _.unescape(params.database_name) || getDatabaseLabel(selectedNodeInfo),
is_selected: true,
Expand Down Expand Up @@ -295,14 +297,14 @@ export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedN
eventBus.current.fireEvent(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, err);
setQtStatePartial({ editor_disabled: true });
});
} else if (qtState.params.sql_id && qtState.params.restore != 'true') {
} else if (qtState.params.sql_id && qtState.params.restore !== true) {
let sqlValue = localStorage.getItem(qtState.params.sql_id);
localStorage.removeItem(qtState.params.sql_id);
if (sqlValue) {
eventBus.current.fireEvent(QUERY_TOOL_EVENTS.EDITOR_SET_SQL, sqlValue);
}
setQtStatePartial({ editor_disabled: false });
} else if (qtState.params.restore == 'true') {
} else if (qtState.params.restore === true) {

@mzabuawala mzabuawala Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes looks good.
I observed that restoreToolContent() is async but is called without await here, can you fix that also?

restoreToolContent();
} else {
setQtStatePartial({ editor_disabled: false });
Expand Down Expand Up @@ -347,7 +349,14 @@ export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedN
if(toolContent?.fileName)eventBus.current.fireEvent(QUERY_TOOL_EVENTS.LOAD_FILE_DONE, toolContent.fileName, true);
}
}
setQtStatePartial({ editor_disabled: false });
setQtStatePartial(prev => ({
...prev,
editor_disabled: false,
params: {
...prev.params,
restore: false
}
}));
};

const initializeQueryTool = (password, explainObject=null, macroSQL='', executeCursor=false, executeServerCursor=false, reexecute=false)=>{
Expand Down Expand Up @@ -938,15 +947,31 @@ export default function QueryToolComponent({params, pgWindow, pgAdmin, selectedN
eol: qtState.eol,
connection_list: qtState.connection_list,
current_file: qtState.current_file,
toggleQueryTool: () => setQtStatePartial((prev)=>{
return {
toggleQueryTool: () => {
let panel = qtPanelDocker?.find(qtPanelId);

@dpage dpage Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two smaller points on this block.

Firstly, the docker lookup, the panel.metaData mutation and the saveLayout() call all happen inside the setQtStatePartial((prev)=>{...}) updater. Updater functions are required to be pure, and React is free to invoke them more than once; there's no StrictMode in this codebase today so it won't misbehave right now, but it's a trap for whoever turns it on. Moving the whole block above the setQtStatePartial call costs nothing.

Secondly, if qtPanelDocker?.find(qtPanelId) returns undefined, say because the panel lives in the other workspace's docker, the promotion silently isn't persisted and nothing is logged; only a URL parse failure warns. A console.warn on the miss would save someone a long afternoon.

if (!panel) {
console.warn('toggleQueryTool: panel not found for qtPanelId', qtPanelId);
} else if (panel.metaData?.toolUrl) {
try {
const toolUrl = panel.metaData.toolUrl;
const url = new URL(toolUrl, window.location.origin);
url.searchParams.set('is_query_tool', 'true');
panel.metaData = Object.assign({}, panel.metaData, {
toolUrl: url.pathname + url.search,
});
qtPanelDocker?.saveLayout();
} catch (e) {
console.warn(`Failed to update is_query_tool parameter for toolUrl "${panel?.metaData?.toolUrl}" using origin "${window.location.origin}":`, e);
}
}
setQtStatePartial((prev) => ({
...prev,
params: {
...prev.params,
is_query_tool: true
}
};
}),
is_query_tool: true,
},
}));
},
updateTitle: (title) => {
setPanelTitle(qtPanelDocker, qtPanelId, title, qtState, isDirtyRef.current);
setQtStatePartial((prev) => {
Expand Down Expand Up @@ -1032,6 +1057,7 @@ QueryToolComponent.propTypes = {
bgcolor: PropTypes.string,
fgcolor: PropTypes.string,
is_query_tool: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]).isRequired,
restore: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]),
server_cursor: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]),
user: PropTypes.string,
role: PropTypes.string,
Expand Down
Loading