diff --git a/piccolo/apps/migrations/auto/diffable_table.py b/piccolo/apps/migrations/auto/diffable_table.py index b4457cc4d..89b312018 100644 --- a/piccolo/apps/migrations/auto/diffable_table.py +++ b/piccolo/apps/migrations/auto/diffable_table.py @@ -129,6 +129,7 @@ def __sub__(self, value: DiffableTable) -> TableDelta: column_class_name=i.column.__class__.__name__, column_class=i.column.__class__, params=i.column._meta.params, + schema=self.schema, ) for i in sorted( {ColumnComparison(column=column) for column in self.columns} @@ -145,6 +146,7 @@ def __sub__(self, value: DiffableTable) -> TableDelta: column_name=i.column._meta.name, db_column_name=i.column._meta.db_column_name, tablename=value.tablename, + schema=self.schema, ) for i in sorted( {ColumnComparison(column=column) for column in value.columns} @@ -184,6 +186,7 @@ def __sub__(self, value: DiffableTable) -> TableDelta: old_params=old_params, column_class=column.__class__, old_column_class=existing_column.__class__, + schema=self.schema, ) ) diff --git a/piccolo/apps/migrations/auto/migration_manager.py b/piccolo/apps/migrations/auto/migration_manager.py index ad1175d81..8c2e9548c 100644 --- a/piccolo/apps/migrations/auto/migration_manager.py +++ b/piccolo/apps/migrations/auto/migration_manager.py @@ -27,6 +27,7 @@ class AddColumnClass: column: Column table_class_name: str tablename: str + schema: t.Optional[str] @dataclass @@ -206,6 +207,7 @@ def rename_table( old_tablename: str, new_class_name: str, new_tablename: str, + schema: t.Optional[str] = None, ): self.rename_tables.append( RenameTable( @@ -213,6 +215,7 @@ def rename_table( old_tablename=old_tablename, new_class_name=new_class_name, new_tablename=new_tablename, + schema=schema, ) ) @@ -225,6 +228,7 @@ def add_column( column_class_name: str = "", column_class: t.Optional[t.Type[Column]] = None, params: t.Dict[str, t.Any] = None, + schema: t.Optional[str] = None, ): """ Add a new column to the table. @@ -255,6 +259,7 @@ def add_column( column=column, tablename=tablename, table_class_name=table_class_name, + schema=schema, ) ) @@ -264,6 +269,7 @@ def drop_column( tablename: str, column_name: str, db_column_name: t.Optional[str] = None, + schema: t.Optional[str] = None, ): self.drop_columns.append( DropColumn( @@ -271,6 +277,7 @@ def drop_column( column_name=column_name, db_column_name=db_column_name or column_name, tablename=tablename, + schema=schema, ) ) @@ -282,6 +289,7 @@ def rename_column( new_column_name: str, old_db_column_name: t.Optional[str] = None, new_db_column_name: t.Optional[str] = None, + schema: t.Optional[str] = None, ): self.rename_columns.append( RenameColumn( @@ -291,6 +299,7 @@ def rename_column( new_column_name=new_column_name, old_db_column_name=old_db_column_name or old_column_name, new_db_column_name=new_db_column_name or new_column_name, + schema=schema, ) ) @@ -304,6 +313,7 @@ def alter_column( old_params: t.Dict[str, t.Any] = None, column_class: t.Optional[t.Type[Column]] = None, old_column_class: t.Optional[t.Type[Column]] = None, + schema: t.Optional[str] = None, ): """ All possible alterations aren't currently supported. @@ -322,6 +332,7 @@ def alter_column( old_params=old_params, column_class=column_class, old_column_class=old_column_class, + schema=schema, ) ) @@ -403,7 +414,10 @@ async def _run_alter_columns(self, backwards=False): _Table: t.Type[Table] = create_table_class( class_name=table_class_name, - class_kwargs={"tablename": alter_columns[0].tablename}, + class_kwargs={ + "tablename": alter_columns[0].tablename, + "schema": alter_columns[0].schema, + }, ) for alter_column in alter_columns: @@ -635,7 +649,10 @@ async def _run_drop_columns(self, backwards=False): _Table: t.Type[Table] = create_table_class( class_name=table_class_name, - class_kwargs={"tablename": columns[0].tablename}, + class_kwargs={ + "tablename": columns[0].tablename, + "schema": columns[0].schema, + }, ) for column in columns: @@ -662,7 +679,11 @@ async def _run_rename_tables(self, backwards=False): ) _Table: t.Type[Table] = create_table_class( - class_name=class_name, class_kwargs={"tablename": tablename} + class_name=class_name, + class_kwargs={ + "tablename": tablename, + "schema": rename_table.schema, + }, ) await self._run_query( @@ -680,7 +701,10 @@ async def _run_rename_columns(self, backwards=False): _Table: t.Type[Table] = create_table_class( class_name=table_class_name, - class_kwargs={"tablename": columns[0].tablename}, + class_kwargs={ + "tablename": columns[0].tablename, + "schema": columns[0].schema, + }, ) for rename_column in columns: @@ -746,7 +770,10 @@ async def _run_add_columns(self, backwards=False): _Table: t.Type[Table] = create_table_class( class_name=add_column.table_class_name, - class_kwargs={"tablename": add_column.tablename}, + class_kwargs={ + "tablename": add_column.tablename, + "schema": add_column.schema, + }, ) await self._run_query( @@ -765,7 +792,10 @@ async def _run_add_columns(self, backwards=False): # sets up the columns correctly. _Table: t.Type[Table] = create_table_class( class_name=add_columns[0].table_class_name, - class_kwargs={"tablename": add_columns[0].tablename}, + class_kwargs={ + "tablename": add_columns[0].tablename, + "schema": add_columns[0].schema, + }, class_members={ add_column.column._meta.name: add_column.column for add_column in add_columns diff --git a/piccolo/apps/migrations/auto/operations.py b/piccolo/apps/migrations/auto/operations.py index 2a5192b3b..0676bdbd4 100644 --- a/piccolo/apps/migrations/auto/operations.py +++ b/piccolo/apps/migrations/auto/operations.py @@ -10,6 +10,7 @@ class RenameTable: old_tablename: str new_class_name: str new_tablename: str + schema: t.Optional[str] = None @dataclass @@ -28,6 +29,7 @@ class RenameColumn: new_column_name: str old_db_column_name: str new_db_column_name: str + schema: t.Optional[str] = None @dataclass @@ -40,6 +42,7 @@ class AlterColumn: old_params: t.Dict[str, t.Any] column_class: t.Optional[t.Type[Column]] = None old_column_class: t.Optional[t.Type[Column]] = None + schema: t.Optional[str] = None @dataclass @@ -48,6 +51,7 @@ class DropColumn: column_name: str db_column_name: str tablename: str + schema: t.Optional[str] = None @dataclass @@ -58,3 +62,4 @@ class AddColumn: column_class_name: str column_class: t.Type[Column] params: t.Dict[str, t.Any] + schema: t.Optional[str] = None diff --git a/piccolo/apps/migrations/auto/schema_differ.py b/piccolo/apps/migrations/auto/schema_differ.py index f2a01ee3b..a8deb2e57 100644 --- a/piccolo/apps/migrations/auto/schema_differ.py +++ b/piccolo/apps/migrations/auto/schema_differ.py @@ -191,6 +191,7 @@ def check_rename_tables(self) -> RenameTableCollection: old_tablename=drop_table.tablename, new_class_name=new_table.class_name, new_tablename=new_table.tablename, + schema=new_table.schema, ) ) break @@ -212,6 +213,7 @@ def check_rename_tables(self) -> RenameTableCollection: old_tablename=drop_table.tablename, new_class_name=new_table.class_name, new_tablename=new_table.tablename, + schema=new_table.schema, ) ) break @@ -288,6 +290,7 @@ def check_renamed_columns(self) -> RenameColumnCollection: new_column_name=add_column.column_name, old_db_column_name=drop_column.db_column_name, new_db_column_name=add_column.db_column_name, + schema=add_column.schema, ) ) break @@ -516,8 +519,14 @@ def alter_columns(self) -> AlterStatements: ) ) + schema_str = ( + "None" + if alter_column.schema is None + else f'"{alter_column.schema}"' + ) + response.append( - f"manager.alter_column(table_class_name='{table.class_name}', tablename='{table.tablename}', column_name='{alter_column.column_name}', db_column_name='{alter_column.db_column_name}', params={new_params.params}, old_params={old_params.params}, column_class={column_class}, old_column_class={old_column_class})" # noqa: E501 + f"manager.alter_column(table_class_name='{table.class_name}', tablename='{table.tablename}', column_name='{alter_column.column_name}', db_column_name='{alter_column.db_column_name}', params={new_params.params}, old_params={old_params.params}, column_class={column_class}, old_column_class={old_column_class}, schema={schema_str})" # noqa: E501 ) return AlterStatements( @@ -543,8 +552,12 @@ def drop_columns(self) -> AlterStatements: ): continue + schema_str = ( + "None" if column.schema is None else f'"{column.schema}"' + ) + response.append( - f"manager.drop_column(table_class_name='{table.class_name}', tablename='{table.tablename}', column_name='{column.column_name}', db_column_name='{column.db_column_name}')" # noqa: E501 + f"manager.drop_column(table_class_name='{table.class_name}', tablename='{table.tablename}', column_name='{column.column_name}', db_column_name='{column.db_column_name}', schema={schema_str})" # noqa: E501 ) return AlterStatements(statements=response) @@ -585,8 +598,14 @@ def add_columns(self) -> AlterStatements: ) ) + schema_str = ( + "None" + if add_column.schema is None + else f'"{add_column.schema}"' + ) + response.append( - f"manager.add_column(table_class_name='{table.class_name}', tablename='{table.tablename}', column_name='{add_column.column_name}', db_column_name='{add_column.db_column_name}', column_class_name='{add_column.column_class_name}', column_class={column_class.__name__}, params={str(cleaned_params)})" # noqa: E501 + f"manager.add_column(table_class_name='{table.class_name}', tablename='{table.tablename}', column_name='{add_column.column_name}', db_column_name='{add_column.db_column_name}', column_class_name='{add_column.column_class_name}', column_class={column_class.__name__}, params={str(cleaned_params)}, schema={schema_str})" # noqa: E501 ) return AlterStatements( statements=response, @@ -647,8 +666,12 @@ def new_table_columns(self) -> AlterStatements: ) ) + schema_str = ( + "None" if table.schema is None else f'"{table.schema}"' + ) + response.append( - f"manager.add_column(table_class_name='{table.class_name}', tablename='{table.tablename}', column_name='{column._meta.name}', db_column_name='{column._meta.db_column_name}', column_class_name='{column.__class__.__name__}', column_class={column.__class__.__name__}, params={str(cleaned_params)})" # noqa: E501 + f"manager.add_column(table_class_name='{table.class_name}', tablename='{table.tablename}', column_name='{column._meta.name}', db_column_name='{column._meta.db_column_name}', column_class_name='{column.__class__.__name__}', column_class={column.__class__.__name__}, params={str(cleaned_params)}, schema={schema_str})" # noqa: E501 ) return AlterStatements( statements=response, diff --git a/tests/apps/migrations/auto/integration/test_migrations.py b/tests/apps/migrations/auto/integration/test_migrations.py index dbcdbf032..fcd4ba5ae 100644 --- a/tests/apps/migrations/auto/integration/test_migrations.py +++ b/tests/apps/migrations/auto/integration/test_migrations.py @@ -168,14 +168,14 @@ def _test_migrations( time.sleep(1e-6) if test_function: - column_name = ( - table_snapshots[-1][-1] - ._meta.non_default_columns[0] - ._meta.db_column_name - ) + column = table_snapshots[-1][-1]._meta.non_default_columns[0] + column_name = column._meta.db_column_name + schema = column._meta.table._meta.schema + tablename = column._meta.table._meta.tablename row_meta = self.get_postgres_column_definition( - tablename="my_table", + tablename=tablename, column_name=column_name, + schema=schema or "public", ) self.assertTrue( test_function(row_meta), @@ -1213,6 +1213,72 @@ def test_move_table_to_public_schema(self): ).run_sync(), ) + def test_altering_table_in_schema(self): + """ + Make sure tables in schemas can be altered. + + https://github.com/piccolo-orm/piccolo/issues/883 + + """ + self._test_migrations( + table_snapshots=[ + # Create a table with a single column + [ + create_table_class( + class_name="Manager", + class_kwargs={"schema": self.new_schema}, + class_members={"first_name": Varchar()}, + ) + ], + # Rename the column + [ + create_table_class( + class_name="Manager", + class_kwargs={"schema": self.new_schema}, + class_members={"name": Varchar()}, + ) + ], + # Add a column + [ + create_table_class( + class_name="Manager", + class_kwargs={"schema": self.new_schema}, + class_members={ + "name": Varchar(), + "age": Integer(), + }, + ) + ], + # Remove a column + [ + create_table_class( + class_name="Manager", + class_kwargs={"schema": self.new_schema}, + class_members={ + "name": Varchar(), + }, + ) + ], + # Alter a column + [ + create_table_class( + class_name="Manager", + class_kwargs={"schema": self.new_schema}, + class_members={ + "name": Varchar(length=512), + }, + ) + ], + ], + test_function=lambda x: all( + [ + x.column_name == "name", + x.data_type == "character varying", + x.character_maximum_length == 512, + ] + ), + ) + @engines_only("postgres", "cockroach") class TestSameTableName(MigrationTestCase): diff --git a/tests/apps/migrations/auto/test_schema_differ.py b/tests/apps/migrations/auto/test_schema_differ.py index 7a5f63fb0..cf621a916 100644 --- a/tests/apps/migrations/auto/test_schema_differ.py +++ b/tests/apps/migrations/auto/test_schema_differ.py @@ -46,7 +46,7 @@ def test_add_table(self): self.assertTrue(len(new_table_columns.statements) == 1) self.assertEqual( new_table_columns.statements[0], - "manager.add_column(table_class_name='Band', tablename='band', column_name='name', db_column_name='name', column_class_name='Varchar', column_class=Varchar, params={'length': 255, 'default': '', 'null': False, 'primary_key': False, 'unique': False, 'index': False, 'index_method': IndexMethod.btree, 'choices': None, 'db_column_name': None, 'secret': False})", # noqa + "manager.add_column(table_class_name='Band', tablename='band', column_name='name', db_column_name='name', column_class_name='Varchar', column_class=Varchar, params={'length': 255, 'default': '', 'null': False, 'primary_key': False, 'unique': False, 'index': False, 'index_method': IndexMethod.btree, 'choices': None, 'db_column_name': None, 'secret': False}, schema=None)", # noqa ) def test_drop_table(self): @@ -92,7 +92,7 @@ def test_rename_table(self): self.assertTrue(len(schema_differ.rename_tables.statements) == 1) self.assertEqual( schema_differ.rename_tables.statements[0], - "manager.rename_table(old_class_name='Band', old_tablename='band', new_class_name='Act', new_tablename='act')", # noqa: E501 + "manager.rename_table(old_class_name='Band', old_tablename='band', new_class_name='Act', new_tablename='act', schema=None)", # noqa: E501 ) self.assertEqual(schema_differ.create_tables.statements, []) @@ -165,7 +165,7 @@ def test_add_column(self): self.assertTrue(len(schema_differ.add_columns.statements) == 1) self.assertEqual( schema_differ.add_columns.statements[0], - "manager.add_column(table_class_name='Band', tablename='band', column_name='genre', db_column_name='genre', column_class_name='Varchar', column_class=Varchar, params={'length': 255, 'default': '', 'null': False, 'primary_key': False, 'unique': False, 'index': False, 'index_method': IndexMethod.btree, 'choices': None, 'db_column_name': None, 'secret': False})", # noqa: E501 + "manager.add_column(table_class_name='Band', tablename='band', column_name='genre', db_column_name='genre', column_class_name='Varchar', column_class=Varchar, params={'length': 255, 'default': '', 'null': False, 'primary_key': False, 'unique': False, 'index': False, 'index_method': IndexMethod.btree, 'choices': None, 'db_column_name': None, 'secret': False}, schema=None)", # noqa: E501 ) def test_drop_column(self): @@ -200,7 +200,7 @@ def test_drop_column(self): self.assertTrue(len(schema_differ.drop_columns.statements) == 1) self.assertEqual( schema_differ.drop_columns.statements[0], - "manager.drop_column(table_class_name='Band', tablename='band', column_name='genre', db_column_name='genre')", # noqa: E501 + "manager.drop_column(table_class_name='Band', tablename='band', column_name='genre', db_column_name='genre', schema=None)", # noqa: E501 ) def test_rename_column(self): @@ -238,7 +238,7 @@ def test_rename_column(self): self.assertEqual( schema_differ.rename_columns.statements, [ - "manager.rename_column(table_class_name='Band', tablename='band', old_column_name='title', new_column_name='name', old_db_column_name='title', new_db_column_name='name')" # noqa: E501 + "manager.rename_column(table_class_name='Band', tablename='band', old_column_name='title', new_column_name='name', old_db_column_name='title', new_db_column_name='name', schema=None)" # noqa: E501 ], ) @@ -249,13 +249,13 @@ def test_rename_column(self): self.assertEqual( schema_differ.add_columns.statements, [ - "manager.add_column(table_class_name='Band', tablename='band', column_name='name', db_column_name='name', column_class_name='Varchar', column_class=Varchar, params={'length': 255, 'default': '', 'null': False, 'primary_key': False, 'unique': False, 'index': False, 'index_method': IndexMethod.btree, 'choices': None, 'db_column_name': None, 'secret': False})" # noqa: E501 + "manager.add_column(table_class_name='Band', tablename='band', column_name='name', db_column_name='name', column_class_name='Varchar', column_class=Varchar, params={'length': 255, 'default': '', 'null': False, 'primary_key': False, 'unique': False, 'index': False, 'index_method': IndexMethod.btree, 'choices': None, 'db_column_name': None, 'secret': False}, schema=None)" # noqa: E501 ], ) self.assertEqual( schema_differ.drop_columns.statements, [ - "manager.drop_column(table_class_name='Band', tablename='band', column_name='title', db_column_name='title')" # noqa: E501 + "manager.drop_column(table_class_name='Band', tablename='band', column_name='title', db_column_name='title', schema=None)" # noqa: E501 ], ) self.assertTrue(schema_differ.rename_columns.statements == []) @@ -319,8 +319,8 @@ def mock_input(value: str): self.assertEqual( schema_differ.rename_columns.statements, [ - "manager.rename_column(table_class_name='Band', tablename='band', old_column_name='a1', new_column_name='a2', old_db_column_name='a1', new_db_column_name='a2')", # noqa: E501 - "manager.rename_column(table_class_name='Band', tablename='band', old_column_name='b1', new_column_name='b2', old_db_column_name='b1', new_db_column_name='b2')", # noqa: E501 + "manager.rename_column(table_class_name='Band', tablename='band', old_column_name='a1', new_column_name='a2', old_db_column_name='a1', new_db_column_name='a2', schema=None)", # noqa: E501 + "manager.rename_column(table_class_name='Band', tablename='band', old_column_name='b1', new_column_name='b2', old_db_column_name='b1', new_db_column_name='b2', schema=None)", # noqa: E501 ], ) @@ -391,19 +391,19 @@ def mock_input(value: str): self.assertEqual( schema_differ.add_columns.statements, [ - "manager.add_column(table_class_name='Band', tablename='band', column_name='b2', db_column_name='b2', column_class_name='Varchar', column_class=Varchar, params={'length': 255, 'default': '', 'null': False, 'primary_key': False, 'unique': False, 'index': False, 'index_method': IndexMethod.btree, 'choices': None, 'db_column_name': None, 'secret': False})" # noqa: E501 + "manager.add_column(table_class_name='Band', tablename='band', column_name='b2', db_column_name='b2', column_class_name='Varchar', column_class=Varchar, params={'length': 255, 'default': '', 'null': False, 'primary_key': False, 'unique': False, 'index': False, 'index_method': IndexMethod.btree, 'choices': None, 'db_column_name': None, 'secret': False}, schema=None)" # noqa: E501 ], ) self.assertEqual( schema_differ.drop_columns.statements, [ - "manager.drop_column(table_class_name='Band', tablename='band', column_name='b1', db_column_name='b1')" # noqa: E501 + "manager.drop_column(table_class_name='Band', tablename='band', column_name='b1', db_column_name='b1', schema=None)" # noqa: E501 ], ) self.assertEqual( schema_differ.rename_columns.statements, [ - "manager.rename_column(table_class_name='Band', tablename='band', old_column_name='a1', new_column_name='a2', old_db_column_name='a1', new_db_column_name='a2')", # noqa: E501 + "manager.rename_column(table_class_name='Band', tablename='band', old_column_name='a1', new_column_name='a2', old_db_column_name='a1', new_db_column_name='a2', schema=None)", # noqa: E501 ], ) @@ -448,7 +448,7 @@ def test_alter_column_precision(self): self.assertTrue(len(schema_differ.alter_columns.statements) == 1) self.assertEqual( schema_differ.alter_columns.statements[0], - "manager.alter_column(table_class_name='Ticket', tablename='ticket', column_name='price', db_column_name='price', params={'digits': (4, 2)}, old_params={'digits': (5, 2)}, column_class=Numeric, old_column_class=Numeric)", # noqa + "manager.alter_column(table_class_name='Ticket', tablename='ticket', column_name='price', db_column_name='price', params={'digits': (4, 2)}, old_params={'digits': (5, 2)}, column_class=Numeric, old_column_class=Numeric, schema=None)", # noqa ) def test_db_column_name(self): @@ -486,7 +486,7 @@ def test_db_column_name(self): self.assertTrue(len(schema_differ.alter_columns.statements) == 1) self.assertEqual( schema_differ.alter_columns.statements[0], - "manager.alter_column(table_class_name='Ticket', tablename='ticket', column_name='price', db_column_name='custom', params={'digits': (4, 2)}, old_params={'digits': (5, 2)}, column_class=Numeric, old_column_class=Numeric)", # noqa + "manager.alter_column(table_class_name='Ticket', tablename='ticket', column_name='price', db_column_name='custom', params={'digits': (4, 2)}, old_params={'digits': (5, 2)}, column_class=Numeric, old_column_class=Numeric, schema=None)", # noqa ) def test_alter_default(self): diff --git a/tests/base.py b/tests/base.py index 6978aba98..6f9bfbb55 100644 --- a/tests/base.py +++ b/tests/base.py @@ -174,16 +174,18 @@ def table_exists(self, tablename: str) -> bool: # Postgres specific utils def get_postgres_column_definition( - self, tablename: str, column_name: str + self, tablename: str, column_name: str, schema: str = "public" ) -> RowMeta: query = """ SELECT {columns} FROM information_schema.columns WHERE table_name = '{tablename}' AND table_catalog = 'piccolo' + AND table_schema = '{schema}' AND column_name = '{column_name}' """.format( columns=RowMeta.get_column_name_str(), tablename=tablename, + schema=schema, column_name=column_name, ) response = self.run_sync(query)