Skip to content

fix(datagrid): sort Redis, etcd and Kafka results in place instead of re-running the command with ORDER BY - #3182

Merged
datlechin merged 8 commits into
mainfrom
fix/sort-reruns-non-sql-commands
Oct 1, 2026
Merged

datlechin merged 8 commits into
mainfrom
fix/sort-reruns-non-sql-commands

Conversation

@datlechin

@datlechin datlechin commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Clicking a column header on a Redis, etcd or Kafka query result re-ran the last command with a SQL ORDER BY appended, so LPUSH mylist a pushed ORDER, BY, "length" and ASC, put k v rewrote the key, and a KafkaQL CONSUME was rejected. Root cause: the curated Redis, etcd and Kafka metadata left supportsColumnSort at its default true, so a query tab took the ORDER BY re-run path and a table tab offered a sort the plugins ignore.

Tests

  • MainContentCoordinatorSortTests.commandWithoutOrderBySortsInPlace(command:), parameterized over Redis LPUSH mylist a, etcd put k v and Kafka CONSUME "orders" FROM NEWEST LIMIT 1: a header click sends nothing to the driver, leaves the editor text alone, records the sort state and reorders the held rows. Fails without the fix: the driver received LPUSH mylist a ORDER BY "length" ASC, put k v ORDER BY "Key" ASC and CONSUME "orders" FROM NEWEST ORDER BY "Offset" ASC LIMIT 1, and the held rows were replaced.
  • PluginMetadataRegistryCuratedCapabilityTests.commandLanguagesWithoutOrderByOfferNoColumnSort(typeId:) over Redis, etcd and Kafka: the curated snapshot has supportsColumnSort == false. Fails without the fix, where all three report true.

Docs

The docs rewrite branch covers the user-facing text for this change: the data grid page's per-engine sort notes and the Redis, etcd and Kafka pages.

CI fixes

The failed package test was SyncRecordMapperTests.unknownWireValueFailsClosed. The mapper's duplicate Safe Mode decoder turned unknown values into Off. It now uses the model's SafeModeLevel(wireValue:isReadOnly:) policy. Added regressions for unknown values on read-only connections and renames that preserve unknown wire values.

The AllPlugins build failed because SurrealDB's JSON helper had become a property while its callers still supplied the requested text length. Restored the length-taking helper, keeping display truncation and complete export text.

Verification for these fixes:

  • Full TableProCore package tests: passed, including 1,236 Swift Testing tests plus XCTest.
  • Isolated SwiftPM build of every SurrealDB plugin source with the actual driver test file: 50 tests in 8 suites passed on this branch.
  • AllPlugins Xcode build: passed on feat(plugins): remember row import column mappings per table and add Match by Name and Match by Position #3183 with the corrected helper and PluginKit 34.
  • Project regeneration: passed.
  • SwiftLint on the changed Swift files: zero violations. The helper still flags four existing stale documentation paths (TableProApp/rust-dameng, Contents/MacOS, Contents/Helpers, /Applications/Xcode-beta.app).

Pushing these commits triggers fresh GitHub Actions runs.

Unit-test compile correction

The unit-test build exposed three fixtures still calling the old one-argument cell API: two in Typesense and one in Elasticsearch. They now explicitly pass length: .display, retaining their expected display truncation and write-refusal behavior.

Both fixture files are identical on all four updated branches. The complete test target built on #3183, with all 87 selected storage and write-refusal tests passing. Separately, an isolated SwiftPM build of all Elasticsearch and Typesense driver sources and the actual driver test files passed 175 tests in 14 suites on this branch. SwiftLint found no violations in the two fixture files.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit 4e52581 into main Oct 1, 2026
7 of 9 checks passed
@datlechin
datlechin deleted the fix/sort-reruns-non-sql-commands branch October 1, 2026 05:30
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