Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
21 changes: 20 additions & 1 deletion components/webui/client/src/api/presto-search/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,28 @@ const submitQuery = async (
return axios.post<PrestoQueryJobSchema>("/api/presto-search/query", payload);
};


/**
* Sends post request to server to cancel presto query.
*
* @param payload
* @return
*/
const cancelQuery = async (
payload: PrestoQueryJobSchema
): Promise<AxiosResponse<null>> => {
console.log("Cancelling query:", JSON.stringify(payload));

return axios.post("/api/presto-search/cancel", payload);
};


export type {
PrestoQueryJobCreationSchema,
PrestoQueryJobSchema,
};

export {submitQuery};
export {
cancelQuery,
submitQuery,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.cancelButton {
width: 100%;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import {useCallback} from "react";

import {CloseOutlined} from "@ant-design/icons";
import {
Button,
Tooltip,
} from "antd";

import useSearchStore from "../../../../SearchState/index";
import {handlePrestoQueryCancel} from "../../presto-search-requests";
import styles from "./index.module.css";


/**
* Renders a button to cancel the SQL query.
*
* @return
*/
const CancelButton = () => {
const searchJobId = useSearchStore((state) => state.searchJobId);

const handleClick = useCallback(() => {
if (null === searchJobId) {
console.error("Cannot cancel query: searchJobId is not set.");

return;
}
handlePrestoQueryCancel({searchJobId});
}, [searchJobId]);
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Use getState() in event handler to avoid stale deps and re-creations.

For non-reactive callbacks, prefer reading the latest store value at call-time.

-    const handleClick = useCallback(() => {
-        if (null === searchJobId) {
+    const handleClick = useCallback(() => {
+        const {searchJobId} = useSearchStore.getState();
+        if (null === searchJobId) {
             console.error("Cannot cancel query: searchJobId is not set.");
             return;
         }
         handlePrestoQueryCancel({searchJobId});
-    }, [searchJobId]);
+    }, []);
🤖 Prompt for AI Agents
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/PrestoSearchButton/CancelButton/index.tsx
around lines 22 to 29, the handleClick callback depends on searchJobId from
props or state, which can cause stale closures and unnecessary re-creations. To
fix this, remove searchJobId from the dependency array and instead use
getState() inside the handleClick function to retrieve the latest searchJobId
value at call-time, ensuring the callback always uses the current state without
needing to re-subscribe.


return (
<Tooltip title={"Cancel query"}>
<Button
className={styles["cancelButton"] || ""}
color={"red"}
icon={<CloseOutlined/>}
size={"large"}
variant={"solid"}
onClick={handleClick}
>
Cancel
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify antd Button props color/variant; they’re non-standard in antd.

Ant Design typically uses type="primary" and danger flags rather than color/variant. If not using a wrapped Button, these props may be ignored or cause TS issues.

Run to confirm antd version and whether color/variant are project-wide conventions via a wrapper:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "antd version in package.json files:"
fd package.json | xargs -I{} jq -r '.dependencies.antd // .devDependencies.antd // empty' {} | nl -ba

echo
echo "Occurrences of Button with color/variant props in client code:"
rg --glob 'components/**/client/**' -n 'Button\s*\([\s\S]*?(color=|variant=)' -A 2 -B 2

Length of output: 336


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Looking for 'antd' in package.json files:"
rg '"antd"' -n -g 'package.json' || echo "No antd entry found in package.json"

echo
echo "Import of Button in CancelButton file:"
rg "import .*Button" -n components/webui/client/src/pages/SearchPage/SearchControls/Presto/PrestoSearchButton/CancelButton/index.tsx || echo "No Button import found"

echo
echo "Occurrences of <Button> with color prop:"
rg "<Button[^>]*color=" -n -g '*.tsx' || echo "None found in TSX files"

echo
echo "Occurrences of <Button> with variant prop:"
rg "<Button[^>]*variant=" -n -g '*.tsx' || echo "None found in TSX files"

Length of output: 897


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Import statements in CancelButton/index.tsx:"
rg "^import" -n components/webui/client/src/pages/SearchPage/SearchControls/Presto/PrestoSearchButton/CancelButton/index.tsx

Length of output: 523


Import Button and adjust props in CancelButton/index.tsx

It looks like the <Button> element isn’t even imported in this file, and the color/variant props you’ve used don’t match Ant Design’s API. Please:

• At the top of
components/webui/client/src/pages/SearchPage/SearchControls/Presto/PrestoSearchButton/CancelButton/index.tsx
add the correct import—for example:

import { Button } from "antd";

(Or import your project’s custom Button wrapper if one exists.)
• Replace non-standard props with AntD’s props:
– Use type="default" | "primary" | "dashed" | … instead of variant
– Use the danger boolean (or a custom styling class) instead of color="red"
• Confirm that size="large" and icon={<CloseOutlined/>} are supported by your Button component.

Fixing these will restore correct typing and styling for the cancel button.

🤖 Prompt for AI Agents
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/PrestoSearchButton/CancelButton/index.tsx
around lines 33 to 42, the Button component is not imported and the props used
(color and variant) do not align with Ant Design's Button API. Import Button
from "antd" at the top of the file, replace the color="red" prop with the danger
boolean prop, and replace variant="solid" with the appropriate type prop such as
type="default" or type="primary". Also verify that size="large" and
icon={<CloseOutlined/>} are valid props for the Button component. This will
ensure correct import, typing, and styling of the Cancel button.

</Tooltip>
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Optional: Show cancelling progress rather than instant UI flip.

Current flow flips to “Run” immediately (state set to DONE before the API returns). Consider keeping the Cancel button visible with a loading spinner until the cancel completes to avoid starting a new query while cancellation is in-flight.

If you adopt the refactor in handlePrestoQueryCancel (set state to DONE on success only), you can add:

-            <Button
+            <Button
                 className={styles["cancelButton"] || ""}
                 color={"red"}
                 icon={<CloseOutlined/>}
                 size={"large"}
                 variant={"solid"}
                 onClick={handleClick}
+                // Optionally: pass a prop like loading={searchUiState === SEARCH_UI_STATE.CANCEL_PENDING}
             >
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return (
<Tooltip title={"Cancel query"}>
<Button
className={styles["cancelButton"] || ""}
color={"red"}
icon={<CloseOutlined/>}
size={"large"}
variant={"solid"}
onClick={handleClick}
>
Cancel
</Button>
</Tooltip>
return (
<Tooltip title={"Cancel query"}>
<Button
className={styles["cancelButton"] || ""}
color={"red"}
icon={<CloseOutlined/>}
size={"large"}
variant={"solid"}
onClick={handleClick}
// Optionally: pass a prop like loading={searchUiState === SEARCH_UI_STATE.CANCEL_PENDING}
>
Cancel
</Button>
</Tooltip>
)
🤖 Prompt for AI Agents
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/PrestoSearchButton/CancelButton/index.tsx
around lines 31 to 43, the UI flips immediately to "Run" when cancelling a
query, which can confuse users and allow starting a new query before
cancellation completes. Modify the cancel button to show a loading spinner while
the cancellation is in progress by adding a loading state that is true during
the API call and false on completion. Ensure the button remains visible and
disabled with the spinner until the cancel request finishes, updating the state
to DONE only after successful cancellation.

);
};

export default CancelButton;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.runButton {
width: 100%;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import {
Tooltip,
} from "antd";

import useSearchStore from "../../../SearchState/index";
import {handlePrestoQuerySubmit} from "../presto-search-requests";
import useSearchStore from "../../../../SearchState/index";
import {SEARCH_UI_STATE} from "../../../../SearchState/typings";
import {handlePrestoQuerySubmit} from "../../presto-search-requests";
import styles from "./index.module.css";


/**
Expand All @@ -16,6 +18,7 @@ import {handlePrestoQuerySubmit} from "../presto-search-requests";
* @return
*/
const RunButton = () => {
const searchUiState = useSearchStore((state) => state.searchUiState);
const queryString = useSearchStore((state) => state.queryString);

const isQueryStringEmpty = "" === queryString;
Expand All @@ -30,11 +33,13 @@ const RunButton = () => {
return (
<Tooltip title={tooltipTitle}>
<Button
className={styles["runButton"] || ""}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Avoid redundant className fallback.

-                className={styles["runButton"] || ""}
+                className={styles["runButton"]}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
className={styles["runButton"] || ""}
className={styles["runButton"]}
🤖 Prompt for AI Agents
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/PrestoSearchButton/RunButton/index.tsx
at line 36, the className attribute uses a fallback to an empty string which is
redundant. Remove the fallback so that className is assigned directly as
styles["runButton"] without the || "" part.

color={"green"}
disabled={isQueryStringEmpty}
icon={<CaretRightOutlined/>}
size={"large"}
variant={"solid"}
disabled={isQueryStringEmpty ||
searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING}
onClick={handleClick}
>
Run
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.runButtonContainer {
width: 120px;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Optional: avoid hard-coded width; consider a token or min-width

A fixed 120px works but can be brittle on small screens. Consider a CSS variable (e.g., --search-button-width) or min-width/flex-basis to keep it responsive and consistent across themes.

🤖 Prompt for AI Agents
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/PrestoSearchButton/index.module.css
at lines 1 to 3, the runButtonContainer class uses a fixed width of 120px which
can cause layout issues on small screens. Replace the fixed width with a CSS
variable like --search-button-width or use min-width or flex-basis properties to
make the button width responsive and consistent across themes.

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import useSearchStore from "../../../SearchState/index";
import {SEARCH_UI_STATE} from "../../../SearchState/typings";
import CancelButton from "./CancelButton";
import styles from "./index.module.css";
import RunButton from "./RunButton";


/**
* Renders a button to submit or cancel the SQL query.
*
* @return
*/
const PrestoSearchButton = () => {
const searchUiState = useSearchStore((state) => state.searchUiState);

return (
<div className={styles["runButtonContainer"] || ""}>
{ (searchUiState === SEARCH_UI_STATE.QUERYING) ?
<CancelButton/> :
<RunButton/>}
</div>
);
};

export default PrestoSearchButton;
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import {
cancelQuery,
type PrestoQueryJobCreationSchema,
type PrestoQueryJobSchema,
submitQuery,
} from "../../../../api/presto-search";
import useSearchStore from "../../SearchState";
import {SEARCH_UI_STATE} from "../../SearchState/typings";


/**
Expand All @@ -10,9 +14,30 @@ import {
* @param payload
*/
const handlePrestoQuerySubmit = (payload: PrestoQueryJobCreationSchema) => {
const {updateSearchJobId, updateSearchUiState, searchUiState} = useSearchStore.getState();

// User should NOT be able to submit a new query while an existing query is in progress.
if (
searchUiState !== SEARCH_UI_STATE.DEFAULT &&
searchUiState !== SEARCH_UI_STATE.DONE
) {
console.error("Cannot submit query while existing query is in progress.");

return;
}

updateSearchUiState(SEARCH_UI_STATE.QUERY_ID_PENDING);

submitQuery(payload)
.then((result) => {
const {searchJobId} = result.data;

updateSearchJobId(searchJobId);
updateSearchUiState(SEARCH_UI_STATE.QUERYING);

// eslint-disable-next-line no-warning-comments
// TODO: Delete previous query results when the backend is ready

Comment on lines +38 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Keep the TODO; consider clearing stale results on cancel success too.

When the backend supports it, clear prior results on both new submissions and successful cancellations to avoid showing outdated data.

🤖 Prompt for AI Agents
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-search-requests.ts
around lines 38 to 40, keep the existing TODO comment about deleting previous
query results when the backend is ready, and extend the logic to also clear
stale results upon successful cancellation of a query. Update the code to clear
prior results both when a new query is submitted and when a cancellation
succeeds, ensuring outdated data is not displayed.

console.debug(
"Presto search job created - ",
"Search job ID:",
Expand All @@ -24,4 +49,29 @@ const handlePrestoQuerySubmit = (payload: PrestoQueryJobCreationSchema) => {
});
};

export {handlePrestoQuerySubmit};
/**
* Cancels an ongoing Presto search query on server.
*
* @param payload
*/
const handlePrestoQueryCancel = (payload: PrestoQueryJobSchema) => {
const {searchUiState, updateSearchUiState} = useSearchStore.getState();
if (searchUiState !== SEARCH_UI_STATE.QUERYING) {
console.error("Cannot cancel query if there is no ongoing query.");

return;
}

updateSearchUiState(SEARCH_UI_STATE.DONE);
cancelQuery(payload)
.then(() => {
console.debug("Query cancelled successfully");
})
.catch((err: unknown) => {
console.error("Failed to cancel query:", err);
});
};

export {
handlePrestoQueryCancel, handlePrestoQuerySubmit,
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from "../../../config";
import Dataset from "./Dataset";
import styles from "./index.module.css";
import RunButton from "./Presto/RunButton";
import PrestoSearchButton from "./Presto/PrestoSearchButton";
import SqlQueryInput from "./Presto/SqlQueryInput";
import QueryInput from "./QueryInput";
import SearchButton from "./SearchButton";
Expand Down Expand Up @@ -43,7 +43,7 @@ const SearchControls = () => {
(
<>
<SqlQueryInput/>
<RunButton/>
<PrestoSearchButton/>
</>
)}
</div>
Expand Down
34 changes: 33 additions & 1 deletion components/webui/server/src/routes/api/presto-search/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import {FastifyPluginAsyncTypebox} from "@fastify/type-provider-typebox";
import {
FastifyPluginAsyncTypebox,
Type,
} from "@fastify/type-provider-typebox";
import {StatusCodes} from "http-status-codes";

import {ErrorSchema} from "../../../schemas/error.js";
Expand Down Expand Up @@ -120,6 +123,35 @@ const plugin: FastifyPluginAsyncTypebox = async (fastify) => {
return {searchJobId};
}
);

fastify.post(
"/cancel",
{
schema: {
body: PrestoQueryJobSchema,
response: {
[StatusCodes.NO_CONTENT]: Type.Null(),
[StatusCodes.INTERNAL_SERVER_ERROR]: ErrorSchema,
},
tags: ["Presto Search"],
},
},
async (request, reply) => {
const {searchJobId} = request.body;
await new Promise<void>((resolve, reject) => {
Presto.client.kill(searchJobId, (error) => {
if (null !== error) {
reject(new Error("Failed to kill the Presto query job.", {cause: error}));
}
resolve();
});
});
request.log.info(searchJobId, "Presto search cancelled");
reply.code(StatusCodes.NO_CONTENT);

return null;
}
);
};

export default plugin;