You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mistercrunch (via GitHub)" <gi...@apache.org> on 2026/04/08 20:59:04 UTC
[PR] perf(explore): replace TableView with GridTable in SamplesPane for virtualized rendering [superset]
mistercrunch opened a new pull request, #39212:
URL: https://github.com/apache/superset/pull/39212
### SUMMARY
The Samples tab in Explore freezes the browser for ~30 seconds on datasets with many columns. The API request completes quickly — this is a pure frontend rendering performance bug.
**Root cause:** `SamplesPane` uses `TableView` (react-table) which renders **all columns in the DOM** without any virtualization. For datasets with hundreds of columns, this creates a massive DOM tree that blocks the main thread during rendering and remains laggy after.
**Fix:** Replace `TableView` with the existing `GridTable` component (ag-grid) which provides both **row and column virtualization** out of the box. AG Grid only renders the columns/rows visible in the viewport, eliminating the freeze entirely.
This follows the same pattern already used by `FilterableTable` in SQL Lab, which also uses `GridTable` for the same performance benefits.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before:** ~30s browser freeze when opening Samples tab on wide datasets
**After:** Instant rendering with smooth scrolling through all columns
### TESTING INSTRUCTIONS
1. Open Explore on a dataset with many columns (100+)
2. Click the "Samples" tab in the data panel at the bottom
3. Verify the table renders immediately without freezing
4. Verify scrolling through columns and rows is smooth
5. Verify the search/filter input still works
6. Verify row count display is correct
### ADDITIONAL INFORMATION
- [x] Has associated issue: sc-103413
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(explore): replace TableView with GridTable in Samples and Results panes for virtualized rendering [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #39212:
URL: https://github.com/apache/superset/pull/39212#issuecomment-4210155601
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | caeb6a6b7ce7a75ecc78bc2002a11cf68ac0355d |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/69d6d8852b8bea0008be1739 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-39212--superset-docs-preview.netlify.app](https://deploy-preview-39212--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-39212--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): replace TableView with virtualized GridTable, add row limit controls, restore sample filters [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch merged PR #39212:
URL: https://github.com/apache/superset/pull/39212
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): replace TableView with virtualized GridTable, add row limit controls, restore sample filters [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39212:
URL: https://github.com/apache/superset/pull/39212#issuecomment-4221923516
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/24230655783) for [860f8cb](https://github.com/apache/superset/commit/860f8cb)
• **Environment:** http://35.90.100.94:8080 (admin/admin)
• **Lifetime:** 48h auto-cleanup
• **Updates:** New commits create fresh environments automatically
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): replace TableView with virtualized GridTable, add row limit controls, restore sample filters [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #39212:
URL: https://github.com/apache/superset/pull/39212#discussion_r3061714072
##########
superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx:
##########
@@ -563,16 +550,7 @@ describe('Table view with pagination', () => {
expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument();
});
- // Find sortable column header
- const sortableHeaders = screen.getAllByTestId('sort-header');
- expect(sortableHeaders.length).toBeGreaterThan(0);
-
- // Click to sort
- userEvent.click(sortableHeaders[0]);
-
// Table should still be rendered without crashes
- await waitFor(() => {
- expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument();
- });
+ expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument();
});
});
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Weakened Test Assertion</b></div>
<div id="fix">
The test 'should render grid in table view' no longer verifies that column headers are displayed, weakening test coverage. The table should show column headers like 'state' even with empty data. Consider restoring the check for screen.getByText('state') or similar to validate correct rendering.
</div>
</div>
<small><i>Code Review Run #0b88e8</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx:
##########
@@ -144,11 +148,6 @@ describe('ResultsPaneOnDashboard', () => {
expect(queryByText('2 rows')).toBeVisible();
expect(queryByText('Action')).toBeVisible();
expect(queryByText('Horror')).toBeVisible();
-
- userEvent.type(getByPlaceholderText('Search'), 'hor');
- await waitForElementToBeRemoved(() => queryByText('Action'));
- expect(queryByText('Horror')).toBeVisible();
- expect(queryByText('Action')).not.toBeInTheDocument();
});
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing search behavior verification</b></div>
<div id="fix">
The test 'force query, render' no longer verifies the search/filtering behavior that the component supports via FilterInput and useKeywordFilter. While the component still includes search functionality, this test only checks rendering without asserting the core filtering logic. Consider adding search interaction assertions or ensuring this behavior is tested elsewhere.
</div>
</div>
<small><i>Code Review Run #0b88e8</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(explore): replace TableView with GridTable in SamplesPane for virtualized rendering [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39212:
URL: https://github.com/apache/superset/pull/39212#issuecomment-4209613239
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): replace TableView with virtualized GridTable, add row limit controls, restore sample filters [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #39212:
URL: https://github.com/apache/superset/pull/39212#issuecomment-4219520165
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): replace TableView with virtualized GridTable, add row limit controls, restore sample filters [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39212:
URL: https://github.com/apache/superset/pull/39212#issuecomment-4221832404
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/24230655783) for [860f8cb](https://github.com/apache/superset/commit/860f8cb)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(explore): replace TableView with GridTable in SamplesPane for virtualized rendering [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #39212:
URL: https://github.com/apache/superset/pull/39212#discussion_r3054223885
##########
superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx:
##########
@@ -94,18 +104,76 @@ export const SamplesPane = ({
}
}, [datasource, isRequest, queryForce]);
- // this is to preserve the order of the columns, even if there are integer values,
- // while also only grabbing the first column's keys
- const columns = useTableColumns(
- colnames,
- coltypes,
- data,
- datasourceId,
- isVisible,
- {}, // moreConfig
- true, // allowHTML
+ useEffect(() => {
+ const container = gridContainerRef.current;
+ if (!container) return undefined;
+ const observer = new ResizeObserver(entries => {
+ const entry = entries[0];
+ if (entry) {
+ setGridHeight(entry.contentRect.height);
+ }
+ });
+ observer.observe(container);
+ return () => observer.disconnect();
+ }, []);
+
+ const columns = useMemo(
+ () =>
+ colnames && data?.length
+ ? colnames
+ .filter((column: string) =>
+ Object.keys(data[0]).includes(column),
+ )
+ .map((key, index) => {
+ const colType = coltypes?.[index];
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incorrect Column Type Mapping</b></div>
<div id="fix">
The colType lookup uses the index from the filtered colnames array, but coltypes is parallel to the original colnames. This causes incorrect type detection for temporal columns, leading to wrong formatting.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
const originalIndex = colnames.indexOf(key);
const colType = coltypes?.[originalIndex];
````
</div>
</details>
</div>
<small><i>Code Review Run #82fdde</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(explore): replace TableView with GridTable in SamplesPane for virtualized rendering [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #39212:
URL: https://github.com/apache/superset/pull/39212#issuecomment-4209849576
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/39212?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `67.44186%` with `14 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 64.46%. Comparing base ([`d63308c`](https://app.codecov.io/gh/apache/superset/commit/d63308ca372c3d87c9ddf48e896c61623788f907?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`6c71800`](https://app.codecov.io/gh/apache/superset/commit/6c718004365f443d1f96ba4b4e25a1cdaadbcb11?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/39212?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...mponents/DataTablesPane/components/SamplesPane.tsx](https://app.codecov.io/gh/apache/superset/pull/39212?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fexplore%2Fcomponents%2FDataTablesPane%2Fcomponents%2FSamplesPane.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhVGFibGVzUGFuZS9jb21wb25lbnRzL1NhbXBsZXNQYW5lLnRzeA==) | 67.44% | [14 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/39212?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #39212 +/- ##
=======================================
Coverage 64.46% 64.46%
=======================================
Files 2541 2541
Lines 131775 131816 +41
Branches 30569 30584 +15
=======================================
+ Hits 84946 84976 +30
- Misses 45363 45374 +11
Partials 1466 1466
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/39212/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/39212/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.08% <67.44%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/39212?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org