Simplify allow_device_custom_id query - remove unnecessary COALESCE#1627
Simplify allow_device_custom_id query - remove unnecessary COALESCE#1627
Conversation
The previous query `COALESCE((to_jsonb(apps) ->> 'allow_device_custom_id')::boolean, true)` would fail when the column doesn't exist because: 1. to_jsonb(apps) ->> 'allow_device_custom_id' returns NULL (as text) 2. NULL::text cast to ::boolean fails with an error 3. COALESCE never gets a chance to provide the default The fix: `COALESCE(to_jsonb(apps) ->> 'allow_device_custom_id', 'true')::boolean` 1. Provide the default 'true' string BEFORE the boolean cast 2. COALESCE returns either the column value or 'true' as text 3. Cast the resulting text to boolean Co-authored-by: riderx <4084527+riderx@users.noreply.github.com>
Applied the same fix to the has_usage_credits query in pg.ts to prevent potential failures when the column doesn't exist on replicas. Co-authored-by: riderx <4084527+riderx@users.noreply.github.com>
|
@copilot In stats what need to be done. |
Since the column is NOT NULL DEFAULT true, we can query it directly without the to_jsonb workaround. Reverted pg.ts change as well since that was based on the same incorrect assumption. Co-authored-by: riderx <4084527+riderx@users.noreply.github.com>
The logic is already correct:
When The fix in commit af6c362 resolved the query error, so the feature now works as designed. |
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a production issue in the /stats endpoint by simplifying the query for the allow_device_custom_id column. The column was added with NOT NULL DEFAULT true constraints in migration 20260210132811_stats_customid_guard.sql, making COALESCE and to_jsonb workarounds unnecessary. The fix removes these workarounds in favor of direct schema column access via Drizzle ORM.
Changes:
- Simplified
allow_device_custom_idquery in stats.ts from COALESCE/to_jsonb pattern to direct schema column access - Removed unused
sqlimport from drizzle-orm
Comments suppressed due to low confidence (1)
supabase/functions/_backend/plugins/stats.ts:171
- This use of variable 'old_version_name' always evaluates to true.
statsActions.push({ action: 'uninstall', versionName: old_version_name ?? 'unknown' })
Summary
The
/statsendpoint was failing in production when queryingallow_device_custom_id. The initial fix attempted to handle replicas with COALESCE, but since the column is defined asNOT NULL DEFAULT true, we can query it directly without any workarounds.Root cause:
The column
allow_device_custom_idis always set with a default value (NOT NULL DEFAULT truein migration20260210132811_stats_customid_guard.sql), so COALESCE and to_jsonb workarounds are unnecessary.Fix:
Direct column access using Drizzle schema:
Changes:
supabase/functions/_backend/plugins/stats.ts- Simplified to direct column query, removed unusedsqlimportsupabase/functions/_backend/utils/pg.ts- Reverted to original state (change was based on incorrect assumption)Test plan
/statsendpoint processes telemetry without SQL errorsScreenshots
N/A - Backend fix
Checklist
bun run lint:backend && bun run lint.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.