DDL: create table #63
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements DDL (Data Definition Language) support for PostgreSQL by adding CREATE TABLE functionality. The implementation includes comprehensive column definitions, constraints, and table options following PostgreSQL syntax.
- Adds DDL Context to Expression system to handle literal values without query parameters
- Implements comprehensive CREATE TABLE grammar with column constraints and table constraints
- Updates type system to use
new this(v)pattern for proper inheritance
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/record.ts | Updates compile method to accept Context parameter for DDL support |
| src/types/bool.ts | Adds static new method overloads using inheritance pattern |
| src/types/any.ts | Changes static new method to use new this(v) for proper inheritance |
| src/grammar/utils.ts | Adds Identifier type for SQL injection prevention |
| src/grammar/create-table.ts | Implements complete CREATE TABLE grammar with constraints and options |
| src/grammar/create-table.test.ts | Comprehensive test suite for CREATE TABLE functionality |
| src/gen/gen.ts | Updates code generation to use inheritance pattern and remove type prefixes |
| src/expression.ts | Adds DDL context support and literal escaping for DDL statements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import type { OrderBySpec } from "./query/order-by"; | ||
| import { compileOrderBy } from "./query/order-by"; | ||
| import { escapeLiteral } from "pg"; | ||
|
|
There was a problem hiding this comment.
The import of escapeLiteral from the 'pg' package may not be available in all environments or could introduce a dependency that's not needed everywhere. Consider wrapping this in a try-catch or providing a fallback implementation.
| // Helper to provide escapeLiteral with fallback if 'pg' is not available | |
| function getEscapeLiteral() { | |
| try { | |
| // Try to require 'pg' dynamically (works in Node.js environments) | |
| // eslint-disable-next-line @typescript-eslint/no-var-requires | |
| return require("pg").escapeLiteral as (str: string) => string; | |
| } catch { | |
| // Fallback: basic SQL literal escaping (not as robust as pg's) | |
| return (str: string) => "'" + str.replace(/'/g, "''") + "'"; | |
| } | |
| } | |
| const escapeLiteral = getEscapeLiteral(); |
| class ExtendedType extends (DataType as any) { | ||
| static options = [...(combined ? [combined] : []), ...(constraints ?? [])]; | ||
| } | ||
| return ExtendedType.new(new BareColumnExpression("TODO")) as ColumnWithNullability<T, C>; |
There was a problem hiding this comment.
The hardcoded "TODO" string suggests incomplete implementation. This should be replaced with a proper value or the TODO should be addressed before merging.
| return ExtendedType.new(new BareColumnExpression("TODO")) as ColumnWithNullability<T, C>; | |
| return ExtendedType.new(new BareColumnExpression("")) as ColumnWithNullability<T, C>; |
|
|
||
| return sql`${sqlJoin(parts, sql` `)} (${sqlJoin(columnDefs)})${afterTable ? sql` ${afterTable}` : sql``}`; | ||
| } | ||
|
|
There was a problem hiding this comment.
The todo() method throws an error for unimplemented features. Consider either implementing these features or documenting which specific PostgreSQL CREATE TABLE features are intentionally not supported.
| /** | |
| * Not implemented: The following PostgreSQL CREATE TABLE features are intentionally not supported: | |
| * - INHERITS | |
| * - PARTITION BY | |
| * - USING (storage method) | |
| * - WITH (storage parameters) / WITHOUT OIDS | |
| * - ON COMMIT | |
| * - TABLESPACE | |
| * | |
| * If you require these features, please open an issue or submit a pull request. | |
| */ |
| for (const key of Object.keys(instance)) { | ||
| const columnDef = instance[key as keyof T]; | ||
| invariant(isColumn(columnDef), `Property ${key} is not a valid column definition`); | ||
| columnDef.v = new BareColumnExpression(key); |
There was a problem hiding this comment.
[nitpick] Direct assignment to columnDef.v suggests mutation of an object that may be expected to be immutable. Consider if this side effect is necessary or if there's a more functional approach.
| columnDef.v = new BareColumnExpression(key); | |
| instance[key as keyof T] = { ...columnDef, v: new BareColumnExpression(key) }; |
a23f8ed to
c865686
Compare
c865686 to
230354c
Compare
No description provided.