Skip to content

fix(plugin-redis): the Redis, Explain and connection defects found alongside #3036 - #3039

Merged
datlechin merged 30 commits into
mainfrom
fix/redis-3036-followups
Sep 21, 2026
Merged

datlechin merged 30 commits into
mainfrom
fix/redis-3036-followups

Conversation

@datlechin

Copy link
Copy Markdown
Member

Follow-up to #3038. Refs #3036.

#3038 fixed the reported bug and the defects found alongside it. Its report listed more that were real but out of its scope; this PR fixes all of them, plus what turned up while fixing those.

Redis on the Mac

Commit What was wrong
fix(connections): open the Redis database a connection names… The Mac resolved one Redis database index four different ways, and none read the db4 spelling iOS and the plugin accept. A connection synced from iPhone with database db4 opened db4 there and db0 on the Mac. The driver config, the post-connect step, the toolbar, the form and the URL export now share one resolution: the Database Index field, then the value saved before that field existed, then the database name. A Cluster connection always opens on db0, even when a Database Index field left over from a Standalone setup still holds another value. Over an SSH tunnel the tunnel forces Standalone, so the index is resolved before that happens. The driver opens that index at connect. A second window for a connection shows the database the live session is on, not the configured one.
fix(connections): refuse a Redis URL whose path is not a database index redis://host/-1 was accepted and failed at connect. /1.5 and /abc were dropped without a word, so db0 opened. redis+ssh://…/3 never read the index at all. All of these now fail with the existing "is not a Redis database index" message.
fix(plugin-redis): keep every argument of a Redis command the parser models Commands the parser models dropped the arguments it had no field for: SCAN … TYPE hash listed every type, CONFIG GET a b returned one row, and PING hello answered 1. GET a b ran as GET a. Valid XGROUP HELP, OBJECT HELP and CONFIG RESETSTAT were refused. A command with arguments its result view does not model is now sent exactly as typed.
test(plugin-redis): test the real Redis command parser…, refactor(plugin-redis): render Redis replies from a type the tests compile Two suites were testing hand-copied versions of the parser and the reply renderer, and the copies had already drifted from the driver. The suites now test the real code.
fix(sidebar): load the Redis key tree from its own database The key tree sent KEYTREE with no database, so after a SELECT typed in the editor it listed, and opened keys from, the editor's database. It now sends KEYTREE DB n, and a key opened from the tree reads through DB n <command>, so the editor's session stays where it was.
fix(plugin-redis): read the server's subcommand routing tips… The COMMAND reply parser put the container name in front of subcommand names that already carried it (config|config|set). Every subcommand tip was therefore lost, and FUNCTION LOAD, ACL SETUSER and CONFIG REWRITE reached one primary out of three.
fix(plugin-redis): report a split Redis Cluster write that only some shards applied A split DEL or a fanned-out FLUSHDB that one shard refused reported only the refusal. Redis cannot undo the part that already ran, so the error now names the keys, or for FLUSHDB the nodes, that were already changed. That includes fan-outs with no write flag that still change every node, such as CONFIG SET and ACL SETUSER.
feat(plugin-redis): browse numbered databases on a Valkey 9 cluster Valkey 9 clusters with cluster-databases above 1 listed only db0, and SELECT 3 was refused locally. The count is learned once per primary at connect. A cluster cannot wrap a grid save in MULTI, so there each write carries its database as DB <n> <command> and a failed save cannot strand the session. The editor accepts the same syntax, and the Safe Mode classifier reads past it, so DB 0 FLUSHDB still asks for confirmation. A DB n command on a primary holding the user's open MULTI block is refused rather than queued into the block on the home database.
fix(editor): classify a quoted Redis command as the command the driver runs Safe Mode and the MCP destructive-statement check read Redis statements split on spaces, while the driver reads them the way redis-cli does. "FLUSHALL", 'FLUSHDB' ASYNC and "\x46LUSHALL" all passed as ordinary writes. The classifier now uses the driver's own codec, which is compiled into the app along with RedisDatabaseIndex.swift, and the key tree quotes keys with it too.
fix(plugin-redis): match a Redis namespace literally when counting its keys buildCountQuery put the namespace in quotes without escaping it. No driver path passes a non-empty namespace today, so this could not be reached from the app.

Explain

Commit What was wrong
fix(plugins): give Redshift and Teradata their own Explain variants… Redshift inherited PostgreSQL's EXPLAIN (FORMAT JSON) and EXPLAIN (ANALYZE, FORMAT JSON), which Redshift rejects as syntax errors. It now offers EXPLAIN and EXPLAIN VERBOSE as text. The plugin registry now keeps a curated variant list when a plugin declares none. #3038 had given DuckDB an empty list, which took Explain away from DuckDB the moment its plugin loaded. Teradata gets its EXPLAIN request modifier.
fix(mcp): send explain_query only a variant the database declares… explain_query made up EXPLAIN for engines with no variant, so Redis was sent EXPLAIN GET k. analyze: true on an engine without an ANALYZE variant quietly returned an estimate. Both are now refused, as is analyze: true with an explicit variant that only estimates. Each error names the variants to use instead. A statement that already starts with EXPLAIN still passes through verbatim.
fix(editor): dim Run and Explain Query in the Query menu… The Query menu treated whitespace and invisible characters as query text, so Run and Explain Query were enabled over a blank editor. The menu now uses the same blank rule as the run path.
refactor(coordinator): remove the empty driver branches… Three if let driver = … {} blocks and their unreachable else branches.

