Skip to content

fix(table-chart): support orderby adhoc columns with server-side pagination - #37521

Merged
SBIN2010 merged 4 commits into
apache:masterfrom
wuqicyber:fix/orderby-adhoc-column-server-side-pagination
Feb 20, 2026
Merged

fix(table-chart): support orderby adhoc columns with server-side pagination#37521
SBIN2010 merged 4 commits into
apache:masterfrom
wuqicyber:fix/orderby-adhoc-column-server-side-pagination

Conversation

@wuqicyber

@wuqicyber wuqicyber commented Jan 28, 2026

Copy link
Copy Markdown
Contributor

SUMMARY

When using server-side pagination in table charts, sorting by custom SQL expression columns (adhoc columns) fails with:
Error: Unknown column used in orderby: <column_label>

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:
image
After:
image

ROOT CAUSE

In superset/models/helpers.py, the orderby validation loop (lines ~2767-2810) handles:

  • Adhoc metrics via utils.is_adhoc_metric(col)
  • Named metrics via lookup in metrics_exprs_by_label
  • Physical columns via lookup in columns_by_name
    However, it does NOT handle adhoc columns (SQL expression columns with expressionType: "SQL"). When the orderby value is a string label that matches an adhoc column, the code fails to recognize it and raises QueryObjectValidationError.

TESTING INSTRUCTIONS

  1. Create a table chart with server-side pagination enabled
  2. Add a custom SQL expression column (e.g., {"expressionType": "SQL", "label": "custom_col", "sqlExpression": "column_a + 1"})
  3. Click the adhoc column header to sort
  4. Verify sorting works without error

ADDITIONAL INFORMATION

@bito-code-review

bito-code-review Bot commented Jan 28, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #737cc3

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 6d4d9d7..6d4d9d7
    • superset/models/helpers.py
    • tests/unit_tests/models/helpers_test.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot Bot added change:backend Requires changing the backend viz:charts:table Related to the Table chart labels Jan 28, 2026
Comment thread superset/models/helpers.py
@bito-code-review

bito-code-review Bot commented Jan 28, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #873428

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 6d4d9d7..1521ab1
    • superset/models/helpers.py
    • tests/unit_tests/models/helpers_test.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@codecov

codecov Bot commented Jan 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.58%. Comparing base (76d897e) to head (1521ab1).
⚠️ Report is 3911 commits behind head on master.

Files with missing lines Patch % Lines
superset/models/helpers.py 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #37521      +/-   ##
==========================================
+ Coverage   60.48%   66.58%   +6.09%     
==========================================
  Files        1931      643    -1288     
  Lines       76236    49053   -27183     
  Branches     8568     5502    -3066     
==========================================
- Hits        46114    32661   -13453     
+ Misses      28017    15097   -12920     
+ Partials     2105     1295     -810     
Flag Coverage Δ
hive 41.91% <0.00%> (-7.24%) ⬇️
javascript ?
mysql 64.64% <0.00%> (?)
postgres 64.72% <0.00%> (?)
presto 41.93% <0.00%> (-11.87%) ⬇️
python 66.55% <0.00%> (+3.04%) ⬆️
sqlite 64.41% <0.00%> (?)
unit 100.00% <ø> (+42.36%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rusackas
rusackas requested a review from SBIN2010 February 19, 2026 21:19
@SBIN2010 SBIN2010 added the 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR label Feb 20, 2026
@github-actions github-actions Bot added 🎪 1521ab1 🚦 building 🎪 ⌛ 48h Environment expires after 48 hours (default) and removed 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR labels Feb 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🎪 Showtime is building environment on GHA for 1521ab1

@github-actions

Copy link
Copy Markdown
Contributor

🎪 Showtime deployed environment on GHA for 1521ab1

Environment: http://35.89.228.163:8080 (admin/admin)
Lifetime: 48h auto-cleanup
Updates: New commits create fresh environments automatically

@SBIN2010 SBIN2010 left a comment

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.

LGTM! Thank you for your contribution!
During manual testing, everything works correctly.

@SBIN2010
SBIN2010 merged commit 6f34ba7 into apache:master Feb 20, 2026
78 checks passed
@dosubot

dosubot Bot commented Feb 20, 2026

Copy link
Copy Markdown

Related Documentation

Checked 0 published document(s) in 2 knowledge base(s). No updates required.

How did I do? Any feedback?  Join Discord

sadpandajoe pushed a commit that referenced this pull request Feb 26, 2026
@sadpandajoe sadpandajoe added the v6.0 Label added by the release manager to track PRs to be included in the 6.0 branch label Feb 28, 2026
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
@mistercrunch mistercrunch removed the 🎪 ⌛ 48h Environment expires after 48 hours (default) label Jun 28, 2026
prathamesh04 added a commit to prathamesh04/superset that referenced this pull request Jul 19, 2026
…e chart

When a column label is renamed in the Table chart (e.g. 'CustomerID' to
'Id'), the search filter sends the display label ('Id') but the backend
columns_by_name dict is keyed by physical column names. This causes the
filter to be silently rejected with 'not_in_datasource'.

Add a fallback that resolves the filter column against adhoc_columns_by_label
(the same pattern used by PR apache#37521 for ORDER BY). Also update the
rejected/applied filter columns computation to recognize adhoc column labels.

Fixes apache#38339
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:backend Requires changing the backend size/M v6.0 Label added by the release manager to track PRs to be included in the 6.0 branch viz:charts:table Related to the Table chart

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unknown column used in orderby for SQL expression columns (server-side pagination)

4 participants