feat(cells) Add date_updated to organizationmapping #103772
Conversation
Add date_updated to organizationmapping. Synapse will need a way to fetch records that have changed since it last synced. Refs INFRENG-172
github-actions
bot
added
the
Scope: Backend
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| prevent_superuser_access = models.BooleanField(default=False, db_default=False) | ||
| disable_member_invite = models.BooleanField(default=False, db_default=False) | ||
|
|
||
| date_updated = models.DateTimeField(db_default=Now(), default=timezone.now) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
It's right here, if you want this to always be updated it's better to use auto_now=True
There was a problem hiding this comment.
Do we have a mistake in DefaultFieldsModelExisting then? That class defines date_updated as
date_updated = models.DateTimeField(default=timezone.now)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| prevent_superuser_access = models.BooleanField(default=False, db_default=False) | ||
| disable_member_invite = models.BooleanField(default=False, db_default=False) | ||
|
|
||
| date_updated = models.DateTimeField(db_default=Now(), default=timezone.now) |
There was a problem hiding this comment.
Bug: Missing index on date_updated
The date_updated field is intended to be used for fetching changed records (synchronization). Since OrganizationMapping is a high-volume table, querying by this field without a database index will result in full table scans and potential performance degradation.
There was a problem hiding this comment.
If you're planning on querying this you do need an index, but I'm not sure what your patterns look like
|
This PR has a migration; here is the generated SQL for for --
-- Add field date_updated to organizationmapping
--
ALTER TABLE "sentry_organizationmapping" ADD COLUMN "date_updated" timestamp with time zone DEFAULT (STATEMENT_TIMESTAMP()) NOT NULL;
CREATE INDEX CONCURRENTLY "sentry_organizationmapping_date_updated_cb5e0369" ON "sentry_organizationmapping" ("date_updated"); |
| prevent_superuser_access = models.BooleanField(default=False, db_default=False) | ||
| disable_member_invite = models.BooleanField(default=False, db_default=False) | ||
|
|
||
| date_updated = models.DateTimeField(db_default=Now(), default=timezone.now) |
There was a problem hiding this comment.
It's right here, if you want this to always be updated it's better to use auto_now=True
| prevent_superuser_access = models.BooleanField(default=False, db_default=False) | ||
| disable_member_invite = models.BooleanField(default=False, db_default=False) | ||
|
|
||
| date_updated = models.DateTimeField(db_default=Now(), default=timezone.now) |
There was a problem hiding this comment.
Do you want all existing rows to be set to have been updated at the time this migration ran? It's fine if you do, just making sure
There was a problem hiding this comment.
Do you want all existing rows to be set to have been updated at the time this migration ran?
Yes, that's fine. This column will be out for a few weeks/months before we start using it.
| prevent_superuser_access = models.BooleanField(default=False, db_default=False) | ||
| disable_member_invite = models.BooleanField(default=False, db_default=False) | ||
|
|
||
| date_updated = models.DateTimeField(db_default=Now(), default=timezone.now) |
There was a problem hiding this comment.
If you're planning on querying this you do need an index, but I'm not sure what your patterns look like
|
PR reverted: 94e744c |
This reverts commit 71e4553. Co-authored-by: markstory <24086+markstory@users.noreply.github.com>
Redo this change as it was reverted due to gaps in the control silo deployment pipeline that didn't run migrations. This migration will have run in development environment, and once the control silo migration pipeline is fixed, I'll get a new migration number and use a `SeparateDatabaseAndState` migration to handle this scenario. Reapply "feat(cells) Add date_updated to organizationmapping (#103772)" This reverts commit 94e744c. Refs INFRENG-172
Redo this change as it was reverted due to gaps in the control silo deployment pipeline that didn't run migrations. This migration will have run in development environment, and once the control silo migration pipeline is fixed, I'll get a new migration number and use a `SeparateDatabaseAndState` migration to handle this scenario. Reapply "feat(cells) Add date_updated to organizationmapping (#103772)" This reverts commit 94e744c. Refs INFRENG-172
Redo this change as it was reverted due to gaps in the control silo deployment pipeline that didn't run migrations. This migration will have run in development environment, and once the control silo migration pipeline is fixed, I'll get a new migration number and use a `SeparateDatabaseAndState` migration to handle this scenario. Reapply "feat(cells) Add date_updated to organizationmapping (#103772)" This reverts commit 94e744c. Refs INFRENG-172
Redo this change as it was reverted due to gaps in the control silo deployment pipeline that didn't run migrations. This migration will have run in development environment, and once the control silo migration pipeline is fixed, I'll get a new migration number and use a `SeparateDatabaseAndState` migration to handle this scenario. Reapply "feat(cells) Add date_updated to organizationmapping (#103772)" This reverts commit 94e744c. Refs INFRENG-172
Add date_updated to organizationmapping. Synapse will need a way to fetch records that have changed since it last synced.
Refs INFRENG-172