iPhone and iPad

Commit What was wrong
fix(ios): report a Redis command the server refuses… An error reply showed as a successful one-row result.
fix(ios): list each Redis key once… SCAN may return a key twice, and the list showed it twice.
fix(ios): browse Redis keys with Redis commands instead of SQL Opening a key sent SELECT * FROM key LIMIT …, which Redis rejects with ERR wrong number of arguments for 'select' command. A key now opens by type: a string's value, a hash's fields, a list's elements, a set's members, a sorted set's members and scores, or a stream's entries.
fix(ios): report a Redis ACL refusal as a permission error… NOPERM read as Authentication Failed, with a hint to check the username and password.

Connection form and tooling

Commit What was wrong
fix(connection-form): accept only a whole number in a number field DuckDB Port and BigQuery Max Bytes Billed kept -, . and non-ASCII digits, and nothing stopped Save. Neither value ever worked at connect.
fix(ci): read wrapped and escaped plugin strings into the app catalog localization.py plugins missed any String(localized:) call wrapped after String(, and any literal containing \", so 52 plugin strings never reached the catalog. Most of them are AWS SSO and credential_process errors.

The Repo Hygiene failure linked on #3038 was already fixed there by 5b54e3c98.

Verification

  • Unit tests, macOS: 231 suites: every Redis suite, every suite in a file this branch touches, plus the classifier and MCP gate suites. 2,425 of 2,428 passed.
    • Two of the three failures were key-open tests: a bare five:x read as a query parameter. The key is now always quoted, and both suites then passed (36 of 36 and 25 of 25).
    • The third is WorkspaceRailCellRenderingTests, an offscreen pixel test in a file this branch does not touch. It was only in the run because it contains the word isRedish.
    • The suites edited after that run pass again: 43 of 43 for the classifier, MCP explain, session factory and cluster database suites, and 64 of 64 for cluster routing.
  • iOS: the app builds for the simulator, and the four iOS commits passed 767 tests on their own branch. After the review fix, the data browser suites pass 25 of 25, including the new overtaken-read case.
  • Build: the app and all 41 plugins (AllPlugins).
  • Live, against redis-server 8.10.1:
    • Parser: SCAN 0 TYPE hash returns two keys where it returned all seven.
    • Parser: CONFIG GET maxmemory maxclients returns two rows where it returned one.
    • Parser: XGROUP HELP and OBJECT HELP reach the server instead of being refused.
    • Key tree: after SELECT 5, KEYTREE DB 0 lists db0's seven keys, and DBSIZE still reports db5.
  • Live, against a three-primary Redis 8.10.1 cluster and a Valkey 9.1.2 cluster with cluster-databases 16:
    • FUNCTION LOAD and ACL SETUSER reach 3 of 3 primaries where they reached 1.
    • A DEL that one shard refused names allowed:1 as already deleted.
    • Valkey lists db0 through db15, and a db3 grid save goes out as DB 3 SET … and leaves the session on db0.
    • Redis OSS still refuses SELECT 3 with nothing sent.
  • Routing check: scripts/check-redis-command-routing.sh compared 151 commands against Redis 8.10.1 and Valkey 9.1.2 and found 0 mismatches.
  • Lint: no violations on any line this branch writes.
  • Other checks: verify.sh docs, localization.py plugins and check-ios-shared-isolation.py all pass.
  • Review: Codex is unavailable until 2026-09-22, so the code-review skill reviewed the whole diff and returned 15 findings.
    • Fixed 10:
      • a quoted "FLUSHALL" passing Safe Mode;
      • a key opened from the tree moving the session;
      • a cluster DB n write queued into an open MULTI block;
      • a partial CONFIG SET or ACL SETUSER fan-out going unreported;
      • racing iOS key reads;
      • explain_query accepting analyze with a variant that only estimates;
      • a second window's toolbar;
      • the generic post-connect switch;
      • a misplaced doc comment;
      • comments this branch added.
    • The declined ones are under Not covered.

Not covered

  • UI automation: every flow here needs a live Redis or a registry plugin, and the UI test runner has neither.
  • Screenshots: none of the changed screens are pictured in the docs. The iOS key browser changed shape, but docs/ios has no Redis screenshot.
  • Found while working, not fixed:
    • On iOS, the Add Row Shortcuts build an SQL INSERT for a Redis connection.
    • An iOS Redis result shows any even-length array of bulk strings as key/value rows, so KEYS, LRANGE, SMEMBERS and MGET results read wrong.
    • String(cString:) in the iOS reply parser stops a binary value at its first NUL byte.
    • The Mac FLUSHDB truncate still refuses a database other than the session's, even though DB <n> FLUSHDB could now handle it.
    • In Cluster mode, a user's own MULTI opens on one primary while the other shards keep running.
    • A Valkey cluster whose primaries all refuse CONFIG stays on db0 even when cluster-databases is above 1. This is documented; INFO keyspace could stand in for it.
    • The cluster reads CONFIG GET cluster-databases at connect and INFO keyspace for each count one primary at a time, not in parallel.
    • Paging deep into a large iOS stream or hash re-reads from the start on every page.
    • Not verified: the Redis editor splits statements with the SQL splitter, so a ; after \" inside a quoted argument may split one command in two.

@mintlify

mintlify Bot commented Sep 21, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 21, 2026, 12:48 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

@datlechin
datlechin merged commit 532f981 into main Sep 21, 2026
6 of 9 checks passed
@datlechin
datlechin deleted the fix/redis-3036-followups branch September 21, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant