feat(launch): identify database files by their contents, and open .parquet from Finder (#2476) - #2587
Merged
Merged
Conversation
…rquet from Finder (#2476)
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
What was wrong
URLClassifier.classifyFiledecided a file's type fromurl.pathExtensionand nothing else. It is the single funnel for a Finder double-click,open -a, Open With, and a drag onto a window or the Dock icon, so a database with no extension or a foreign one (store.bin,ledger) was unrecognised however unambiguous its bytes were, and a.dbfile written by DuckDB was handed to the SQLite driver, which reports "file is not a database".Two smaller gaps came with it.
Info.plistdeclared no Parquet document type, so LaunchServices never offered TablePro for one even though the DuckDB driver already reads Parquet. And File > Open File… was a window command restricted tosql,psql,pgsql, so no database file could be picked from inside the app at all.Two of the four claims in #2476 were already shipped: the SQLite extension family and
duckdb/ddbare declared (#1331), andDuckDBFileKinds.readOnlyDataalready opens Parquet read-only.What the engines actually do
Every claim here was measured against the shipping tools, sqlite3 3.43 and DuckDB 1.5.4, not taken from documentation.
Recognising a format is worth nothing unless the driver can then open the file, and the three candidates differ on exactly that:
SQLite format 3\0at 0store.bin,ledgerandnotes.csvall query fineDUCKat 8warehouse.db,plainandthing.binall query finePAR1at both endsIO Error: … not a valid DuckDB database fileduckdb_openpicks a data format's reader from the extension. A Parquet file namedexport.binis refused outright, and one namedfake.csvgets the CSV sniffer and fails on the first query rather than at connect. So Parquet is deliberately not recognised by content: the classifier would route a file the driver cannot open, which is worse than not recognising it. It reaches TablePro by name, through the newInfo.plistclaim, which is what the issue asked for. Both classifier and routing tests pin that decision with the reason.The DuckDB marker needed hardening for the opposite reason. Four bytes are not enough to name a format, and
SELECT 'DUCK';spells them at exactly offset 8, so an ordinary SQL file would have been routed to DuckDB ahead of its own extension. The real header is[8-byte checksum][DUCK][storage version as little-endian uint64], so the signature also requires the version's high six bytes to be zero. Text cannot satisfy that, and a storage version past 65535 would cost recognition by content rather than break it.The fix
DatabaseFileSignatureis a set of byte markers at fixed offsets, all of which must match.DatabaseFileClassifierreads only as far as the longest one reaches, refuses anything that is not a regular file so a FIFO cannot block the main thread, and answers nil rather than throwing so the caller falls back to the extension.The signatures live on
PluginMetadataSnapshot.SchemaInfo, beside thefileExtensionsthey belong with, curated per type in the app. They are curated rather than declared by the plugin because claiming a format from the system also needs aCFBundleDocumentTypesentry only the app bundle can make. That meansbuildMetadataSnapshothas to carry them over or registering the plugin resets them to[], which is #1970's failure mode, soPluginMetadataRegistryCuratedCapabilityTestsnow pins that too.classifyFileconsults the contents after TablePro's own.tablepluginand.tablepro, and before every other extension, so contents win a disagreement.Recognising a
.dbfile as DuckDB exposed a latent defect in the route it then takes.TabRouter.openDatabaseFilededuplicated live sessions onconnection.database, but a driver that opens a local file keeps its path in whichever field it declares, and DuckDB and libSQL leavedatabaseempty. So the scan missed an already-open DuckDB connection and opened a secondduckdb_openon the same file, whichCLAUDE.mddocuments as two independent read-write instances whose writes are invisible to each other. Both the scan and the connection it builds now go throughlocalFilePathField.Other consequences handled in the same change:
FileDropDestination.isOpenablenow reads from disk, andEditorWindow.draggingUpdated:calls it on every pointer move. A drag session's pasteboard cannot change while it is in flight, so the answer is settled once indraggingEntered:and released ondraggingExited:/draggingEnded:.MainSplitViewControllertoAppDelegate, which is where the File menu's other window-independent commands already live. It was gated oncontext.isConnectedand unreachable from the welcome window, which was defensible while it only opened SQL files into a session and wrong now that it opens databases, connection shares and plugins that need no connection at all.allowedContentTypescan only match a name, so it disables the very files this fixes;NSOpenSavePanelDelegate.panel(_:shouldEnable:)asks the classifier instead. It asks the new name-onlyURLClassifier.classifyByNamefirst, so browsing a folder of files the app already recognises never reads from a stalled mount on the main actor, and caches per panel session.Info.plistgains a Parquet document type asViewer/Alternatewith an importedorg.apache.parquet. DuckDB opens Parquet read-only, soVieweris the honest role, and no app owns the format, soAlternateleaves a dedicated tool its association. This follows what #1594 established rather than claiming ownership.Finally, DuckDB ships from the registry, so opening a
.parquetwithout it used to raise a window headlined "Could not connect" whose only action left for the connection list.MissingDriverPluginPromptasks about the file instead, before anything opens, and installs on a yes. It awaitswaitForInitialLoad()before deciding, because a plugin withoutTableProProvidesDatabaseTypeIdsregisters on the eager path and a Finder open beats it to the question, which would offer to install a plugin the user already has.PluginMetadataRegistry+RegistryDefaults.swiftsat at exactly the 1200-line limit, so DuckDB moved intoPluginMetadataRegistry+DuckDBDefaults.swift, following Elasticsearch, SurrealDB, Kafka and Turso, and joining the+DuckDBConnectionFields.swiftthat was already beside it.What this cannot do
A Finder double-click on an extensionless file still will not open in TablePro. LaunchServices has nothing to match on, and declaring
public.datawould put TablePro in Open With for every file on the disk. Fixed here: drag onto a window, drag onto the Dock icon, Open With > Other,open -a TablePro, File > Open File…, and any SQLite or DuckDB file whose extension contradicts its contents.Testing
DatabaseFileClassifierTests: both markers against files written with the measured header bytes,SELECT 'DUCK';and a longer SQL statement asserted not to match, a truncated header, an empty file, a directory, a missing path, a remote URL, and Parquet under both names asserting it is left alone.URLClassifierTests: a SQLite database namedledger,store.bin,notes.csvandquery.sql;SELECT 'DUCK';still routed to the editor; Parquet refused asexport.binand routed asexport.parquet;classifyByNameanswering without reading; a JPEG still refused;.tableprostill decided by name.FileDropDestinationTests: an extensionless SQLite database is accepted by a drop.PluginMetadataRegistryCuratedCapabilityTests: signatures survive plugin registration.DocumentTypeDeclarationTests: the Parquet claim, and every claimed extension is one its driver actually declares.Built the app and ran 205 cases across 29 suites, plus
swiftlint --stricton both targets and the docs checks. Reviewed by Codex twice; the review pass caught the Parquet routing defect and the connection-state gate, and the adversarial pass caught theDUCKcollision, the duplicate DuckDB writer and the cold-launch plugin race.No
TableProUITestscoverage: the flows are a Finder drag, a system open panel hosted by another process, and a plugin download. None runs deterministically here.Fixes #2476