Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds database schema introspection functionality to the deployment system. It implements automatic detection of database dialects, schema retrieval, and a periodic refresh mechanism to keep schema information current.
Key changes:
- Adds comprehensive SQL dialect detection and schema introspection for multiple database types (PostgreSQL, MySQL, SQLite, SQL Server, Oracle, DuckDB)
- Implements a periodic background schema refresh loop that updates cached database schemas
- Provides a new API endpoint to fetch cached database schema information for deployments
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| api/sql.ts | Core implementation of database dialect detection, schema introspection queries, and refresh logic |
| api/server.ts | Integration of schema refresh loop startup in the main server initialization |
| api/schema.ts | Database schema model definition and collection setup for caching schema data |
| api/routes.ts | New API endpoint to retrieve cached database schema for a deployment |
Comments suppressed due to low confidence (1)
api/sql.ts:1
- The Oracle query has incorrect SQL syntax. The assignment
table_schema = owner AS table_schemais invalid. It should beowner AS table_schema.
import { DatabaseSchemasCollection, DeploymentsCollection } from './schema.ts'
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| export function startSchemaRefreshLoop() { | ||
| if (intervalHandle) return | ||
| // initial kick (non-blocking) | ||
| refreshAllSchemas() |
There was a problem hiding this comment.
The initial schema refresh call is not awaited, which means the interval starts immediately without waiting for the first refresh to complete. This could lead to overlapping refresh operations if the refresh takes longer than the interval. Consider awaiting this call or adding concurrency protection.
| export function startSchemaRefreshLoop() { | |
| if (intervalHandle) return | |
| // initial kick (non-blocking) | |
| refreshAllSchemas() | |
| export async function startSchemaRefreshLoop() { | |
| if (intervalHandle) return | |
| // initial kick (awaited) | |
| await refreshAllSchemas() |
| for (const dep of DeploymentsCollection.values()) { | ||
| await refreshOneSchema(dep) | ||
| } |
There was a problem hiding this comment.
Sequential processing of deployments could be slow with many deployments. Consider using Promise.allSettled() to refresh schemas concurrently while preventing one failure from blocking others.
| for (const dep of DeploymentsCollection.values()) { | |
| await refreshOneSchema(dep) | |
| } | |
| await Promise.allSettled( | |
| Array.from(DeploymentsCollection.values()).map(dep => refreshOneSchema(dep)) | |
| ); |
| output: OBJ({ | ||
| deploymentUrl: STR('Deployment url (matches deployment.url)'), | ||
| dialect: STR('Detected SQL dialect'), | ||
| refreshedAt: STR('ISO datetime of last refresh'), | ||
| tables: ARR(OBJ({ | ||
| columns: ARR(OBJ({ | ||
| name: STR('Column name'), | ||
| type: STR('Column data type'), | ||
| ordinal: NUM('Column ordinal position'), | ||
| })), | ||
| schema: optional(STR('Schema name')), | ||
| table: STR('Table name'), | ||
| })), | ||
| }, 'Database schema cache for a deployment'), |
There was a problem hiding this comment.
The output schema definition duplicates the DatabaseSchemaDef from schema.ts. Consider importing and reusing DatabaseSchemaDef to maintain consistency and reduce duplication.
…ent schema refresh loop
8d19bd3 to
a28d776
Compare
…ion SQL for unsupported databases
feat(schema): add endpoint to fetch cached database schema and implement schema refresh loop