Skip to content

feat(webui): Display total result count rather than UI table row count for Presto queries. #1236

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

Merged
hoophalab merged 6 commits into from
Aug 25, 2025
Merged

feat(webui): Display total result count rather than UI table row count for Presto queries. #1236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4ce6adc
latest
Aug 18, 2025
6b9cb77
latest
Aug 19, 2025
5afa51a
Update components/webui/client/src/pages/SearchPage/SearchQueryStatus…
davemarco Aug 22, 2025
330ef3e
Merge branch 'main' into countUpdate
davemarco Aug 22, 2025
a7b9f16
Merge branch 'main' into countUpdate
junhaoliao Aug 25, 2025
8e8ef0f
Merge branch 'main' into countUpdate
hoophalab Aug 25, 2025
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
11 changes: 7 additions & 4 deletions components/webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,19 @@ type TextTypes = GetProps<typeof Text>["type"];
* @return
*/
const Results = () => {
const numSearchResultsMetadata = useSearchStore((state) => state.numSearchResultsMetadata);
const numSearchResultsTimeline = useSearchStore((state) => state.numSearchResultsTimeline);
const numSearchResultsTable = useSearchStore((state) => state.numSearchResultsTable);
const searchUiState = useSearchStore((state) => state.searchUiState);

// Number of results is the maximum of the number of results in the timeline and table. The
// timeline may have more results since the table results are capped. Having two sources may
// provide more timely updates to the user.
// Number of results is the maximum from timeline, table, and server metadata sources.
// Multiple sources provide more timely updates. Source behavior differs by query engine:
// - clp/clp-s: table and server metadata counts are capped
// - presto: table count is capped, no timeline count available
const numResults = useMemo(
() => Math.max(numSearchResultsTimeline, numSearchResultsTable),
() => Math.max(numSearchResultsMetadata, numSearchResultsTimeline, numSearchResultsTable),
[
numSearchResultsMetadata,
numSearchResultsTimeline,
numSearchResultsTable,
]
Expand Down
3 changes: 2 additions & 1 deletion components/webui/client/src/pages/SearchPage/SearchQueryStatus/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ const SearchQueryStatus = () => {
{(searchUiState === SEARCH_UI_STATE.QUERYING ||
searchUiState === SEARCH_UI_STATE.DONE) && (
<Text type={"secondary"}>
Search job #
Search job
{" "}
{searchJobId}
{" "}
found
Expand Down
10 changes: 10 additions & 0 deletions components/webui/client/src/pages/SearchPage/SearchState/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {SEARCH_UI_STATE} from "./typings";
const SEARCH_STATE_DEFAULT = Object.freeze({
aggregationJobId: null,
cachedDataset: null,
numSearchResultsMetadata: 0,

@coderabbitai coderabbitai Bot Aug 19, 2025

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.

💡 Verification agent

🧩 Analysis chain

Consider resetting metadata count at query start to avoid stale max() values

Initialising to 0 is fine, but if a new query begins and the first metadata payload lacks numTotalResults, the previous query’s (possibly large) metadata count can linger and dominate the max(metadata, timeline, table) in Results.tsx. Ensure this field is explicitly reset to 0 on new query start or when a new results set (_id) is detected.

Run this to find where the metadata count is reset on query start and whether a 0 reset exists:

🏁 Script executed:

#!/bin/bash
set -euo pipefail
# All call sites of updateNumSearchResultsMetadata
rg -nP -C2 --type=ts --type=tsx '\bupdateNumSearchResultsMetadata\s*\('

# Explicit resets to 0
rg -nP -C3 --type=ts --type=tsx 'updateNumSearchResultsMetadata\s*\(\s*0\s*\)'

Length of output: 121

I’ll retry the grep without relying on --type=tsx support:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find all call sites of updateNumSearchResultsMetadata in .ts and .tsx files
rg -nP -C2 '\bupdateNumSearchResultsMetadata\s*\(' -g '*.ts' -g '*.tsx'

# Find any explicit resets to 0
rg -nP -C3 'updateNumSearchResultsMetadata\s*\(\s*0\s*\)' -g '*.ts' -g '*.tsx'

Length of output: 783

Ensure metadata count is reset on every new query

The handler updateNumSearchResultsMetadata is only invoked when resultsMetadata.numTotalResults is defined—there’s no explicit reset to 0 when a query starts or when a new results set (_id) arrives. As a result, if the first payload of a new query omits numTotalResults, the previous (larger) value will persist and skew any max() calculations in Results.tsx.

Please add an explicit reset to 0 in useUpdateStateWithMetadata.ts, for example:

• File: components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts

useEffect(() => {
  // Reset metadata count on each new query (or when _id changes)
  updateNumSearchResultsMetadata(0);

  if (typeof resultsMetadata.numTotalResults !== "undefined") {
    updateNumSearchResultsMetadata(resultsMetadata.numTotalResults);
  }
}, [resultsMetadata._id /* or the relevant query identifier */]);

This guarantees stale values are cleared before processing each new result set.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts
around where resultsMetadata is handled (related to lines near the current
numSearchResultsMetadata usage), the metadata counter is never explicitly reset
when a new query or results set (_id) begins, allowing stale larger values to
persist; modify the effect that reads resultsMetadata so it first calls
updateNumSearchResultsMetadata(0) to clear the count, then only sets the real
value if resultsMetadata.numTotalResults is defined, and ensure the effect
depends on the query identifier (e.g., resultsMetadata._id) so the reset runs
for each new query.

numSearchResultsTable: 0,
numSearchResultsTimeline: 0,
queryIsCaseSensitive: false,
Expand All @@ -43,6 +44,11 @@ interface SearchState {
*/
cachedDataset: Nullable<string>;

/**
* The number of search results from server metadata.
*/
numSearchResultsMetadata: number;

/**
* The number of search table results.
*/
Expand Down Expand Up @@ -97,6 +103,7 @@ interface SearchState {

updateAggregationJobId: (id: string | null) => void;
updateCachedDataset: (dataset: string) => void;
updateNumSearchResultsMetadata: (num: number) => void;
updateNumSearchResultsTable: (num: number) => void;
updateNumSearchResultsTimeline: (num: number) => void;
updateQueryIsCaseSensitive: (newValue: boolean) => void;
Expand All @@ -117,6 +124,9 @@ const useSearchStore = create<SearchState>((set) => ({
updateCachedDataset: (dataset) => {
set({cachedDataset: dataset});
},
updateNumSearchResultsMetadata: (num) => {
set({numSearchResultsMetadata: num});
},
updateNumSearchResultsTable: (num) => {
set({numSearchResultsTable: num});
},
Expand Down
20 changes: 14 additions & 6 deletions components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,25 @@ import {useResultsMetadata} from "./useResultsMetadata";
const PRESTO_CANCEL_ERROR_NAME = "USER_CANCELED";

/**
* Custom hook to update the UI state to `DONE` when the results metadata signal indicates
* that the query is complete, or `FAILED` if the query fails. If there is an error, it will
* also display a notification with the error message.
* Custom hook to update the client state based on results metadata from the server.
* - Sets the UI state to `DONE` when the results metadata signal indicates that the query is
* complete, or `FAILED` if the query fails. If there is an error, it will display a notification
* with the error message.
* - Updates the number of search results from the metadata.
*/
const useUiUpdateOnDoneSignal = () => {
const {updateSearchUiState} = useSearchStore();
const useUpdateStateWithMetadata = () => {
const {updateSearchUiState, updateNumSearchResultsMetadata} = useSearchStore();
const resultsMetadata = useResultsMetadata();

Comment on lines +26 to +29

@coderabbitai coderabbitai Bot Aug 19, 2025

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.

🛠️ Refactor suggestion

Select store actions with selectors to avoid re-rendering on every state change

Destructuring the whole store subscribes to all state changes, causing SearchPage to re-render frequently. Select only the actions needed.

Apply this diff:

-const useUpdateStateWithMetadata = () => {
-    const {updateSearchUiState, updateNumSearchResultsMetadata} = useSearchStore();
+const useUpdateStateWithMetadata = () => {
+    const updateSearchUiState = useSearchStore((s) => s.updateSearchUiState);
+    const updateNumSearchResultsMetadata = useSearchStore((s) => s.updateNumSearchResultsMetadata);
     const resultsMetadata = useResultsMetadata();
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts
around lines 21 to 24, the current destructuring of useSearchStore subscribes to
the whole store and causes unnecessary re-renders; instead, select only the
specific actions you need from the store (e.g. use the selector form of
useSearchStore to pick updateSearchUiState and updateNumSearchResultsMetadata,
or call useSearchStore twice to read each action separately) so the component
only subscribes to those actions; ensure the selected action references are
stable (wrap callbacks where needed) and remove the current broad destructuring.

useEffect(() => {
if (null === resultsMetadata) {
return;
}

if ("undefined" !== typeof resultsMetadata.numTotalResults) {
updateNumSearchResultsMetadata(resultsMetadata.numTotalResults);
}

Comment thread
hoophalab marked this conversation as resolved.
switch (resultsMetadata.lastSignal) {
case SEARCH_SIGNAL.RESP_DONE:
case PRESTO_SEARCH_SIGNAL.FINISHED:
Expand Down Expand Up @@ -59,7 +66,8 @@ const useUiUpdateOnDoneSignal = () => {
}, [
resultsMetadata,
updateSearchUiState,
updateNumSearchResultsMetadata,
]);
};

export {useUiUpdateOnDoneSignal};
export {useUpdateStateWithMetadata};
4 changes: 2 additions & 2 deletions components/webui/client/src/pages/SearchPage/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import SearchControls from "./SearchControls";
import SearchQueryStatus from "./SearchQueryStatus";
import SearchResultsTable from "./SearchResults/SearchResultsTable";
import SearchResultsTimeline from "./SearchResults/SearchResultsTimeline";
import {useUiUpdateOnDoneSignal} from "./SearchState/useUpdateStateWithMetadata";
import {useUpdateStateWithMetadata} from "./SearchState/useUpdateStateWithMetadata";


/**
Expand All @@ -17,7 +17,7 @@ import {useUiUpdateOnDoneSignal} from "./SearchState/useUpdateStateWithMetadata"
* @return
*/
const SearchPage = () => {
useUiUpdateOnDoneSignal();
useUpdateStateWithMetadata();

return (
<>
Expand Down
Loading