Skip to content

bug(sql-workspace): Save Changes stays enabled in already-open tabs after connection becomes read-only #1007

Description

@ZhuchkaTriplesix

Summary

PostgresSqlWorkspace, MysqlSqlWorkspace and SqliteSqlWorkspace each cache their built per-tab pane widget in _paneCache (keyed by SqlQueryTabSession.id) to skip rebuilding untouched tabs on tab-switch. The cached pane's onApplyChanges callback closes over widget.isReadOnly at build time:

onApplyChanges: widget.isReadOnly ||
        !sqlResultGridSaveEnabled(...)
    ? null
    : () => _applyStagedChanges(session),

None of the three workspaces' didUpdateWidget invalidates _paneCache when widget.isReadOnly changes — each only reacts to it by dropping the connection lease:

void didUpdateWidget(covariant PostgresSqlWorkspace oldWidget) {
  super.didUpdateWidget(oldWidget);
  if (oldWidget.connectionRow.id != widget.connectionRow.id) {
    _lastAppliedSqlContextToken = -1;
  }
  if (oldWidget.isReadOnly != widget.isReadOnly) {
    _dropLease();
  }
  ...
}

(same shape in mysql_sql_workspace.dart:213 and sqlite_sql_workspace.dart:221, both only calling _lease?.release()).

Impact

If a connection's read-only lock is turned on while a query tab with a dirty staging buffer is already open and cached, that tab's "Save Changes" button stays enabled and wired to _applyStagedChanges, which itself does not re-check widget.isReadOnly before running the mutation SQL. A newly-opened tab would correctly show the button disabled (since it's built fresh), but the already-open one would not update until something else invalidates its cache entry (e.g. running a new query in it).

Suggested fix

  • Call _invalidateAllPanes() from each workspace's didUpdateWidget whenever oldWidget.isReadOnly != widget.isReadOnly.
  • Defense in depth: have _applyStagedChanges bail out early if widget.isReadOnly is true, so a stale cached pane can't trigger a write even if the cache invalidation is missed somewhere.

Found during a code-quality review of the dev branch, not yet confirmed against a live "toggle read-only while a tab is open" user flow — flagging as the pane-cache/didUpdateWidget gap is clear from the code regardless of exactly how read-only gets toggled at runtime.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcoreCore library logic and servicesdata-gridInteractive data grid, cell editor, filtering, groupings

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions