perseas / Pyrseas

Provides utilities for Postgres database schema versioning.
https://perseas.github.io/
BSD 3-Clause "New" or "Revised" License
395 stars 67 forks source link

Pyrseas classes should have declared, known attributes #113

Closed jmafc closed 6 years ago

jmafc commented 9 years ago

Pyrseas classes that correspond to Postgres database object types have attributes that are initialized based on the queries of the aggregate (Dict) classes. This then requires testing for the existence of attributes in multiple places in the code (over 300 if hasattr tests). Each class should have known attributes.

jmafc commented 9 years ago

Completed a preliminary analysis of all classes in pyrseas/dbobject. Of 26 classes (combines, for example, classes such as Table, Sequence, View and MaterializedView into DbClass), 24 have a name attribute. Cast and UserMapping are the two exceptions. However, UserMapping's username attribute could definitely be used as name (at least internally) and Cast's source could possibly be known as name. UserMapping is the only class without a description (COMMENT text). The bottom line is that both name and description should be declared as known attributes of the DbObject class and therefore all its descendants.

There are nine classes without an owner attribute, namely, Cast, UserMapping, Column, Constraint (and its four subclasses), Index, Rule, Trigger, TSParser and TSTemplate. In addition, Conversion doesn't have owner but it should. There are 17 classes without a privileges attribute, so I'll list those that do have it: Language, ForeignDataWrapper, ForeignServer, Schema, DbClass (and its four descendants), ForeignTable, Column, DbType (not implemented yet), and Proc (and Function/Aggregate). The case for making owner and privileges known attributes of DbObject is less clear, particularly for the latter, but it is conceivable that permissions and ownership will become defined for some additional classes. Therefore, it may be proper to add these as attributes of DbObject.

In addtion, all descendants of DbSchemaObject have a schema attribute so this should be a known attribute of that class.

Lastly, for @dvarrazzo's deptrack branch, the oid is used to some extent. There is only one DbObject descendant class that doesn't have an oid and that is the Column class. ForeignTable and Index don't have an oid column in the respective catalog but they have a reference to a pg_class oid.

Daniele, I'd appreciate your review and comments on the above.

jmafc commented 6 years ago

Although there are still improvements that can be made, this has been addressed by the commits in Feb 2015 and from Jul-Oct 2017.