feat: add raw SQL column defaults#92
Conversation
This change adds a DefaultRaw(sql string) method to the Column builder, allowing developers to specify dialect-specific or complex SQL expressions as default values in schema definitions. This matches Drizzle ORM's .default(sql'...') behavior. Included tests verify correct DDL generation for PostgreSQL, MySQL, and SQLite. Co-authored-by: cungminh2710 <8063319+cungminh2710@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Greptile SummaryThis PR adds
Confidence Score: 5/5Safe to merge. The change is well-scoped, follows the existing generated-column pattern throughout, and is covered by multi-dialect tests. The implementation is consistent with the existing GeneratedExpr code path, mutual exclusivity between Default/DefaultNow/DefaultRaw is correctly enforced, and the only gap is that DefaultExpr is not deep-cloned in cloneTableDef — a minor inconsistency with no practical impact for the expected RawExpr use case. The column-cloning block in pkg/schema/schema.go (cloneTableDef) is the one spot worth a second look — DefaultExpr should be cloned alongside GeneratedExpr.
|
| Filename | Overview |
|---|---|
| pkg/schema/schema.go | Adds DefaultExpr Expression field to ColumnDef and DefaultRaw(expr Expression) builder. Default/DefaultNow/DefaultRaw are made mutually exclusive by clearing sibling fields. Minor: DefaultExpr is not cloned in cloneTableDef unlike the parallel GeneratedExpr field. |
| pkg/rain/ddl.go | Updates columnDefaultSQL to accept *schema.TableDef and handle DefaultExpr first via expressionDDLSQL; guard conditions updated throughout. Changes are consistent and follow the existing GeneratedExpr pattern. |
| pkg/rain/ddl_test.go | Adds TestCreateTableSQLWithDefaultRaw covering now() and random() raw defaults across postgres, mysql, and sqlite dialects. Good coverage of the new feature. |
| pkg/rain/coverage_target_internal_test.go | Mechanical update to pass the new table *schema.TableDef argument to the now-refactored columnDefaultSQL internal function. No logic changes. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Column builder call"] --> B{Which default method?}
B -->|"Default(value)"| C["HasDefault=true\nDefault=value\nDefaultSQL=''\nDefaultExpr=nil"]
B -->|"DefaultNow()"| D["HasDefault=true\nDefaultSQL='CURRENT_TIMESTAMP'\nDefault=nil\nDefaultExpr=nil"]
B -->|"DefaultRaw(expr)"| E{expr == nil?}
E -->|yes| F["panic"]
E -->|no| G["HasDefault=true\nDefaultExpr=expr\nDefault=nil\nDefaultSQL=''"]
C --> H["columnDefaultSQL()"]
D --> H
G --> H
H --> I{DefaultExpr != nil?}
I -->|yes| J["expressionDDLSQL(d, table, DefaultExpr)"]
I -->|no| K{DefaultSQL != ''?}
K -->|yes| L["return DefaultSQL"]
K -->|no| M["switch Default type to literal SQL"]
J --> N["RawExpr: return value.SQL\n(args must be empty)"]
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
pkg/schema/schema.go:1459-1462
`DefaultExpr` is shallow-copied in `cloneTableDef` while the parallel field `GeneratedExpr` is explicitly deep-cloned via `cloneExpressionForTable`. For a `RawExpr` without args (the expected use case) this is harmless, but any expression type that carries `ColumnReference` values would retain pointers to the original table's columns, causing an ownership-check error in `expressionDDLSQL` if DDL is ever generated from an aliased table.
```suggestion
copyColumn.Table = cloned
if column.GeneratedExpr != nil {
copyColumn.GeneratedExpr = cloneExpressionForTable(column.GeneratedExpr, cloned)
}
if column.DefaultExpr != nil {
copyColumn.DefaultExpr = cloneExpressionForTable(column.DefaultExpr, cloned)
}
```
Reviews (2): Last reviewed commit: "feat(schema): add DefaultRaw method for ..." | Re-trigger Greptile
| // DefaultRaw sets a raw SQL string as the default value. | ||
| func (c *Column[T]) DefaultRaw(sql string) *Column[T] { | ||
| c.def.HasDefault = true | ||
| c.def.DefaultSQL = sql | ||
| return c | ||
| } |
There was a problem hiding this comment.
Empty string silently produces
DEFAULT NULL
DefaultRaw("") sets HasDefault = true and DefaultSQL = "". In columnDefaultSQL, the DefaultSQL != "" guard is skipped, so the renderer falls through to the column.Default switch where Default is nil, returning "NULL". The column DDL becomes DEFAULT NULL instead of an error. Every other value-taking builder in this file (Placeholder, As, Sum, etc.) panics on empty input — this one should do the same.
| // DefaultRaw sets a raw SQL string as the default value. | |
| func (c *Column[T]) DefaultRaw(sql string) *Column[T] { | |
| c.def.HasDefault = true | |
| c.def.DefaultSQL = sql | |
| return c | |
| } | |
| // DefaultRaw sets a raw SQL string as the default value. | |
| func (c *Column[T]) DefaultRaw(sql string) *Column[T] { | |
| if strings.TrimSpace(sql) == "" { | |
| panic("schema: DefaultRaw requires a non-empty SQL expression") | |
| } | |
| c.def.HasDefault = true | |
| c.def.DefaultSQL = sql | |
| return c | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/schema/schema.go
Line: 641-646
Comment:
**Empty string silently produces `DEFAULT NULL`**
`DefaultRaw("")` sets `HasDefault = true` and `DefaultSQL = ""`. In `columnDefaultSQL`, the `DefaultSQL != ""` guard is skipped, so the renderer falls through to the `column.Default` switch where `Default` is `nil`, returning `"NULL"`. The column DDL becomes `DEFAULT NULL` instead of an error. Every other value-taking builder in this file (`Placeholder`, `As`, `Sum`, etc.) panics on empty input — this one should do the same.
```suggestion
// DefaultRaw sets a raw SQL string as the default value.
func (c *Column[T]) DefaultRaw(sql string) *Column[T] {
if strings.TrimSpace(sql) == "" {
panic("schema: DefaultRaw requires a non-empty SQL expression")
}
c.def.HasDefault = true
c.def.DefaultSQL = sql
return c
}
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
I've refactored the implementation to use schema.Expression instead of a raw string. This is safer and more consistent with the rest of the builder API. I've also made the default-setting methods (Default, DefaultNow, DefaultRaw) mutually exclusive to prevent ambiguous column definitions.
This change adds a DefaultRaw(expr Expression) method to the Column builder, allowing developers to specify dialect-specific or complex SQL expressions as default values in schema definitions. Updates: - Added DefaultExpr field to ColumnDef. - DefaultRaw now accepts schema.Expression for improved safety and consistency. - Default(), DefaultNow(), and DefaultRaw() are now mutually exclusive. - DDL generation updated to handle DefaultExpr. - Updated tests to use schema.Raw() with DefaultRaw(). This matches Drizzle ORM's .default(sql'...') behavior more closely by leveraging Rain's expression system. Co-authored-by: cungminh2710 <8063319+cungminh2710@users.noreply.github.com>
Add DefaultRaw(sql string) to the Column builder to allow specifying raw SQL expressions as column defaults, matching Drizzle ORM's .default(sql'...') behavior.
PR created automatically by Jules for task 6277780916839863788 started by @cungminh2710