glance-manage db purge failure for limit

MySQL does not support LIMIT in subqueries for certain subquery
operators.

Ref :-
https://dev.mysql.com/doc/mysql-reslimits-excerpt/5.7/en/subquery-restrictions.html

Using nested select statements to resolve this issue.

NOTE:
Added order by clause on 'deleted_at' column so that old records will
be deleted first.

Closes-bug: #1543092
Change-Id: Id90a6686bed60dea74644057c3929643a4c1d201

glance/db/sqlalchemy/api.py

@@ -34,8 +34,10 @@ import six
 # NOTE(jokke): simplified transition to py3, behaves like py2 xrange
 from six.moves import range
 import sqlalchemy
-from sqlalchemy import MetaData, Table, select
+from sqlalchemy.ext.compiler import compiles
+from sqlalchemy import MetaData, Table
 import sqlalchemy.orm as sa_orm
+from sqlalchemy import sql
 import sqlalchemy.sql as sa_sql
 
 from glance.common import exception
@@ -1247,6 +1249,24 @@ def image_tag_get_all(context, image_id, session=None):
     return [tag[0] for tag in tags]
 
 
+class DeleteFromSelect(sa_sql.expression.UpdateBase):
+    def __init__(self, table, select, column):
+        self.table = table
+        self.select = select
+        self.column = column
+
+
+# NOTE(abhishekk): MySQL doesn't yet support subquery with
+# 'LIMIT & IN/ALL/ANY/SOME' We need work around this with nesting select.
+@compiles(DeleteFromSelect)
+def visit_delete_from_select(element, compiler, **kw):
+    return "DELETE FROM %s WHERE %s in (SELECT T1.%s FROM (%s) as T1)" % (
+        compiler.process(element.table, asfrom=True),
+        compiler.process(element.column),
+        element.column.name,
+        compiler.process(element.select))
+
+
 def purge_deleted_rows(context, age_in_days, max_rows, session=None):
     """Purges soft deleted rows
 
@@ -1294,16 +1314,19 @@ def purge_deleted_rows(context, age_in_days, max_rows, session=None):
             _LI('Purging deleted rows older than %(age_in_days)d day(s) '
                 'from table %(tbl)s'),
             {'age_in_days': age_in_days, 'tbl': tbl})
+
+        column = tab.c.id
+        deleted_at_column = tab.c.deleted_at
+
+        query_delete = sql.select(
+            [column], deleted_at_column < deleted_age).order_by(
+            deleted_at_column).limit(max_rows)
+
+        delete_statement = DeleteFromSelect(tab, query_delete, column)
+
         with session.begin():
-            result = session.execute(
-                tab.delete().where(
-                    tab.columns.id.in_(
-                        select([tab.columns.id]).where(
-                            tab.columns.deleted_at < deleted_age
-                        ).limit(max_rows)
-                    )
-                )
-            )
+            result = session.execute(delete_statement)
+
         rows = result.rowcount
         LOG.info(_LI('Deleted %(rows)d row(s) from table %(tbl)s'),
                  {'rows': rows, 'tbl': tbl})

glance/tests/unit/test_glance_manage.py

@@ -0,0 +1,44 @@
+# Copyright 2016 OpenStack Foundation.
+# Copyright 2016 NTT Data.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import mock
+
+from glance.cmd import manage
+from glance import context
+from glance.db.sqlalchemy import api as db_api
+import glance.tests.utils as test_utils
+
+TENANT1 = '6838eb7b-6ded-434a-882c-b344c77fe8df'
+USER1 = '54492ba0-f4df-4e4e-be62-27f4d76b29cf'
+
+
+class DBCommandsTestCase(test_utils.BaseTestCase):
+    def setUp(self):
+        super(DBCommandsTestCase, self).setUp()
+        self.commands = manage.DbCommands()
+        self.context = context.RequestContext(
+            user=USER1, tenant=TENANT1)
+
+    @mock.patch.object(db_api, 'purge_deleted_rows')
+    @mock.patch.object(context, 'get_admin_context')
+    def test_purge_command(self, mock_context, mock_db_purge):
+        mock_context.return_value = self.context
+        self.commands.purge(1, 100)
+        mock_db_purge.assert_called_once_with(self.context, 1, 100)
+
+    def test_purge_command_negative_rows(self):
+        exit = self.assertRaises(SystemExit, self.commands.purge, 1, -1)
+        self.assertEqual("Minimal rows limit is 1.", exit.code)