From e14fe2bf8242de5cb7d6bdda666b13120978d57c Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Mon, 29 Jun 2026 21:38:12 -0300 Subject: [PATCH 01/11] refactor(table): extract _ColumnGroups to reduce TableMeta instance attributes (R0902) --- piccolo/table.py | 97 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 80 insertions(+), 17 deletions(-) diff --git a/piccolo/table.py b/piccolo/table.py index 8ff955d59..00a701596 100644 --- a/piccolo/table.py +++ b/piccolo/table.py @@ -69,12 +69,14 @@ @dataclass -class TableMeta: +class _ColumnGroups: # pylint: disable=too-many-instance-attributes """ - This is used to store info about the table. + Groups all column-related attributes for a Table. + + This reduces the number of instance attributes on TableMeta, keeping + the Pylint too-many-instance-attributes warning under control. """ - tablename: str = "" columns: list[Column] = field(default_factory=list) default_columns: list[Column] = field(default_factory=list) non_default_columns: list[Column] = field(default_factory=list) @@ -85,17 +87,76 @@ class TableMeta: json_columns: list[Union[JSON, JSONB]] = field(default_factory=list) secret_columns: list[Column] = field(default_factory=list) auto_update_columns: list[Column] = field(default_factory=list) + m2m_relationships: list[M2M] = field(default_factory=list) + + +@dataclass +class TableMeta: + """ + This is used to store info about the table. + """ + + tablename: str = "" tags: list[str] = field(default_factory=list) help_text: Optional[str] = None - _db: Optional[Engine] = None - m2m_relationships: list[M2M] = field(default_factory=list) schema: Optional[str] = None + _db: Optional[Engine] = None # Records reverse foreign key relationships - i.e. when the current table # is the target of a foreign key. Used by external libraries such as # Piccolo API. _foreign_key_references: list[ForeignKey] = field(default_factory=list) + _column_groups: _ColumnGroups = field(default_factory=_ColumnGroups) + + # ------------------------------------------------------------------ # + # Backward-compatible properties delegating to _column_groups + # ------------------------------------------------------------------ # + + @property + def columns(self) -> list[Column]: + return self._column_groups.columns + + @property + def default_columns(self) -> list[Column]: + return self._column_groups.default_columns + + @property + def non_default_columns(self) -> list[Column]: + return self._column_groups.non_default_columns + + @property + def array_columns(self) -> list[Array]: + return self._column_groups.array_columns + + @property + def email_columns(self) -> list[Email]: + return self._column_groups.email_columns + + @property + def foreign_key_columns(self) -> list[ForeignKey]: + return self._column_groups.foreign_key_columns + + @property + def primary_key(self) -> Column: + return self._column_groups.primary_key + + @property + def json_columns(self) -> list[Union[JSON, JSONB]]: + return self._column_groups.json_columns + + @property + def secret_columns(self) -> list[Column]: + return self._column_groups.secret_columns + + @property + def auto_update_columns(self) -> list[Column]: + return self._column_groups.auto_update_columns + + @property + def m2m_relationships(self) -> list[M2M]: + return self._column_groups.m2m_relationships + def get_formatted_tablename( self, include_schema: bool = True, quoted: bool = True ) -> str: @@ -147,7 +208,7 @@ def refresh_db(self) -> None: engine = engine_finder() if engine is None: raise ValueError("The engine can't be found") - self.db = engine + self._db = engine def get_column_by_name( self, @@ -360,21 +421,23 @@ def __init_subclass__( cls._meta = TableMeta( tablename=tablename, - columns=columns, - default_columns=default_columns, - non_default_columns=non_default_columns, - array_columns=array_columns, - email_columns=email_columns, - primary_key=primary_key, - foreign_key_columns=foreign_key_columns, - json_columns=json_columns, - secret_columns=secret_columns, - auto_update_columns=auto_update_columns, tags=tags, help_text=help_text, _db=db, - m2m_relationships=m2m_relationships, schema=schema, + _column_groups=_ColumnGroups( + columns=columns, + default_columns=default_columns, + non_default_columns=non_default_columns, + array_columns=array_columns, + email_columns=email_columns, + foreign_key_columns=foreign_key_columns, + primary_key=primary_key, + json_columns=json_columns, + secret_columns=secret_columns, + auto_update_columns=auto_update_columns, + m2m_relationships=m2m_relationships, + ), ) for foreign_key_column in foreign_key_columns: From 5b816dbfa3516118328f2d40b494471b9b7c6c2d Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Mon, 29 Jun 2026 22:09:30 -0300 Subject: [PATCH 02/11] refactor: group numeric fields into NumericInfo to reduce RowMeta instance attributes (R0902) --- piccolo/apps/schema/commands/generate.py | 43 +++++++++++++++++++++--- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/piccolo/apps/schema/commands/generate.py b/piccolo/apps/schema/commands/generate.py index d4e94ef16..08dedd0d2 100644 --- a/piccolo/apps/schema/commands/generate.py +++ b/piccolo/apps/schema/commands/generate.py @@ -58,6 +58,13 @@ class ConstraintTable: schema: str = "" +@dataclasses.dataclass +class NumericInfo: + numeric_precision: Optional[Union[int, str]] + numeric_scale: Optional[Union[int, str]] + numeric_precision_radix: Optional[Literal[2, 10]] + + @dataclasses.dataclass class RowMeta: column_default: str @@ -66,13 +73,41 @@ class RowMeta: table_name: str character_maximum_length: Optional[int] data_type: str - numeric_precision: Optional[Union[int, str]] - numeric_scale: Optional[Union[int, str]] - numeric_precision_radix: Optional[Literal[2, 10]] + numeric_info: Optional[NumericInfo] = None + numeric_precision: dataclasses.InitVar[Optional[Union[int, str]]] = None + numeric_scale: dataclasses.InitVar[Optional[Union[int, str]]] = None + numeric_precision_radix: dataclasses.InitVar[Optional[Literal[2, 10]]] = None + + def __post_init__( + self, + numeric_precision: Optional[Union[int, str]], + numeric_scale: Optional[Union[int, str]], + numeric_precision_radix: Optional[Literal[2, 10]], + ) -> None: + if self.numeric_info is None: + self.numeric_info = NumericInfo( + numeric_precision=numeric_precision, + numeric_scale=numeric_scale, + numeric_precision_radix=numeric_precision_radix, + ) + + @property + def numeric_precision(self) -> Optional[Union[int, str]]: + return self.numeric_info.numeric_precision if self.numeric_info else None + + @property + def numeric_scale(self) -> Optional[Union[int, str]]: + return self.numeric_info.numeric_scale if self.numeric_info else None + + @property + def numeric_precision_radix(self) -> Optional[Literal[2, 10]]: + return self.numeric_info.numeric_precision_radix if self.numeric_info else None @classmethod def get_column_name_str(cls) -> str: - return ", ".join(i.name for i in dataclasses.fields(cls)) + return ", ".join( + i.name for i in dataclasses.fields(cls) if i.name != "numeric_info" + ) @dataclasses.dataclass From a5a9aa05c027f5281c0d2dd2d0d705a154c1fd45 Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Mon, 29 Jun 2026 22:27:42 -0300 Subject: [PATCH 03/11] refactor: group foreign key fields into ForeignKeyAction to reduce Trigger instance attributes (R0902) --- piccolo/apps/schema/commands/generate.py | 37 ++++++++++++++++++------ 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/piccolo/apps/schema/commands/generate.py b/piccolo/apps/schema/commands/generate.py index 08dedd0d2..42dbe82a4 100644 --- a/piccolo/apps/schema/commands/generate.py +++ b/piccolo/apps/schema/commands/generate.py @@ -171,11 +171,7 @@ def get_foreign_key_constraint_name(self, column_name) -> ConstraintTable: @dataclasses.dataclass -class Trigger: - constraint_name: str - constraint_type: str - table_name: str - column_name: str +class ForeignKeyAction: on_update: str on_delete: Literal[ "NO ACTION", "RESTRICT", "CASCADE", "SET NULL", "SET_DEFAULT" @@ -184,6 +180,15 @@ class Trigger: references_column: str +@dataclasses.dataclass +class Trigger: + constraint_name: str + constraint_type: str + table_name: str + column_name: str + action: ForeignKeyAction + + @dataclasses.dataclass class TableTriggers: """ @@ -202,7 +207,7 @@ def get_column_ref_trigger( for trigger in self.triggers: if ( trigger.column_name == column_name - and trigger.references_table == references_table + and trigger.action.references_table == references_table ): return trigger @@ -573,7 +578,21 @@ async def get_fk_triggers( ) return TableTriggers( tablename=tablename, - triggers=[Trigger(**i) for i in triggers], + triggers=[ + Trigger( + constraint_name=i["constraint_name"], + constraint_type=i["constraint_type"], + table_name=i["table_name"], + column_name=i["column_name"], + action=ForeignKeyAction( + on_update=i["on_update"], + on_delete=i["on_delete"], + references_table=i["references_table"], + references_column=i["references_column"], + ), + ) + for i in triggers + ], ) @@ -788,8 +807,8 @@ async def create_table_class_from_db( column_name, constraint_table.name ) if trigger: - kwargs["on_update"] = OnUpdate(trigger.on_update) - kwargs["on_delete"] = OnDelete(trigger.on_delete) + kwargs["on_update"] = OnUpdate(trigger.action.on_update) + kwargs["on_delete"] = OnDelete(trigger.action.on_delete) else: output_schema.trigger_warnings.append( f"{tablename}.{column_name}" From 5fb5c501ae80b294fccf81cb5ed563febefc66b1 Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Mon, 29 Jun 2026 23:16:11 -0300 Subject: [PATCH 04/11] refactor: group MigrationManager attributes into MigrationMeta and MigrationOperations with backward-compatible constructor (R0902) --- .../apps/migrations/auto/migration_manager.py | 165 ++++++++++++------ .../apps/migrations/auto/schema_snapshot.py | 20 ++- piccolo/apps/migrations/commands/backwards.py | 2 +- piccolo/apps/migrations/commands/base.py | 2 +- piccolo/apps/migrations/commands/forwards.py | 4 +- .../auto/integration/test_migrations.py | 2 +- .../migrations/auto/test_migration_manager.py | 10 +- tests/apps/migrations/commands/test_base.py | 2 +- 8 files changed, 134 insertions(+), 73 deletions(-) diff --git a/piccolo/apps/migrations/auto/migration_manager.py b/piccolo/apps/migrations/auto/migration_manager.py index 57b97fa2d..9c59bd9f5 100644 --- a/piccolo/apps/migrations/auto/migration_manager.py +++ b/piccolo/apps/migrations/auto/migration_manager.py @@ -138,28 +138,37 @@ async def __aexit__(self, *args, **kwargs): @dataclass -class MigrationManager: +class MigrationMeta: """ - Each auto generated migration returns a MigrationManager. It contains - all of the schema changes that migration wants to make. - - :param wrap_in_transaction: - By default, the migration is wrapped in a transaction, so if anything - fails, the whole migration will get rolled back. You can disable this - behaviour if you want - for example, in a manual migration you might - want to create the transaction yourself (perhaps you're using - savepoints), or you may want multiple transactions. - + Metadata and configuration for a migration. """ migration_id: str = "" app_name: str = "" description: str = "" preview: bool = False + fake: bool = False + wrap_in_transaction: bool = True + + +@dataclass +class TableOperations: + """ + Table level schema changes. + """ + add_tables: list[DiffableTable] = field(default_factory=list) drop_tables: list[DiffableTable] = field(default_factory=list) rename_tables: list[RenameTable] = field(default_factory=list) change_table_schemas: list[ChangeTableSchema] = field(default_factory=list) + + +@dataclass +class ColumnOperations: + """ + Column level schema changes. + """ + add_columns: AddColumnCollection = field( default_factory=AddColumnCollection ) @@ -172,12 +181,62 @@ class MigrationManager: alter_columns: AlterColumnCollection = field( default_factory=AlterColumnCollection ) + + +@dataclass +class MigrationOperations: + """ + The schema changes that a migration wants to make. + """ + + table_operations: TableOperations = field(default_factory=TableOperations) + column_operations: ColumnOperations = field( + default_factory=ColumnOperations + ) raw: list[Union[Callable, AsyncFunction]] = field(default_factory=list) raw_backwards: list[Union[Callable, AsyncFunction]] = field( default_factory=list ) - fake: bool = False - wrap_in_transaction: bool = True + + +@dataclass +class MigrationManager: + """ + Each auto generated migration returns a MigrationManager. It contains + all of the schema changes that migration wants to make. + """ + + meta: MigrationMeta = field(default_factory=MigrationMeta) + operations: MigrationOperations = field( + default_factory=MigrationOperations + ) + + def __init__( + self, + *, + meta: Optional[MigrationMeta] = None, + operations: Optional[MigrationOperations] = None, + migration_id: str = "", + app_name: str = "", + description: str = "", + preview: bool = False, + fake: bool = False, + wrap_in_transaction: bool = True, + ): + if meta is not None: + self.meta = meta + else: + self.meta = MigrationMeta( + migration_id=migration_id, + app_name=app_name, + description=description, + preview=preview, + fake=fake, + wrap_in_transaction=wrap_in_transaction, + ) + self.operations = ( + operations if operations is not None else MigrationOperations() + ) def add_table( self, @@ -189,7 +248,7 @@ def add_table( if not columns: columns = [] - self.add_tables.append( + self.operations.table_operations.add_tables.append( DiffableTable( class_name=class_name, tablename=tablename, @@ -201,7 +260,7 @@ def add_table( def drop_table( self, class_name: str, tablename: str, schema: Optional[str] = None ): - self.drop_tables.append( + self.operations.table_operations.drop_tables.append( DiffableTable( class_name=class_name, tablename=tablename, schema=schema ) @@ -214,7 +273,7 @@ def change_table_schema( new_schema: Optional[str] = None, old_schema: Optional[str] = None, ): - self.change_table_schemas.append( + self.operations.table_operations.change_table_schemas.append( ChangeTableSchema( class_name=class_name, tablename=tablename, @@ -231,7 +290,7 @@ def rename_table( new_tablename: str, schema: Optional[str] = None, ): - self.rename_tables.append( + self.operations.table_operations.rename_tables.append( RenameTable( old_class_name=old_class_name, old_tablename=old_tablename, @@ -277,7 +336,7 @@ def add_column( if db_column_name: column._meta.db_column_name = db_column_name - self.add_columns.append( + self.operations.column_operations.add_columns.append( AddColumnClass( column=column, tablename=tablename, @@ -294,7 +353,7 @@ def drop_column( db_column_name: Optional[str] = None, schema: Optional[str] = None, ): - self.drop_columns.append( + self.operations.column_operations.drop_columns.append( DropColumn( table_class_name=table_class_name, column_name=column_name, @@ -314,7 +373,7 @@ def rename_column( new_db_column_name: Optional[str] = None, schema: Optional[str] = None, ): - self.rename_columns.append( + self.operations.column_operations.rename_columns.append( RenameColumn( table_class_name=table_class_name, tablename=tablename, @@ -345,7 +404,7 @@ def alter_column( params = {} if old_params is None: old_params = {} - self.alter_columns.append( + self.operations.column_operations.alter_columns.append( AlterColumn( table_class_name=table_class_name, tablename=tablename, @@ -364,14 +423,14 @@ def add_raw(self, raw: Union[Callable, AsyncFunction]): A migration manager can execute arbitrary functions or coroutines when run. This is useful if you want to execute raw SQL. """ - self.raw.append(raw) + self.operations.raw.append(raw) def add_raw_backwards(self, raw: Union[Callable, AsyncFunction]): """ When reversing a migration, you may want to run extra code to help clean up. """ - self.raw_backwards.append(raw) + self.operations.raw_backwards.append(raw) ########################################################################### @@ -394,10 +453,10 @@ async def get_table_from_snapshot( from piccolo.apps.migrations.commands.base import BaseMigrationManager if migration_id is None: - migration_id = self.migration_id + migration_id = self.meta.migration_id if app_name is None: - app_name = self.app_name + app_name = self.meta.app_name diffable_table = await BaseMigrationManager().get_table_from_snapshot( app_name=app_name, @@ -421,14 +480,14 @@ async def _run_query(self, query: Union[DDL, Query, SchemaDDLBase]): If MigrationManager is in preview mode then it just print the query instead of executing it. """ - if self.preview: + if self.meta.preview: await self._print_query(query) else: await query.run() async def _run_alter_columns(self, backwards: bool = False): - for table_class_name in self.alter_columns.table_class_names: - alter_columns = self.alter_columns.for_table_class_name( + for table_class_name in self.operations.column_operations.alter_columns.table_class_names: + alter_columns = self.operations.column_operations.alter_columns.for_table_class_name( table_class_name ) @@ -534,7 +593,7 @@ async def _run_alter_columns(self, backwards: bool = False): if on_delete is not None or on_update is not None: existing_table = await self.get_table_from_snapshot( table_class_name=table_class_name, - app_name=self.app_name, + app_name=self.meta.app_name, ) fk_column = existing_table._meta.get_column_by_name( @@ -665,11 +724,11 @@ async def _run_alter_columns(self, backwards: bool = False): ) async def _run_drop_tables(self, backwards=False): - for diffable_table in self.drop_tables: + for diffable_table in self.operations.table_operations.drop_tables: if backwards: _Table = await self.get_table_from_snapshot( table_class_name=diffable_table.class_name, - app_name=self.app_name, + app_name=self.meta.app_name, offset=-1, ) await self._run_query(_Table.create_table()) @@ -680,10 +739,10 @@ async def _run_drop_tables(self, backwards=False): async def _run_drop_columns(self, backwards: bool = False): if backwards: - for drop_column in self.drop_columns.drop_columns: + for drop_column in self.operations.column_operations.drop_columns.drop_columns: _Table = await self.get_table_from_snapshot( table_class_name=drop_column.table_class_name, - app_name=self.app_name, + app_name=self.meta.app_name, offset=-1, ) column_to_restore = _Table._meta.get_column_by_name( @@ -695,8 +754,8 @@ async def _run_drop_columns(self, backwards: bool = False): ) ) else: - for table_class_name in self.drop_columns.table_class_names: - columns = self.drop_columns.for_table_class_name( + for table_class_name in self.operations.column_operations.drop_columns.table_class_names: + columns = self.operations.column_operations.drop_columns.for_table_class_name( table_class_name ) @@ -717,7 +776,7 @@ async def _run_drop_columns(self, backwards: bool = False): ) async def _run_rename_tables(self, backwards: bool = False): - for rename_table in self.rename_tables: + for rename_table in self.operations.table_operations.rename_tables: class_name = ( rename_table.new_class_name if backwards @@ -747,8 +806,8 @@ async def _run_rename_tables(self, backwards: bool = False): ) async def _run_rename_columns(self, backwards: bool = False): - for table_class_name in self.rename_columns.table_class_names: - columns = self.rename_columns.for_table_class_name( + for table_class_name in self.operations.column_operations.rename_columns.table_class_names: + columns = self.operations.column_operations.rename_columns.for_table_class_name( table_class_name ) @@ -784,9 +843,9 @@ async def _run_rename_columns(self, backwards: bool = False): async def _run_add_tables(self, backwards: bool = False): table_classes: list[type[Table]] = [] - for add_table in self.add_tables: + for add_table in self.operations.table_operations.add_tables: add_columns: list[AddColumnClass] = ( - self.add_columns.for_table_class_name(add_table.class_name) + self.operations.column_operations.add_columns.for_table_class_name(add_table.class_name) ) _Table: type[Table] = create_table_class( class_name=add_table.class_name, @@ -816,9 +875,9 @@ async def _run_add_columns(self, backwards: bool = False): Add columns, which belong to existing tables """ if backwards: - for add_column in self.add_columns.add_columns: + for add_column in self.operations.column_operations.add_columns.add_columns: if add_column.table_class_name in [ - i.class_name for i in self.add_tables + i.class_name for i in self.operations.table_operations.add_tables ]: # Don't reverse the add column as the table is going to # be deleted. @@ -836,12 +895,12 @@ async def _run_add_columns(self, backwards: bool = False): _Table.alter().drop_column(add_column.column) ) else: - for table_class_name in self.add_columns.table_class_names: - if table_class_name in [i.class_name for i in self.add_tables]: + for table_class_name in self.operations.column_operations.add_columns.table_class_names: + if table_class_name in [i.class_name for i in self.operations.table_operations.add_tables]: continue # No need to add columns to new tables add_columns: list[AddColumnClass] = ( - self.add_columns.for_table_class_name(table_class_name) + self.operations.column_operations.add_columns.for_table_class_name(table_class_name) ) ############################################################### @@ -874,7 +933,7 @@ async def _run_add_columns(self, backwards: bool = False): existing_table = ( await self.get_table_from_snapshot( table_class_name=table_class_name, - app_name=self.app_name, + app_name=self.meta.app_name, offset=-1, ) ) @@ -925,7 +984,7 @@ async def _run_change_table_schema(self, backwards: bool = False): schema_manager = SchemaManager() - for change_table_schema in self.change_table_schemas: + for change_table_schema in self.operations.table_operations.change_table_schemas: if backwards: # Note, we don't try dropping any schemas we may have created. # It's dangerous to do so, just in case the user manually @@ -972,9 +1031,9 @@ async def _run_change_table_schema(self, backwards: bool = False): async def run(self, backwards: bool = False): direction = "backwards" if backwards else "forwards" - if self.preview: + if self.meta.preview: direction = "preview " + direction - print(f" - {self.migration_id} [{direction}]... ", end="") + print(f" - {self.meta.migration_id} [{direction}]... ", end="") engine = engine_finder() @@ -983,18 +1042,18 @@ async def run(self, backwards: bool = False): async with ( engine.transaction() - if self.wrap_in_transaction + if self.meta.wrap_in_transaction else SkippedTransaction() ) as transaction: if isinstance(transaction, CockroachTransaction): # To enable DDL rollbacks in CockroachDB. await transaction.autocommit_before_ddl(enabled=False) - if not self.preview: + if not self.meta.preview: if direction == "backwards": - raw_list = self.raw_backwards + raw_list = self.operations.raw_backwards else: - raw_list = self.raw + raw_list = self.operations.raw for raw in raw_list: if inspect.iscoroutinefunction(raw): diff --git a/piccolo/apps/migrations/auto/schema_snapshot.py b/piccolo/apps/migrations/auto/schema_snapshot.py index 5bf343063..73ba00b3f 100644 --- a/piccolo/apps/migrations/auto/schema_snapshot.py +++ b/piccolo/apps/migrations/auto/schema_snapshot.py @@ -31,39 +31,41 @@ def get_snapshot(self) -> list[DiffableTable]: tables: list[DiffableTable] = [] # Make sure the managers are sorted correctly: - sorted_managers = sorted(self.managers, key=lambda x: x.migration_id) + sorted_managers = sorted( + self.managers, key=lambda x: x.meta.migration_id + ) for manager in sorted_managers: - for table in manager.add_tables: + for table in manager.operations.table_operations.add_tables: tables.append(table) - for drop_table in manager.drop_tables: + for drop_table in manager.operations.table_operations.drop_tables: tables = [ i for i in tables if i.class_name != drop_table.class_name ] - for rename_table in manager.rename_tables: + for rename_table in manager.operations.table_operations.rename_tables: for table in tables: if table.class_name == rename_table.old_class_name: table.class_name = rename_table.new_class_name table.tablename = rename_table.new_tablename break - for change_table_schema in manager.change_table_schemas: + for change_table_schema in manager.operations.table_operations.change_table_schemas: for table in tables: if table.tablename == change_table_schema.tablename: table.schema = change_table_schema.new_schema break for table in tables: - add_columns = manager.add_columns.columns_for_table_class_name( + add_columns = manager.operations.column_operations.add_columns.columns_for_table_class_name( table.class_name ) table.columns.extend(add_columns) ############################################################### - drop_columns = manager.drop_columns.for_table_class_name( + drop_columns = manager.operations.column_operations.drop_columns.for_table_class_name( table.class_name ) for drop_column in drop_columns: @@ -75,7 +77,7 @@ def get_snapshot(self) -> list[DiffableTable]: ############################################################### - alter_columns = manager.alter_columns.for_table_class_name( + alter_columns = manager.operations.column_operations.alter_columns.for_table_class_name( table.class_name ) for alter_column in alter_columns: @@ -101,7 +103,7 @@ def get_snapshot(self) -> list[DiffableTable]: for ( rename_column - ) in manager.rename_columns.for_table_class_name( + ) in manager.operations.column_operations.rename_columns.for_table_class_name( table.class_name ): for column in table.columns: diff --git a/piccolo/apps/migrations/commands/backwards.py b/piccolo/apps/migrations/commands/backwards.py index 25b72e8e3..366641cd9 100644 --- a/piccolo/apps/migrations/commands/backwards.py +++ b/piccolo/apps/migrations/commands/backwards.py @@ -90,7 +90,7 @@ async def run_migrations_backwards(self, app_config: AppConfig): if isinstance(response, MigrationManager): if self.preview: - response.preview = True + response.meta.preview = True await response.run(backwards=True) if not self.preview: await Migration.delete().where( diff --git a/piccolo/apps/migrations/commands/base.py b/piccolo/apps/migrations/commands/base.py index 5a4e9615d..c9d6fccfa 100644 --- a/piccolo/apps/migrations/commands/base.py +++ b/piccolo/apps/migrations/commands/base.py @@ -101,7 +101,7 @@ async def get_migration_managers( migration_managers.append(response) if ( max_migration_id - and response.migration_id == max_migration_id + and response.meta.migration_id == max_migration_id ): break diff --git a/piccolo/apps/migrations/commands/forwards.py b/piccolo/apps/migrations/commands/forwards.py index adc7e657d..eab88e1d3 100644 --- a/piccolo/apps/migrations/commands/forwards.py +++ b/piccolo/apps/migrations/commands/forwards.py @@ -75,11 +75,11 @@ async def run_migrations(self, app_config: AppConfig) -> MigrationResult: response = await migration_module.forwards() if isinstance(response, MigrationManager): - if self.fake or response.fake: + if self.fake or response.meta.fake: print(f"- {_id}: faked! ⏭️") else: if self.preview: - response.preview = True + response.meta.preview = True await response.run() print("ok! ✔️") diff --git a/tests/apps/migrations/auto/integration/test_migrations.py b/tests/apps/migrations/auto/integration/test_migrations.py index 726152ea1..ee5d8b9b7 100644 --- a/tests/apps/migrations/auto/integration/test_migrations.py +++ b/tests/apps/migrations/auto/integration/test_migrations.py @@ -1535,7 +1535,7 @@ def test_rename_table(self): migration_managers = self._get_migration_managers() self.assertListEqual( - migration_managers[-1].rename_tables, + migration_managers[-1].operations.table_operations.rename_tables, [ RenameTable( old_class_name="Manager", diff --git a/tests/apps/migrations/auto/test_migration_manager.py b/tests/apps/migrations/auto/test_migration_manager.py index 0952e1895..b49674eea 100644 --- a/tests/apps/migrations/auto/test_migration_manager.py +++ b/tests/apps/migrations/auto/test_migration_manager.py @@ -144,7 +144,7 @@ def test_rename_column(self): self.assertTrue("name" in response[0].keys()) # Preview - manager.preview = True + manager.meta.preview = True with patch("sys.stdout", new=StringIO()) as fake_out: asyncio.run(manager.run()) self.assertEqual( @@ -254,7 +254,7 @@ def test_add_table(self, get_app_config: MagicMock): self.run_sync("DROP TABLE IF EXISTS musician;") # Preview - manager.preview = True + manager.meta.preview = True with patch("sys.stdout", new=StringIO()) as fake_out: asyncio.run(manager.run()) @@ -324,7 +324,7 @@ def test_add_column(self) -> None: self.assertEqual(response, [{"id": row_id, "name": "Dave"}]) # Preview - manager.preview = True + manager.meta.preview = True with patch("sys.stdout", new=StringIO()) as fake_out: asyncio.run(manager.run()) self.assertEqual( @@ -369,7 +369,7 @@ def test_add_column_with_index(self): self.assertTrue(index_name not in Manager.indexes().run_sync()) # Preview - manager.preview = True + manager.meta.preview = True with patch("sys.stdout", new=StringIO()) as fake_out: asyncio.run(manager.run()) self.assertEqual( @@ -1088,7 +1088,7 @@ def test_change_table_schema(self): ) # Preview - manager.preview = True + manager.meta.preview = True with patch("sys.stdout", new=StringIO()) as fake_out: asyncio.run(manager.run()) diff --git a/tests/apps/migrations/commands/test_base.py b/tests/apps/migrations/commands/test_base.py index 25d1b9494..6cd9adae2 100644 --- a/tests/apps/migrations/commands/test_base.py +++ b/tests/apps/migrations/commands/test_base.py @@ -37,7 +37,7 @@ def test_get_migration_modules(self): self.assertTrue(len(migration_managers) == 1) self.assertTrue( - migration_managers[0].migration_id == "2020-03-31T20:38:22" + migration_managers[0].meta.migration_id == "2020-03-31T20:38:22" ) From 24f80f3378db1a27a0d6bc726274336ce4730f25 Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Mon, 29 Jun 2026 23:36:16 -0300 Subject: [PATCH 05/11] refactor: extract TableRef and ColumnRef from AlterColumn to reduce instance attributes (R0902) --- .../apps/migrations/auto/diffable_table.py | 16 +++-- .../apps/migrations/auto/migration_manager.py | 58 ++++++++++--------- piccolo/apps/migrations/auto/operations.py | 20 +++++-- piccolo/apps/migrations/auto/schema_differ.py | 6 +- .../apps/migrations/auto/schema_snapshot.py | 2 +- 5 files changed, 62 insertions(+), 40 deletions(-) diff --git a/piccolo/apps/migrations/auto/diffable_table.py b/piccolo/apps/migrations/auto/diffable_table.py index a3e80500f..3b76301e5 100644 --- a/piccolo/apps/migrations/auto/diffable_table.py +++ b/piccolo/apps/migrations/auto/diffable_table.py @@ -6,7 +6,9 @@ from piccolo.apps.migrations.auto.operations import ( AddColumn, AlterColumn, + ColumnRef, DropColumn, + TableRef, ) from piccolo.apps.migrations.auto.serialisation import ( deserialise_params, @@ -184,15 +186,19 @@ def __sub__(self, value: DiffableTable) -> TableDelta: if delta or (column.__class__ != existing_column.__class__): alter_columns.append( AlterColumn( - table_class_name=self.class_name, - tablename=self.tablename, - column_name=column._meta.name, - db_column_name=column._meta.db_column_name, + table=TableRef( + class_name=self.class_name, + tablename=self.tablename, + schema=self.schema, + ), + column=ColumnRef( + name=column._meta.name, + db_column_name=column._meta.db_column_name, + ), params=deserialise_params(delta), 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 9c59bd9f5..046d871dd 100644 --- a/piccolo/apps/migrations/auto/migration_manager.py +++ b/piccolo/apps/migrations/auto/migration_manager.py @@ -10,9 +10,11 @@ from piccolo.apps.migrations.auto.operations import ( AlterColumn, ChangeTableSchema, + ColumnRef, DropColumn, RenameColumn, RenameTable, + TableRef, ) from piccolo.apps.migrations.auto.serialisation import deserialise_params from piccolo.columns import Column, column_types @@ -118,12 +120,12 @@ def for_table_class_name(self, table_class_name: str) -> list[AlterColumn]: return [ i for i in self.alter_columns - if i.table_class_name == table_class_name + if i.table.class_name == table_class_name ] @property def table_class_names(self) -> list[str]: - return list({i.table_class_name for i in self.alter_columns}) + return list({i.table.class_name for i in self.alter_columns}) AsyncFunction = Callable[[], Coroutine] @@ -406,15 +408,19 @@ def alter_column( old_params = {} self.operations.column_operations.alter_columns.append( AlterColumn( - table_class_name=table_class_name, - tablename=tablename, - column_name=column_name, - db_column_name=db_column_name or column_name, + table=TableRef( + class_name=table_class_name, + tablename=tablename, + schema=schema, + ), + column=ColumnRef( + name=column_name, + db_column_name=db_column_name or column_name, + ), params=params, old_params=old_params, column_class=column_class, old_column_class=old_column_class, - schema=schema, ) ) @@ -497,8 +503,8 @@ async def _run_alter_columns(self, backwards: bool = False): _Table: type[Table] = create_table_class( class_name=table_class_name, class_kwargs={ - "tablename": alter_columns[0].tablename, - "schema": alter_columns[0].schema, + "tablename": alter_columns[0].table.tablename, + "schema": alter_columns[0].table.schema, }, ) @@ -535,16 +541,16 @@ async def _run_alter_columns(self, backwards: bool = False): if old_column_class != column_class: old_column = old_column_class(**old_params) old_column._meta._table = _Table - old_column._meta._name = alter_column.column_name + old_column._meta._name = alter_column.column.name old_column._meta.db_column_name = ( - alter_column.db_column_name + alter_column.column.db_column_name ) new_column = column_class(**params) new_column._meta._table = _Table - new_column._meta._name = alter_column.column_name + new_column._meta._name = alter_column.column.name new_column._meta.db_column_name = ( - alter_column.db_column_name + alter_column.column.db_column_name ) using_expression: Optional[str] = None @@ -563,7 +569,7 @@ async def _run_alter_columns(self, backwards: bool = False): ) using_expression = "{}::{}".format( - alter_column.db_column_name, + alter_column.column.db_column_name, new_column.column_type, ) @@ -597,7 +603,7 @@ async def _run_alter_columns(self, backwards: bool = False): ) fk_column = existing_table._meta.get_column_by_name( - alter_column.column_name + alter_column.column.name ) assert isinstance(fk_column, ForeignKey) @@ -626,7 +632,7 @@ async def _run_alter_columns(self, backwards: bool = False): if null is not None: await self._run_query( _Table.alter().set_null( - column=alter_column.db_column_name, boolean=null + column=alter_column.column.db_column_name, boolean=null ) ) @@ -634,7 +640,7 @@ async def _run_alter_columns(self, backwards: bool = False): if length is not None: await self._run_query( _Table.alter().set_length( - column=alter_column.db_column_name, length=length + column=alter_column.column.db_column_name, length=length ) ) @@ -644,8 +650,8 @@ async def _run_alter_columns(self, backwards: bool = False): # a column type, and not just the column name. column = Column() column._meta._table = _Table - column._meta._name = alter_column.column_name - column._meta.db_column_name = alter_column.db_column_name + column._meta._name = alter_column.column.name + column._meta.db_column_name = alter_column.column.db_column_name await self._run_query( _Table.alter().set_unique( column=column, boolean=unique @@ -661,9 +667,9 @@ async def _run_alter_columns(self, backwards: bool = False): # to change the index type. column = Column() column._meta._table = _Table - column._meta._name = alter_column.column_name + column._meta._name = alter_column.column.name column._meta.db_column_name = ( - alter_column.db_column_name + alter_column.column.db_column_name ) await self._run_query(_Table.drop_index([column])) await self._run_query( @@ -678,8 +684,8 @@ async def _run_alter_columns(self, backwards: bool = False): # dropping, or creating an index. column = Column() column._meta._table = _Table - column._meta._name = alter_column.column_name - column._meta.db_column_name = alter_column.db_column_name + column._meta._name = alter_column.column.name + column._meta.db_column_name = alter_column.column.db_column_name if index is True: kwargs = ( @@ -698,8 +704,8 @@ async def _run_alter_columns(self, backwards: bool = False): if default is not ...: column = Column() column._meta._table = _Table - column._meta._name = alter_column.column_name - column._meta.db_column_name = alter_column.db_column_name + column._meta._name = alter_column.column.name + column._meta.db_column_name = alter_column.column.db_column_name if default is None: await self._run_query( @@ -718,7 +724,7 @@ async def _run_alter_columns(self, backwards: bool = False): if digits is not ...: await self._run_query( _Table.alter().set_digits( - column=alter_column.db_column_name, + column=alter_column.column.db_column_name, digits=digits, ) ) diff --git a/piccolo/apps/migrations/auto/operations.py b/piccolo/apps/migrations/auto/operations.py index 84e0d261a..d7e56bb32 100644 --- a/piccolo/apps/migrations/auto/operations.py +++ b/piccolo/apps/migrations/auto/operations.py @@ -33,16 +33,26 @@ class RenameColumn: @dataclass -class AlterColumn: - table_class_name: str - column_name: str - db_column_name: str +class TableRef: + class_name: str tablename: str + schema: Optional[str] = None + + +@dataclass +class ColumnRef: + name: str + db_column_name: str + + +@dataclass +class AlterColumn: + table: TableRef + column: ColumnRef params: dict[str, Any] old_params: dict[str, Any] column_class: Optional[type[Column]] = None old_column_class: Optional[type[Column]] = None - schema: Optional[str] = None @dataclass diff --git a/piccolo/apps/migrations/auto/schema_differ.py b/piccolo/apps/migrations/auto/schema_differ.py index 7dbc9a469..a7d775797 100644 --- a/piccolo/apps/migrations/auto/schema_differ.py +++ b/piccolo/apps/migrations/auto/schema_differ.py @@ -522,12 +522,12 @@ def alter_columns(self) -> AlterStatements: schema_str = ( "None" - if alter_column.schema is None - else f'"{alter_column.schema}"' + if alter_column.table.schema is None + else f'"{alter_column.table.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}, schema={schema_str})" # 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.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( diff --git a/piccolo/apps/migrations/auto/schema_snapshot.py b/piccolo/apps/migrations/auto/schema_snapshot.py index 73ba00b3f..63fec3cdc 100644 --- a/piccolo/apps/migrations/auto/schema_snapshot.py +++ b/piccolo/apps/migrations/auto/schema_snapshot.py @@ -82,7 +82,7 @@ def get_snapshot(self) -> list[DiffableTable]: ) for alter_column in alter_columns: for index, column in enumerate(table.columns): - if column._meta.name == alter_column.column_name: + if column._meta.name == alter_column.column.name: for key, value in alter_column.params.items(): setattr(column._meta, key, value) column._meta.params.update({key: value}) From e296567f06b5e2f9fa6e4349d8e3e118f47b1484 Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Mon, 29 Jun 2026 23:50:01 -0300 Subject: [PATCH 06/11] refactor: group Select delegates into semantic sub-dataclasses to reduce instance attributes (R0902) --- piccolo/query/methods/select.py | 170 +++++++++++++++++++++++++++----- 1 file changed, 145 insertions(+), 25 deletions(-) diff --git a/piccolo/query/methods/select.py b/piccolo/query/methods/select.py index 4266c3c7d..3987608f2 100644 --- a/piccolo/query/methods/select.py +++ b/piccolo/query/methods/select.py @@ -1,5 +1,7 @@ from __future__ import annotations +from dataclasses import dataclass, field + import itertools from collections import OrderedDict from collections.abc import Callable, Sequence @@ -147,22 +149,46 @@ async def run( return dump_json(rows) +@dataclass +class _ConditionDelegates: + where: WhereDelegate = field(default_factory=WhereDelegate) + having: WhereDelegate = field(default_factory=WhereDelegate) + + +@dataclass +class _SortDelegates: + order_by: OrderByDelegate = field(default_factory=OrderByDelegate) + group_by: GroupByDelegate = field(default_factory=GroupByDelegate) + distinct: DistinctDelegate = field(default_factory=DistinctDelegate) + + +@dataclass +class _PaginationDelegates: + limit: LimitDelegate = field(default_factory=LimitDelegate) + offset: OffsetDelegate = field(default_factory=OffsetDelegate) + + +@dataclass +class _OutputDelegates: + output: OutputDelegate = field(default_factory=OutputDelegate) + callback: CallbackDelegate = field(default_factory=CallbackDelegate) + + +@dataclass +class _MiscDelegates: + as_of: AsOfDelegate = field(default_factory=AsOfDelegate) + columns: ColumnsDelegate = field(default_factory=ColumnsDelegate) + lock_rows: LockRowsDelegate = field(default_factory=LockRowsDelegate) + + class Select(Query[TableInstance, list[dict[str, Any]]]): __slots__ = ( - "columns_list", "exclude_secrets", - "as_of_delegate", - "columns_delegate", - "distinct_delegate", - "group_by_delegate", - "limit_delegate", - "offset_delegate", - "order_by_delegate", - "output_delegate", - "callback_delegate", - "where_delegate", - "having_delegate", - "lock_rows_delegate", + "_condition", + "_sort", + "_pagination", + "_output", + "_misc", ) def __init__( @@ -177,21 +203,115 @@ def __init__( super().__init__(table, **kwargs) self.exclude_secrets = exclude_secrets - self.as_of_delegate = AsOfDelegate() - self.columns_delegate = ColumnsDelegate() - self.distinct_delegate = DistinctDelegate() - self.group_by_delegate = GroupByDelegate() - self.limit_delegate = LimitDelegate() - self.offset_delegate = OffsetDelegate() - self.order_by_delegate = OrderByDelegate() - self.output_delegate = OutputDelegate() - self.callback_delegate = CallbackDelegate() - self.where_delegate = WhereDelegate() - self.having_delegate = WhereDelegate() - self.lock_rows_delegate = LockRowsDelegate() + self._condition = _ConditionDelegates() + self._sort = _SortDelegates() + self._pagination = _PaginationDelegates() + self._output = _OutputDelegates() + self._misc = _MiscDelegates() self.columns(*columns_list) + ########################################################################### + # Backward-compatible properties for delegates + + @property + def columns_delegate(self) -> ColumnsDelegate: + return self._misc.columns + + @columns_delegate.setter + def columns_delegate(self, value: ColumnsDelegate) -> None: + self._misc.columns = value + + @property + def as_of_delegate(self) -> AsOfDelegate: + return self._misc.as_of + + @as_of_delegate.setter + def as_of_delegate(self, value: AsOfDelegate) -> None: + self._misc.as_of = value + + @property + def lock_rows_delegate(self) -> LockRowsDelegate: + return self._misc.lock_rows + + @lock_rows_delegate.setter + def lock_rows_delegate(self, value: LockRowsDelegate) -> None: + self._misc.lock_rows = value + + @property + def where_delegate(self) -> WhereDelegate: + return self._condition.where + + @where_delegate.setter + def where_delegate(self, value: WhereDelegate) -> None: + self._condition.where = value + + @property + def having_delegate(self) -> WhereDelegate: + return self._condition.having + + @having_delegate.setter + def having_delegate(self, value: WhereDelegate) -> None: + self._condition.having = value + + @property + def order_by_delegate(self) -> OrderByDelegate: + return self._sort.order_by + + @order_by_delegate.setter + def order_by_delegate(self, value: OrderByDelegate) -> None: + self._sort.order_by = value + + @property + def group_by_delegate(self) -> GroupByDelegate: + return self._sort.group_by + + @group_by_delegate.setter + def group_by_delegate(self, value: GroupByDelegate) -> None: + self._sort.group_by = value + + @property + def distinct_delegate(self) -> DistinctDelegate: + return self._sort.distinct + + @distinct_delegate.setter + def distinct_delegate(self, value: DistinctDelegate) -> None: + self._sort.distinct = value + + @property + def limit_delegate(self) -> LimitDelegate: + return self._pagination.limit + + @limit_delegate.setter + def limit_delegate(self, value: LimitDelegate) -> None: + self._pagination.limit = value + + @property + def offset_delegate(self) -> OffsetDelegate: + return self._pagination.offset + + @offset_delegate.setter + def offset_delegate(self, value: OffsetDelegate) -> None: + self._pagination.offset = value + + @property + def output_delegate(self) -> OutputDelegate: + return self._output.output + + @output_delegate.setter + def output_delegate(self, value: OutputDelegate) -> None: + self._output.output = value + + @property + def callback_delegate(self) -> CallbackDelegate: + return self._output.callback + + @callback_delegate.setter + def callback_delegate(self, value: CallbackDelegate) -> None: + self._output.callback = value + + ########################################################################### + def columns(self: Self, *columns: Union[Selectable, str]) -> Self: _columns = self.table._process_column_args(*columns) self.columns_delegate.columns(*_columns) From c106b0eb0e5e6efb5e412a6c77102ab8c3718625 Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Tue, 30 Jun 2026 00:01:55 -0300 Subject: [PATCH 07/11] refactor: consolidate Alter operations into _AlterOperations dataclass to reduce instance attributes (R0902) --- piccolo/query/methods/alter.py | 128 +++++++++++++++------------------ 1 file changed, 59 insertions(+), 69 deletions(-) diff --git a/piccolo/query/methods/alter.py b/piccolo/query/methods/alter.py index 35774acd3..6aa25fc34 100644 --- a/piccolo/query/methods/alter.py +++ b/piccolo/query/methods/alter.py @@ -2,7 +2,7 @@ import itertools from collections.abc import Sequence -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, Optional, TypeVar, Union from piccolo.columns.base import Column @@ -285,44 +285,34 @@ def ddl(self) -> str: return query -class Alter(DDL): - __slots__ = ( - "_add", - "_add_foreign_key_constraint", - "_drop_constraint", - "_drop_default", - "_drop_table", - "_drop", - "_rename_columns", - "_rename_table", - "_set_column_type", - "_set_default", - "_set_digits", - "_set_length", - "_set_null", - "_set_schema", - "_set_unique", - "_rename_constraint", +@dataclass +class _AlterOperations: # pylint: disable=too-many-instance-attributes + add: list[AddColumn] = field(default_factory=list) + add_foreign_key_constraint: list[AddForeignKeyConstraint] = field( + default_factory=list ) + drop: list[DropColumn] = field(default_factory=list) + drop_constraint: list[DropConstraint] = field(default_factory=list) + drop_default: list[DropDefault] = field(default_factory=list) + drop_table: Optional[DropTable] = None + rename_columns: list[RenameColumn] = field(default_factory=list) + rename_table: list[RenameTable] = field(default_factory=list) + rename_constraint: list[RenameConstraint] = field(default_factory=list) + set_column_type: list[SetColumnType] = field(default_factory=list) + set_default: list[SetDefault] = field(default_factory=list) + set_digits: list[SetDigits] = field(default_factory=list) + set_length: list[SetLength] = field(default_factory=list) + set_null: list[SetNull] = field(default_factory=list) + set_schema: list[SetSchema] = field(default_factory=list) + set_unique: list[SetUnique] = field(default_factory=list) + + +class Alter(DDL): + __slots__ = ("_operations",) def __init__(self, table: type[Table], **kwargs): super().__init__(table, **kwargs) - self._add_foreign_key_constraint: list[AddForeignKeyConstraint] = [] - self._add: list[AddColumn] = [] - self._drop_constraint: list[DropConstraint] = [] - self._drop_default: list[DropDefault] = [] - self._drop_table: Optional[DropTable] = None - self._drop: list[DropColumn] = [] - self._rename_columns: list[RenameColumn] = [] - self._rename_table: list[RenameTable] = [] - self._set_column_type: list[SetColumnType] = [] - self._set_default: list[SetDefault] = [] - self._set_digits: list[SetDigits] = [] - self._set_length: list[SetLength] = [] - self._set_null: list[SetNull] = [] - self._set_schema: list[SetSchema] = [] - self._set_unique: list[SetUnique] = [] - self._rename_constraint: list[RenameConstraint] = [] + self._operations = _AlterOperations() def add_column(self: Self, name: str, column: Column) -> Self: """ @@ -338,7 +328,7 @@ def add_column(self: Self, name: str, column: Column) -> Self: if isinstance(column, ForeignKey): column._setup(table_class=self.table) - self._add.append(AddColumn(column, name)) + self._operations.add.append(AddColumn(column, name)) return self def drop_column(self, column: Union[str, Column]) -> Alter: @@ -348,7 +338,7 @@ def drop_column(self, column: Union[str, Column]) -> Alter: >>> await Band.alter().drop_column(Band.popularity) """ - self._drop.append(DropColumn(column)) + self._operations.drop.append(DropColumn(column)) return self def drop_default(self, column: Union[str, Column]) -> Alter: @@ -358,7 +348,7 @@ def drop_default(self, column: Union[str, Column]) -> Alter: >>> await Band.alter().drop_default(Band.popularity) """ - self._drop_default.append(DropDefault(column=column)) + self._operations.drop_default.append(DropDefault(column=column)) return self def drop_table( @@ -370,7 +360,7 @@ def drop_table( >>> await Band.alter().drop_table() """ - self._drop_table = DropTable( + self._operations.drop_table = DropTable( table=self.table, cascade=cascade, if_exists=if_exists, @@ -385,7 +375,7 @@ def rename_table(self, new_name: str) -> Alter: """ # We override the existing one rather than appending. - self._rename_table = [RenameTable(new_name=new_name)] + self._operations.rename_table = [RenameTable(new_name=new_name)] return self def rename_constraint(self, old_name: str, new_name: str) -> Alter: @@ -398,7 +388,7 @@ def rename_constraint(self, old_name: str, new_name: str) -> Alter: ... ) """ - self._rename_constraint = [ + self._operations.rename_constraint = [ RenameConstraint( old_name=old_name, new_name=new_name, @@ -419,7 +409,7 @@ def rename_column( >>> await Band.alter().rename_column('popularity', 'rating') """ - self._rename_columns.append(RenameColumn(column, new_name)) + self._operations.rename_columns.append(RenameColumn(column, new_name)) return self def set_column_type( @@ -440,7 +430,7 @@ def set_column_type( ``'name::integer'``. """ - self._set_column_type.append( + self._operations.set_column_type.append( SetColumnType( old_column=old_column, new_column=new_column, @@ -456,7 +446,7 @@ def set_default(self, column: Column, value: Any) -> Alter: >>> await Band.alter().set_default(Band.popularity, 0) """ - self._set_default.append(SetDefault(column=column, value=value)) + self._operations.set_default.append(SetDefault(column=column, value=value)) return self def set_null( @@ -472,7 +462,7 @@ def set_null( >>> await Band.alter().set_null('name', True) """ - self._set_null.append(SetNull(column, boolean)) + self._operations.set_null.append(SetNull(column, boolean)) return self def set_unique( @@ -488,7 +478,7 @@ def set_unique( >>> await Band.alter().set_unique('name', True) """ - self._set_unique.append(SetUnique(column, boolean)) + self._operations.set_unique.append(SetUnique(column, boolean)) return self def set_length(self, column: Union[str, Varchar], length: int) -> Alter: @@ -516,7 +506,7 @@ def set_length(self, column: Union[str, Varchar], length: int) -> Alter: "Only Varchar columns can have their length changed." ) - self._set_length.append(SetLength(column, length)) + self._operations.set_length.append(SetLength(column, length)) return self def _get_constraint_name(self, column: Union[str, ForeignKey]) -> str: @@ -525,7 +515,7 @@ def _get_constraint_name(self, column: Union[str, ForeignKey]) -> str: return f"{tablename}_{column_name}_fkey" def drop_constraint(self, constraint_name: str) -> Alter: - self._drop_constraint.append( + self._operations.drop_constraint.append( DropConstraint(constraint_name=constraint_name) ) return self @@ -534,7 +524,7 @@ def drop_foreign_key_constraint( self, column: Union[str, ForeignKey] ) -> Alter: constraint_name = self._get_constraint_name(column=column) - self._drop_constraint.append( + self._operations.drop_constraint.append( DropConstraint(constraint_name=constraint_name) ) return self @@ -578,7 +568,7 @@ def add_foreign_key_constraint( else: raise ValueError("Please pass in `referenced_table_name`.") - self._add_foreign_key_constraint.append( + self._operations.add_foreign_key_constraint.append( AddForeignKeyConstraint( constraint_name=constraint_name, foreign_key_column_name=column_name, @@ -603,7 +593,7 @@ def set_digits( if isinstance(column, Numeric) else "NUMERIC" ) - self._set_digits.append( + self._operations.set_digits.append( SetDigits( digits=digits, column=column, @@ -620,34 +610,34 @@ def set_schema(self, schema_name: str) -> Alter: The schema to move the table to. """ - self._set_schema.append(SetSchema(schema_name=schema_name)) + self._operations.set_schema.append(SetSchema(schema_name=schema_name)) return self @property def default_ddl(self) -> Sequence[str]: - if self._drop_table is not None: - return [self._drop_table.ddl] + if self._operations.drop_table is not None: + return [self._operations.drop_table.ddl] query = f"ALTER TABLE {self.table._meta.get_formatted_tablename()}" alterations = [ i.ddl for i in itertools.chain( - self._add, - self._add_foreign_key_constraint, - self._rename_columns, - self._rename_table, - self._rename_constraint, - self._drop, - self._drop_constraint, - self._drop_default, - self._set_column_type, - self._set_unique, - self._set_null, - self._set_length, - self._set_default, - self._set_digits, - self._set_schema, + self._operations.add, + self._operations.add_foreign_key_constraint, + self._operations.rename_columns, + self._operations.rename_table, + self._operations.rename_constraint, + self._operations.drop, + self._operations.drop_constraint, + self._operations.drop_default, + self._operations.set_column_type, + self._operations.set_unique, + self._operations.set_null, + self._operations.set_length, + self._operations.set_default, + self._operations.set_digits, + self._operations.set_schema, ) ] From 81050b357f4631b6c0765537a912e93406506969 Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Tue, 30 Jun 2026 00:14:17 -0300 Subject: [PATCH 08/11] refactor: group Objects delegates into semantic sub-dataclasses to reduce instance attributes (R0902) --- piccolo/query/methods/objects.py | 141 ++++++++++++++++++++++++++----- 1 file changed, 121 insertions(+), 20 deletions(-) diff --git a/piccolo/query/methods/objects.py b/piccolo/query/methods/objects.py index 1bc0d711f..aaedb5cce 100644 --- a/piccolo/query/methods/objects.py +++ b/piccolo/query/methods/objects.py @@ -1,6 +1,7 @@ from __future__ import annotations from collections.abc import Callable, Generator, Sequence +from dataclasses import dataclass, field from typing import ( TYPE_CHECKING, Any, @@ -299,6 +300,38 @@ def run_sync(self, *args, **kwargs) -> Optional[ReferencedTable]: ############################################################################### +@dataclass +class _ObjectsConditionDelegates: + where: WhereDelegate = field(default_factory=WhereDelegate) + + +@dataclass +class _ObjectsSortDelegates: + order_by: OrderByDelegate = field(default_factory=OrderByDelegate) + + +@dataclass +class _ObjectsPaginationDelegates: + limit: LimitDelegate = field(default_factory=LimitDelegate) + offset: OffsetDelegate = field(default_factory=OffsetDelegate) + + +@dataclass +class _ObjectsOutputDelegates: + output: OutputDelegate = field(default_factory=OutputDelegate) + callback: CallbackDelegate = field(default_factory=CallbackDelegate) + + +@dataclass +class _ObjectsMiscDelegates: + as_of: AsOfDelegate = field(default_factory=AsOfDelegate) + lock_rows: LockRowsDelegate = field(default_factory=LockRowsDelegate) + prefetch: PrefetchDelegate = field(default_factory=PrefetchDelegate) + + +############################################################################### + + class Objects( Query[TableInstance, list[TableInstance]], Generic[TableInstance] ): @@ -308,16 +341,11 @@ class Objects( """ __slots__ = ( - "nested", - "as_of_delegate", - "limit_delegate", - "offset_delegate", - "order_by_delegate", - "output_delegate", - "callback_delegate", - "prefetch_delegate", - "where_delegate", - "lock_rows_delegate", + "_condition", + "_sort", + "_pagination", + "_output", + "_misc", ) def __init__( @@ -327,17 +355,90 @@ def __init__( **kwargs, ): super().__init__(table, **kwargs) - self.as_of_delegate = AsOfDelegate() - self.limit_delegate = LimitDelegate() - self.offset_delegate = OffsetDelegate() - self.order_by_delegate = OrderByDelegate() - self.output_delegate = OutputDelegate() - self.output_delegate._output.as_objects = True - self.callback_delegate = CallbackDelegate() - self.prefetch_delegate = PrefetchDelegate() + self._condition = _ObjectsConditionDelegates() + self._sort = _ObjectsSortDelegates() + self._pagination = _ObjectsPaginationDelegates() + self._output = _ObjectsOutputDelegates() + self._output.output._output.as_objects = True + self._misc = _ObjectsMiscDelegates() self.prefetch(*prefetch) - self.where_delegate = WhereDelegate() - self.lock_rows_delegate = LockRowsDelegate() + + ########################################################################### + # Backward-compatible properties for delegates + + @property + def as_of_delegate(self) -> AsOfDelegate: + return self._misc.as_of + + @as_of_delegate.setter + def as_of_delegate(self, value: AsOfDelegate) -> None: + self._misc.as_of = value + + @property + def limit_delegate(self) -> LimitDelegate: + return self._pagination.limit + + @limit_delegate.setter + def limit_delegate(self, value: LimitDelegate) -> None: + self._pagination.limit = value + + @property + def offset_delegate(self) -> OffsetDelegate: + return self._pagination.offset + + @offset_delegate.setter + def offset_delegate(self, value: OffsetDelegate) -> None: + self._pagination.offset = value + + @property + def order_by_delegate(self) -> OrderByDelegate: + return self._sort.order_by + + @order_by_delegate.setter + def order_by_delegate(self, value: OrderByDelegate) -> None: + self._sort.order_by = value + + @property + def output_delegate(self) -> OutputDelegate: + return self._output.output + + @output_delegate.setter + def output_delegate(self, value: OutputDelegate) -> None: + self._output.output = value + + @property + def callback_delegate(self) -> CallbackDelegate: + return self._output.callback + + @callback_delegate.setter + def callback_delegate(self, value: CallbackDelegate) -> None: + self._output.callback = value + + @property + def prefetch_delegate(self) -> PrefetchDelegate: + return self._misc.prefetch + + @prefetch_delegate.setter + def prefetch_delegate(self, value: PrefetchDelegate) -> None: + self._misc.prefetch = value + + @property + def where_delegate(self) -> WhereDelegate: + return self._condition.where + + @where_delegate.setter + def where_delegate(self, value: WhereDelegate) -> None: + self._condition.where = value + + @property + def lock_rows_delegate(self) -> LockRowsDelegate: + return self._misc.lock_rows + + @lock_rows_delegate.setter + def lock_rows_delegate(self, value: LockRowsDelegate) -> None: + self._misc.lock_rows = value + + ########################################################################### def output(self: Self, load_json: bool = False) -> Self: self.output_delegate.output( From 5ba8a21da6027a70514dee7918c67d6d790dbd3d Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Tue, 30 Jun 2026 00:26:17 -0300 Subject: [PATCH 09/11] refactor: combine transaction state booleans into TransactionState enum in SQLiteTransaction (R0902) --- piccolo/engine/sqlite.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/piccolo/engine/sqlite.py b/piccolo/engine/sqlite.py index 2b1840c58..9a441ecc1 100644 --- a/piccolo/engine/sqlite.py +++ b/piccolo/engine/sqlite.py @@ -438,6 +438,12 @@ def __await__(self): ############################################################################### +class TransactionState(enum.Enum): + ACTIVE = "active" + COMMITTED = "committed" + ROLLED_BACK = "rolled_back" + + class Savepoint: def __init__(self, name: str, transaction: SQLiteTransaction): self.name = name @@ -477,8 +483,7 @@ class SQLiteTransaction(BaseTransaction): "allow_nested", "_savepoint_id", "_parent", - "_committed", - "_rolled_back", + "_state", ) def __init__( @@ -501,8 +506,7 @@ def __init__( self._savepoint_id = 0 self._parent = None - self._committed = False - self._rolled_back = False + self._state = TransactionState.ACTIVE if current_transaction: if allow_nested: @@ -530,11 +534,11 @@ async def begin(self): async def commit(self): await self.connection.execute("COMMIT") - self._committed = True + self._state = TransactionState.COMMITTED async def rollback(self): await self.connection.execute("ROLLBACK") - self._rolled_back = True + self._state = TransactionState.ROLLED_BACK async def rollback_to(self, savepoint_name: str): """ @@ -561,12 +565,10 @@ async def __aexit__(self, exception_type, exception, traceback) -> bool: return exception is None if exception: - # The user may have manually rolled it back. - if not self._rolled_back: + if self._state is not TransactionState.ROLLED_BACK: await self.rollback() else: - # The user may have manually committed it. - if not self._committed and not self._rolled_back: + if self._state is TransactionState.ACTIVE: await self.commit() await self.connection.close() From 2b3d7d28a172b07a045b37aba0e77aeede7b5964 Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Tue, 30 Jun 2026 00:41:18 -0300 Subject: [PATCH 10/11] refactor: centralize TransactionState enum in base.py and reuse across SQLite and Postgres engines (R0902) --- piccolo/engine/base.py | 7 +++++++ piccolo/engine/postgres.py | 16 ++++++++-------- piccolo/engine/sqlite.py | 7 +------ 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/piccolo/engine/base.py b/piccolo/engine/base.py index b86af9dd2..3b549e32b 100644 --- a/piccolo/engine/base.py +++ b/piccolo/engine/base.py @@ -1,6 +1,7 @@ from __future__ import annotations import contextvars +import enum import logging import pprint import string @@ -34,6 +35,12 @@ def validate_savepoint_name(savepoint_name: str) -> None: ) +class TransactionState(enum.Enum): + ACTIVE = "active" + COMMITTED = "committed" + ROLLED_BACK = "rolled_back" + + class BaseBatch(metaclass=ABCMeta): @abstractmethod async def __aenter__(self: Self, *args, **kwargs) -> Self: ... diff --git a/piccolo/engine/postgres.py b/piccolo/engine/postgres.py index ec8c11b91..95f5d1f4c 100644 --- a/piccolo/engine/postgres.py +++ b/piccolo/engine/postgres.py @@ -1,6 +1,7 @@ from __future__ import annotations import contextvars +import enum from collections.abc import Sequence from dataclasses import dataclass from typing import TYPE_CHECKING, Any, Mapping, Optional, Union @@ -12,6 +13,7 @@ BaseBatch, BaseTransaction, Engine, + TransactionState, validate_savepoint_name, ) from piccolo.engine.exceptions import TransactionError @@ -180,8 +182,7 @@ class PostgresTransaction(BaseTransaction): "connection", "_savepoint_id", "_parent", - "_committed", - "_rolled_back", + "_state", ) def __init__(self, engine: PostgresEngine, allow_nested: bool = True): @@ -204,8 +205,7 @@ def __init__(self, engine: PostgresEngine, allow_nested: bool = True): self._savepoint_id = 0 self._parent = None - self._committed = False - self._rolled_back = False + self._state = TransactionState.ACTIVE if current_transaction: if allow_nested: @@ -237,11 +237,11 @@ async def begin(self): async def commit(self): await self.transaction.commit() - self._committed = True + self._state = TransactionState.COMMITTED async def rollback(self): await self.transaction.rollback() - self._rolled_back = True + self._state = TransactionState.ROLLED_BACK async def rollback_to(self, savepoint_name: str): """ @@ -269,11 +269,11 @@ async def __aexit__(self, exception_type, exception, traceback) -> bool: if exception: # The user may have manually rolled it back. - if not self._rolled_back: + if self._state is not TransactionState.ROLLED_BACK: await self.rollback() else: # The user may have manually committed it. - if not self._committed and not self._rolled_back: + if self._state is TransactionState.ACTIVE: await self.commit() if self.engine.pool: diff --git a/piccolo/engine/sqlite.py b/piccolo/engine/sqlite.py index 9a441ecc1..354228388 100644 --- a/piccolo/engine/sqlite.py +++ b/piccolo/engine/sqlite.py @@ -19,6 +19,7 @@ BaseBatch, BaseTransaction, Engine, + TransactionState, validate_savepoint_name, ) from piccolo.engine.exceptions import TransactionError @@ -438,12 +439,6 @@ def __await__(self): ############################################################################### -class TransactionState(enum.Enum): - ACTIVE = "active" - COMMITTED = "committed" - ROLLED_BACK = "rolled_back" - - class Savepoint: def __init__(self, name: str, transaction: SQLiteTransaction): self.name = name From d67172ddd2835559e45e1ab6af9daf09836e05ce Mon Sep 17 00:00:00 2001 From: Wania Santos Date: Tue, 30 Jun 2026 01:16:25 -0300 Subject: [PATCH 11/11] refactor: group ColumnMeta configuration attributes into ColumnConfig dataclass (R0902) --- piccolo/columns/base.py | 124 +++++++++++++++++++++++++++++++++++----- 1 file changed, 110 insertions(+), 14 deletions(-) diff --git a/piccolo/columns/base.py b/piccolo/columns/base.py index 879e4088f..d89dbaeeb 100644 --- a/piccolo/columns/base.py +++ b/piccolo/columns/base.py @@ -153,13 +153,12 @@ def __deepcopy__(self, memo) -> ForeignKeyMeta[ReferencedTable]: @dataclass -class ColumnMeta: +class ColumnConfig: """ - We store as many attributes in ColumnMeta as possible, to help avoid name - clashes with user defined attributes. + Holds column configuration attributes, grouped together to reduce the + number of instance attributes on :class:`ColumnMeta`. """ - # General attributes: null: bool = False primary_key: bool = False unique: bool = False @@ -171,6 +170,16 @@ class ColumnMeta: secret: bool = False auto_update: Any = ... + +@dataclass +class ColumnMeta: + """ + We store as many attributes in ColumnMeta as possible, to help avoid name + clashes with user defined attributes. + """ + + config: ColumnConfig = field(default_factory=ColumnConfig) + # Used for representing the table in migrations and the playground. params: dict[str, Any] = field(default_factory=dict) @@ -222,6 +231,90 @@ def table(self, value: type[Table]): # Used by Foreign Keys: call_chain: list["ForeignKey"] = field(default_factory=list) + ########################################################################### + # Column configuration - delegated to self.config for backwards + # compatibility. + + @property + def null(self) -> bool: + return self.config.null + + @null.setter + def null(self, value: bool): + self.config.null = value + + @property + def primary_key(self) -> bool: + return self.config.primary_key + + @primary_key.setter + def primary_key(self, value: bool): + self.config.primary_key = value + + @property + def unique(self) -> bool: + return self.config.unique + + @unique.setter + def unique(self, value: bool): + self.config.unique = value + + @property + def index(self) -> bool: + return self.config.index + + @index.setter + def index(self, value: bool): + self.config.index = value + + @property + def index_method(self) -> IndexMethod: + return self.config.index_method + + @index_method.setter + def index_method(self, value: IndexMethod): + self.config.index_method = value + + @property + def required(self) -> bool: + return self.config.required + + @required.setter + def required(self, value: bool): + self.config.required = value + + @property + def help_text(self) -> Optional[str]: + return self.config.help_text + + @help_text.setter + def help_text(self, value: Optional[str]): + self.config.help_text = value + + @property + def choices(self) -> Optional[type[Enum]]: + return self.config.choices + + @choices.setter + def choices(self, value: Optional[type[Enum]]): + self.config.choices = value + + @property + def secret(self) -> bool: + return self.config.secret + + @secret.setter + def secret(self, value: bool): + self.config.secret = value + + @property + def auto_update(self) -> Any: + return self.config.auto_update + + @auto_update.setter + def auto_update(self, value: Any): + self.config.auto_update = value + ########################################################################### @property @@ -335,6 +428,7 @@ def get_full_name( def copy(self) -> ColumnMeta: kwargs = self.__dict__.copy() kwargs.update( + config=copy.copy(self.config), params=self.params.copy(), call_chain=self.call_chain.copy(), ) @@ -524,18 +618,20 @@ def __init__( self._validate_choices(choices, allowed_type=self.value_type) self._meta = ColumnMeta( - null=null, - primary_key=primary_key, - unique=unique, - index=index, - index_method=index_method, + config=ColumnConfig( + null=null, + primary_key=primary_key, + unique=unique, + index=index, + index_method=index_method, + required=required, + help_text=help_text, + choices=choices, + secret=secret, + auto_update=auto_update, + ), params=kwargs, - required=required, - help_text=help_text, - choices=choices, _db_column_name=db_column_name, - secret=secret, - auto_update=auto_update, ) self._alias: Optional[str] = None