Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements support for PostgreSQL generated columns with the GENERATED ALWAYS AS (...) STORED syntax. The changes enable the schema management tool to properly detect, parse, and generate DDL for stored generated columns.
- Adds detection of generated columns in database queries by checking the
attgeneratedattribute - Updates the column data structure to include generated expression and status fields
- Implements proper DDL generation for
ALTER TABLE ADD COLUMNstatements with generated column syntax
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
testdata/diff/create_table/add_column_generated/* |
Test data files demonstrating generated column functionality |
ir/queries/queries.sql.go |
Updates generated Go code to handle generated column fields from database queries |
ir/queries/queries.sql |
Modifies SQL queries to extract generated column information from PostgreSQL system catalogs |
ir/ir.go |
Adds GeneratedExpr and IsGenerated fields to the Column struct |
ir/parser.go |
Implements parsing logic for GENERATED constraints and expression extraction |
ir/inspector.go |
Updates column building logic to handle generated columns from database inspection |
internal/diff/table.go |
Modifies DDL generation to properly format generated column syntax in ALTER statements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Don't add DEFAULT for SERIAL columns, identity columns, or generated columns | ||
| if column.DefaultValue != nil && column.Identity == nil && !column.IsGenerated && !isSerialColumn(column) { | ||
| stmt += fmt.Sprintf(" DEFAULT %s", *column.DefaultValue) | ||
| } | ||
|
|
||
| // Don't add NOT NULL for identity columns or SERIAL columns as they are implicitly NOT NULL | ||
| // Also skip NOT NULL if we're adding PRIMARY KEY inline (PRIMARY KEY implies NOT NULL) | ||
| // For generated columns, include NOT NULL if explicitly specified (but before GENERATED clause) | ||
| if !column.IsNullable && column.Identity == nil && !isSerialColumn(column) && pkConstraint == nil { | ||
| stmt += " NOT NULL" | ||
| } |
There was a problem hiding this comment.
[nitpick] The logic for determining when to add DEFAULT and NOT NULL clauses has become complex and duplicated. Consider extracting this into separate helper functions like shouldAddDefault(column) and shouldAddNotNull(column, pkConstraint) to improve readability and maintainability.
1 participant
Fix #37