Skip to content

fix(sql-lab): normalize tabViewId in QUERY_EDITOR_SET_SQL reducer#40983

Open
yousoph wants to merge 1 commit into
apache:masterfrom
preset-io:fix-query-history-restore-sql
Open

fix(sql-lab): normalize tabViewId in QUERY_EDITOR_SET_SQL reducer#40983
yousoph wants to merge 1 commit into
apache:masterfrom
preset-io:fix-query-history-restore-sql

Conversation

@yousoph

@yousoph yousoph commented Jun 11, 2026

Copy link
Copy Markdown
Member

Summary

  • Fixes sc-108187: "Overwrite editor" in Query History does nothing when SqllabBackendPersistence is enabled
  • Root cause: restoreSql passes query.sqlEditorId (= tabViewId) as the editor ID, but the reducer's alterUnsavedQueryEditorState routes it to alterInArr looking for an editor with id === tabViewId — no match because editors are indexed by client-side ID
  • Fix: add the same tabViewId → client-side id normalization that START_QUERY already uses

Test plan

  • New reducer unit test: QUERY_EDITOR_SET_SQL with tabViewId correctly updates unsavedQueryEditor.sql
  • Manual: SQL Lab → run a query → type new SQL (don't run) → click pencil icon in Query History → verify editor text replaced ✓ (requires backend persistence enabled)
  • Existing test: "should not fail while setting Sql" still passes (non-regression for non-persistence path)

🤖 Generated with Claude Code

When SqllabBackendPersistence is enabled, queries store sqlEditorId as
the backend tabViewId rather than the client-side id. The restoreSql
handler in QueryTable dispatches queryEditorSetSql with that tabViewId,
but the reducer's alterUnsavedQueryEditorState was routing it to the
queryEditors array path using the tabViewId as a lookup key — which
never matched because editors are indexed by client-side id. Result:
no state change, editor never updated.

Fix mirrors the existing START_QUERY normalization pattern: look up the
editor by tabViewId field first, fall back to using the id as-is.

Fixes sc-108187

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@bito-code-review

bito-code-review Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Bito Review Skipped - Source Branch Not Found

Bito didn’t review this change because the pull request is no longer valid. It may have been merged, or the source/target branch may no longer exist.

@dosubot dosubot Bot added the sqllab Namespace | Anything related to the SQL Lab label Jun 11, 2026
@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.30%. Comparing base (74845ea) to head (cd73544).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #40983      +/-   ##
==========================================
- Coverage   64.30%   64.30%   -0.01%     
==========================================
  Files        2657     2657              
  Lines      144060   144063       +3     
  Branches    33216    33218       +2     
==========================================
- Hits        92641    92638       -3     
- Misses      49797    49803       +6     
  Partials     1622     1622              
Flag Coverage Δ
javascript 68.24% <100.00%> (-0.01%) ⬇️

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

☔ View full report in Codecov by Harness.
📢 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.

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

Labels

size/M sqllab Namespace | Anything related to the SQL Lab

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant