fix(data): BaseModel.CreatedAt and ModifiedAt as NOT NULL#650
Conversation
BeforeCreate always populates these columns, so the model's GORM schema should reflect that. Without the tag, AutoMigrate runs on existing tables would emit `ALTER COLUMN … DROP NOT NULL`, which Postgres rejects when the column has been promoted into a composite primary key (the TimescaleDB hypertable pattern).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the BaseModel struct in data/model.go to include not null constraints on the CreatedAt and ModifiedAt fields, ensuring consistent schema generation for composite primary keys. Feedback includes a suggestion to use the modern primaryKey GORM tag and restore field alignment for the ID field, as well as a recommendation to update the documentation to include the BeforeUpdate hook's role in the lifecycle of the ModifiedAt field.
| ID string `gorm:"type:varchar(50);primary_key"` | ||
| CreatedAt time.Time | ||
| ModifiedAt time.Time | ||
| ID string `gorm:"type:varchar(50);primary_key"` |
There was a problem hiding this comment.
The alignment of the ID field tag was removed in this change, making it inconsistent with the rest of the struct. Additionally, GORM v2 recommends using the primaryKey tag instead of the legacy primary_key tag.
| ID string `gorm:"type:varchar(50);primary_key"` | |
| ID string `gorm:"type:varchar(50);primaryKey"` |
| // CreatedAt and ModifiedAt are NOT NULL because BeforeCreate always | ||
| // populates them. Tagging them here keeps AutoMigrate from trying to | ||
| // relax the columns on tables that promote CreatedAt into a composite | ||
| // primary key (e.g. TimescaleDB hypertables). |
There was a problem hiding this comment.
The documentation for CreatedAt and ModifiedAt is slightly incomplete. While it correctly mentions BeforeCreate, the ModifiedAt field is also updated in the BeforeUpdate hook (line 144). Updating the comment to reflect this would provide a more accurate description of the field's lifecycle.
1 participant
Summary
BeforeCreate always populates CreatedAt and ModifiedAt. Without the
`not null` tag, subsequent AutoMigrate runs on existing tables emit
`ALTER COLUMN … DROP NOT NULL`, which Postgres rejects when the column
has been promoted into a composite primary key — the pattern
TimescaleDB hypertables require.
Tagging the columns NOT NULL aligns the model with runtime behaviour and
makes AutoMigrate idempotent on hypertable-bound tables.
Test plan