From 4dc1a484643e9d0b7c5231432453ab451bb6b32a Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 20 Mar 2020 07:35:15 +0000 Subject: [PATCH] This option removes the subquery all together. It has been recommended that subqueries are really inefficient especially on a delete statement. --- app/dao/notifications_dao.py | 32 ++----------------- ...t_notification_dao_delete_notifications.py | 15 --------- 2 files changed, 2 insertions(+), 45 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 645bf9090..0d6ba4839 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -342,40 +342,12 @@ def delete_notifications_older_than_retention_by_type(notification_type, qry_lim def _delete_notifications(notification_type, date_to_delete_from, service_id, query_limit): - subquery = db.session.query( - Notification.id - ).join(NotificationHistory, NotificationHistory.id == Notification.id).filter( + deleted = Notification.query.filter( Notification.notification_type == notification_type, Notification.service_id == service_id, Notification.created_at < date_to_delete_from, - ).limit(query_limit).subquery() + ).delete() - deleted = _delete_for_query(subquery) - - subquery_for_test_keys = db.session.query( - Notification.id - ).filter( - Notification.notification_type == notification_type, - Notification.service_id == service_id, - Notification.created_at < date_to_delete_from, - Notification.key_type == KEY_TYPE_TEST - ).limit(query_limit).subquery() - - deleted += _delete_for_query(subquery_for_test_keys) - - return deleted - - -def _delete_for_query(subquery): - number_deleted = db.session.query(Notification).filter( - Notification.id.in_(subquery)).delete(synchronize_session='fetch') - deleted = number_deleted - db.session.commit() - while number_deleted > 0: - number_deleted = db.session.query(Notification).filter( - Notification.id.in_(subquery)).delete(synchronize_session='fetch') - deleted += number_deleted - db.session.commit() return deleted diff --git a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py index 648ab303c..202019ed5 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py +++ b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py @@ -227,21 +227,6 @@ def test_delete_notifications_does_try_to_delete_from_s3_when_letter_has_not_bee mock_get_s3.assert_not_called() -@freeze_time("2016-01-10 12:00:00.000000") -def test_should_not_delete_notification_if_history_does_not_exist(sample_service, mocker): - mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects") - mocker.patch("app.dao.notifications_dao.insert_update_notification_history") - with freeze_time('2016-01-01 12:00'): - email_template, letter_template, sms_template = _create_templates(sample_service) - create_notification(template=email_template, status='permanent-failure') - create_notification(template=sms_template, status='delivered') - create_notification(template=letter_template, status='temporary-failure') - assert Notification.query.count() == 3 - delete_notifications_older_than_retention_by_type('sms') - assert Notification.query.count() == 3 - assert NotificationHistory.query.count() == 0 - - def test_delete_notifications_calls_subquery_multiple_times(sample_template): create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8)) create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))