[EN-1038] feat(db): add template disk entitlement schema#3286
Conversation
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
Code Review
This pull request introduces database migrations to add disk entitlement columns to the tiers and addons tables, creates a new team_limits_v2 view, and adds corresponding Go models and tests. The review feedback identifies a critical issue where explicit ::bigint casts in the view definition cause sqlc to generate non-nullable int64 fields in Go, which will lead to runtime scan crashes when database values are NULL. To resolve this, the reviewer recommends removing the redundant casts so sqlc can correctly infer nullability, and updating the migration tests to scan into nullable pointer types.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
LGTM — straightforward, well-tested expand-only schema migration.
What was reviewed:
- Migration adds nullable disk-entitlement columns to tiers/addons with CHECK constraints (NULL-safe, no backfill needed).
- New team_limits_v2 view combines tier + addon values, preserves security_invoker=on and legacy extra_max_disk_size_mb→extra_disk_mb fallback for old add-on writers.
- Verified the effective default/max math in the migration test matches expectations, and that the existing team_limits (v1) view is left untouched.
- Checked the generated TeamLimitsV2 Go struct typing NULL-derived columns as int64 instead of pgtype.Int8 — inert since no query in the codebase currently selects from team_limits_v2, so it has no runtime effect and will be corrected on the next real sqlc generation.
Extended reasoning...
Overview
This PR adds two nullable bigint columns (default_free_disk_size_mb, max_disk_size_mb) to tiers, one nullable column (extra_max_disk_size_mb) to addons, and CHECK constraints preventing negative or inverted tier values. It then introduces a new team_limits_v2 view that surfaces the effective (tier + active add-ons) default and maximum disk sizes, while leaving the existing team_limits view untouched for current consumers. Test coverage exercises column nullability, view column sets for v1 vs v2, security_invoker behavior, and the arithmetic for effective values including legacy add-on fallback.
Security risks
None identified. The new view preserves security_invoker=on, matching the existing team_limits v1 view's RLS-respecting behavior, and this is explicitly covered by a test. No auth, crypto, or permission logic is touched.
Level of scrutiny
This is a pure, additive (expand-only) database schema change with no runtime/application code wired up yet — the PR description explicitly defers data population and adoption to later steps. Given the low blast radius (no existing code paths depend on the new columns/view yet) and strong test coverage of the new schema behavior, a lighter level of scrutiny is appropriate compared to changes touching live request paths.
Other factors
The bug-hunting system found no issues. I independently checked the flagged-and-ruled-out concern about the generated TeamLimitsV2 struct mistyping nullable view columns as non-nullable int64, and confirmed it has no functional impact since nothing in the codebase queries team_limits_v2 yet — it's inert generated code that will be corrected whenever sqlc is actually re-run against a query using it. The migration's up/down scripts are symmetric and CHECK constraints are correctly NULL-safe for the expand-only rollout.
67bcfe1 to
4621212
Compare
There was a problem hiding this comment.
can we add if not exists to the statements🙏🏻
There was a problem hiding this comment.
Added for new columns. Adding constraint checks does not support IF NOT EXISTS.
Add nullable tier entitlements for the default template free-space target and maximum logical rootfs size, plus an add-on contribution to the maximum. Validate non-negative defaults and add-on increments, positive maxima, and that each default does not exceed its maximum. Backfill existing add-ons so their maximum contribution initially matches extra_disk_mb. Keep the new field nullable and fall back to extra_disk_mb for writes from legacy services during rollout. Leave tier population to the audited follow-up data migration. Extend team_limits in place, preserving its existing columns and appending the effective default_free_disk_size_mb and max_disk_size_mb values computed from active add-ons. Regenerate sqlc queries and models with nullable int64 entitlement fields, and cover the schema shape, security-invoker setting, column compatibility, and tier-plus-add-on calculations in the migration test. Part of: EN-1038
4621212 to
6f513fb
Compare
Separate the default template free-space target from the maximum total root filesystem size that a team may build.
Add nullable default_free_disk_size_mb and max_disk_size_mb columns to tiers, plus extra_max_disk_size_mb to add-ons. Keep extra_disk_mb as the add-on contribution to default free space and add constraints preventing invalid negative or inverted tier values.
Keep the existing team_limits view unchanged for current consumers and introduce team_limits_v2 with the new effective default and maximum fields. Preserve compatibility with legacy add-on writers by treating a null extra maximum as equal to extra_disk_mb.
Leave data population and runtime adoption to later rollout steps so this commit remains an expand-only schema change.
Regenerate the database models and add focused migration coverage for the new columns, the unchanged V1 view, the V2 contract, security invoker behavior, and effective tier-plus-add-on calculations.
Part of: EN-1038