Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

db(add columns): created_at #93

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions migrations/000007_add_date_columns_to_objects.down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE dl.objects DROP COLUMN created_at;
1 change: 1 addition & 0 deletions migrations/000007_add_date_columns_to_objects.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE dl.objects ADD COLUMN created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think defaulting to current timestamp is cool, we wouldn't need to add any code changes and would get created_at for free.

The only worry here is what do we do for existing rows, I kind of feel like they should be null instead of some fixed date when we rolled this out.

Can you try testing the following:

  • Create rows without this patch
  • Run this migration (without the NOT NULL part)

Do the existing rows get the date of the migration as their value or null? I suspect they'll get the date of the migration.

We might need to run a first migration with default null and then a second migration with default current_timestamp

Loading