fix(datagrid): sort Redis, etcd and Kafka results in place instead of re-running the command with ORDER BY - #3182
Merged
Merged
Conversation
… re-running the command with ORDER BY
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
# Conflicts: # Plugins/SurrealDBDriverPlugin/SurrealValue+Display.swift
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ORDER BYappended, soLPUSH mylist apushedORDER,BY,"length"andASC,put k vrewrote the key, and a KafkaQLCONSUMEwas rejected. Root cause: the curated Redis, etcd and Kafka metadata leftsupportsColumnSortat its defaulttrue, so a query tab took theORDER BYre-run path and a table tab offered a sort the plugins ignore.Tests
MainContentCoordinatorSortTests.commandWithoutOrderBySortsInPlace(command:), parameterized over RedisLPUSH mylist a, etcdput k vand KafkaCONSUME "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 receivedLPUSH mylist a ORDER BY "length" ASC,put k v ORDER BY "Key" ASCandCONSUME "orders" FROM NEWEST ORDER BY "Offset" ASC LIMIT 1, and the held rows were replaced.PluginMetadataRegistryCuratedCapabilityTests.commandLanguagesWithoutOrderByOfferNoColumnSort(typeId:)overRedis,etcdandKafka: the curated snapshot hassupportsColumnSort == false. Fails without the fix, where all three reporttrue.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'sSafeModeLevel(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:
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.