Uh oh!
There was an error while loading. Please reload this page.
feat(appkit): add the database runtime and harden its schema builder - #525
feat(appkit): add the database runtime and harden its schema builder#525ditadi wants to merge 3 commits into
Conversation
Repair schema invariants and add a bounded Drizzle execution boundary for future database APIs. Signed-off-by: ditadi <victordperd@gmail.com>
📦 Bundle size reportCompared against
|
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 869 KB | 303 KB |
| Type declarations | 315 KB | 109 KB |
| Source maps | 1.7 MB (+159 B) | 566 KB (+143 B) |
| Other | 11 KB | 3.7 KB |
| Total | 2.9 MB (+159 B) | 982 KB (+143 B) |
Per-entry composition (own code — deps external (as shipped))
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
. | 88 KB | 2.5 KB | 91 KB | external | 288 KB |
./beta | 49 KB | 457 B | 49 KB | external | 143 KB |
./type-generator | 21 KB | 0 B | 21 KB | external | 61 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
. | index.js | initial | 84 KB |
. | utils.js | initial | 4.0 KB |
. | remote-tunnel-manager.js | lazy | 2.5 KB |
./beta | beta.js | initial | 33 KB |
./beta | stream-manager.js | initial | 5.8 KB |
./beta | wide-event-emitter.js | initial | 3.2 KB |
./beta | databricks.js | initial | 3.0 KB |
./beta | configuration.js | initial | 2.1 KB |
./beta | service-context.js | initial | 1.3 KB |
./beta | client.js | initial | 434 B |
./beta | client-options.js | initial | 220 B |
./beta | supervisor-api.js | lazy | 192 B |
./beta | databricks.js | lazy | 142 B |
./beta | index.js | lazy | 123 B |
./type-generator | index.js | initial | 21 KB |
@databricks/appkit-ui
npm tarball (packed): 342 KB (-291 B) — gzipped download (dist + bin; excludes release-only docs/NOTICE).
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 390 KB | 130 KB (+1 B) |
| Type declarations | 228 KB | 83 KB (+3 B) |
| Source maps | 752 KB (-334 B) | 247 KB (-197 B) |
| CSS | 16 KB (-462 B) | 3.2 KB (-90 B) |
| Total | 1.4 MB (-796 B) | 464 KB (-283 B) |
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
./js | 5.3 KB | 49 KB | 55 KB | 208 KB | 14 KB |
./js/beta | 20 B | 0 B | 20 B | 0 B | 0 B |
./react | 432 KB (+127 B) | 49 KB | 480 KB (+127 B) | 1.3 MB | 175 KB |
./react/beta | 1.0 KB | 0 B | 1.0 KB | 0 B | 1.9 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
./js | index.js | initial | 5.2 KB |
./js | chunk | initial | 120 B |
./js | apache-arrow | lazy | 49 KB |
./js/beta | beta.js | initial | 20 B |
./react | index.js | initial | 430 KB |
./react | tslib | initial | 2.1 KB |
./react | apache-arrow | lazy | 49 KB |
./react/beta | beta.js | initial | 1.0 KB |
🤖 AppKit PR bot🔬 Run evalsStart an eval for this PR from the evals-monitor app: Go to Evals Monitor → 📦 Try this PR's app templateScaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh run download 32342551263 -R databricks/appkit -n appkit-template-0.61.1-pr.e75d479-stack-database-mvp-01-runtime-525 -D appkit-pr-525 \
&& unzip -o "appkit-pr-525/appkit-template-0.61.1-pr.e75d479-stack-database-mvp-01-runtime-525.zip" -d "appkit-pr-525" \
&& databricks apps init --template "appkit-pr-525"The template pins |
atilafassina
left a comment
There was a problem hiding this comment.
Awesome work. just 2 question/suggestions before approving :)
| const engineTable = pgTable(table); | ||
| const parameters = mutationValues(table, values); | ||
| const rows = await runDatabaseOperation(() => | ||
| db.insert(engineTable).values(parameters).returning(), |
There was a problem hiding this comment.
I think here (and in other operations) we're using .returning() but it doesn't take into account if a column is private(), I think we'll need to abstract this away and avoid passing all columns to expect* methods. Wdyt?
There was a problem hiding this comment.
Nice catch. returningColumns() now shares the same public-column set as default reads.
| } | ||
| /** Reject offsets that PostgreSQL cannot represent safely as JS integers. */ | ||
| export function validateOffset(offset: number): number { |
There was a problem hiding this comment.
thoughts about setting an offset max boundary as well here?
Maybe this is worth it as belt-and-suspenders protection?
There was a problem hiding this comment.
Nice, yes. I added a MAX_OFFSET cap.
Keep this branch's schema-builder and runtime refactor over oxfmt-only edits from main, and drop tests for APIs this branch already removed. Signed-off-by: ditadi <victordperd@gmail.com>
| }); | ||
| } | ||
| export function selectToColumns( |
There was a problem hiding this comment.
[High] Private-column protection is projection-default-only.isPrivate is consulted only in defaultColumns; selectToColumns (here) has no isPrivate gate, so an explicit select: ["secret"] projects a .private() column. Separately, translateWhere/translateOrder resolve private columns too, so where: {secret: {like: "a%"}} / order: {secret: "asc"} act as a prefix/binary-search exfiltration oracle even without projecting the value. This is a trusted-port design (the HTTP layer in #527 filters via selectable/queryable), but the runtime offers no defense-in-depth on select/where/order.
Automated review finding.
| } | ||
| const conditions: SQL[] = []; | ||
| for (const [key, value] of Object.entries(clause)) { | ||
| if (key === "and" || key === "or") { |
There was a problem hiding this comment.
[Medium] The where predicate tree has no depth or breadth cap. The and/or branch recurses with no depth limit and maps the group array with no size limit — unlike translateInclude (bounded by MAX_INCLUDES). A deeply nested {and:[{and:[…]}]} can overflow the stack (caught as a generic 500), and a wide {or:[…50k…]} defeats IN_CAP and forces an arbitrarily expensive predicate. #527's HTTP decoder bounds this, but the runtime itself does not, so direct/SP callers remain unbounded.
Automated review finding.
| .values(parameters) | ||
| .onConflictDoUpdate({ | ||
| target: columnOf(table, target.columnName), | ||
| set: parameters, |
There was a problem hiding this comment.
[Medium] upsert can rewrite an existing row's primary key.onConflictDoUpdate({ set: parameters }) reuses the full insert payload — including the PK — as the DO UPDATE SET. With a natural (non-serverGenerated) PK plus a separate unique column, upsert({pk:"new", uniqueCol:"existing"}, {onConflict:"uniqueCol"}) flips the existing row's PK → FK orphaning / identity reassignment. deriveUpdateSchema excludes the PK precisely to prevent this; the upsert set should do the same (exclude the PK and ideally the conflict target).
Automated review finding.
| } | ||
| /** Keep mutation identifiers schema-owned and every supplied value parameterized. */ | ||
| function mutationValues(table: AppKitTable, values: Row): Row { |
There was a problem hiding this comment.
[Medium] The mutation port applies none of the schema's own write invariants.mutationValues only checks key membership and rejects SQL wrappers; it does not run columnValueSchema (type check) or exclude serverGenerated/primaryKey. A caller reaching DataPath directly can forge a server-generated id (identity is generatedByDefaultAsIdentity, which accepts explicit values) or rewrite a PK via update. This is by-contract (the typed client in #526 applies $insertSchema/$updateSchema first), but any future caller that reaches DataPath without that layer bypasses every write guard.
Automated review finding.
Stack
Each PR targets the one above it. This one is the base of the stack, so its diff is against
main.What
Rebuilds
packages/appkit/src/database/into a foundation the upcomingDatabasePlugincan be built on: the schema builder now rejects the declarations it used to accept silently, and a new runtime layer gives every future database API a single, bounded path to PostgreSQL.Nothing here is exported. Neither
index.tsnorbeta.tsreaches intodatabase/today, so this PR changes no public API and no generated types or docs — it is groundwork, reviewable on its own, and the plugin that consumes it lands in the following PRs.Changes
A backend-neutral execution boundary (
database/runtime/)DataPathis the only interface the layers above will talk to —select,findOne,count,insert,update,upsert,delete,raw, andtransaction. Its field names are schema keys that an adapter resolves through column metadata, never caller-supplied SQL identifiers. The Drizzle adapter and thetranslatemodule behind it hold the guarantees:rawSQL interpolates values only; a structural interpolation is rejected.DEFAULT_LIMIT, explicit bounds are validated againstMAX_LIMIT, and to-many includes are bounded the same way..private()never leaves the database unless it is asked for by name.inlists are bounded, an emptyinhas deterministic semantics rather than matching everything, null matching is only everis: null, and an emptyand/orgroup is rejected instead of silently widening the query.insertandupsertmust produce exactly one row,updateanddeleteaccept zero or one and reject many.Schema-builder invariants
The builder previously accepted declarations that could not hold at runtime. It now refuses them at declaration time:
SET NULLrequires a nullable column andSET DEFAULTa compatible local default. A generated identity mirrors to non-generated integer storage on the referencing side.varcharlengths and enum declarations, record literal defaults rather than synthesizing them, and restrict thedefaultNow/defaultRandomhelpers to the timestamp and UUID kinds.defineSchemarequires each declared table exactly once, rejects aliases and duplicate handles, validates literal defaults against both storage and enum values, refuses a table that would overwrite Drizzle relation metadata, and no longer leaves handles half-finalized when validation fails partway through.Removals
.owner()and theisOwnermetadata it set are gone. They described row ownership for an RLS story this runtime does not implement, and nothing in the repository called them.contract/column-info.ts,contract/relation.ts, andschema-builder/private.tswere folded into the modules that own their data, which narrows the internal barrel accordingly.Verification
pnpm vitest run— 3966 passing, 1 skippedpnpm -r typecheck— clean across all packagespnpm run generate:types,pnpm run sync:template, andpnpm run docs:buildproduce no drift, as expected for a change with no exported surfacemainat v0.57.0