[PATCH] PGEnum PostgreSQL enum type

[Former user]

First posted at ticket:1109#comment:6 and followups. MySQL enums (a filtered string type) are very different from PostgreSQL enums (each enum a distinct, first-class type), hence the separate ticket.

The patch adds support for PGEnum to @r6287, along with generic support for DDL-defined types. To run the tests:

nosetests --db=postgresql test.dialect.test_postgresql:SchemaDefinedTypesTest -v

[Former user]

PGEnum patch (trunk @r6287)

[Mike Bayer]

OK this is a really great patch and I'm impressed of the work that is done here. One thing that I would need to change though is the embedding of DDL within the TypeEngine subclass itself - a core feature of 0.6 is that all DDL/SQL generation moves into the compiler framework. So the CREATE TYPE/DROP TYPE stuff would go into CreateType/DropType objects, subclasses of DDLElement, into postgreql/base.py, and all the correspoinding string generation methods in PGDDLCompiler. Similarly get_col_spec() is out the door in 0.6....thats only in UserDefinedType to make life easier for purely user-defined type subclasses being ported to 0.6 - PGTypeCompiler gets visit_enum and does what it needs with the state of the type object.

I was looking into maybe adding the type to the MetaData via the Column event system, but i think keeping the explicit metadata argument on the type is best here. When metadata is set, the CreateType/DropType stuff gets added as a metadata create/drop event. I don't think there's any need for the "exists in schema" stuff, and I don't really see the need yet for the base SchemaDefinedType stuff - since most of the methods it defines aren't needed.

[Former user]

I've implemented the changes you proposed. The patches apply on top of the first.

I've kept the {{{SchemaDefinedType}}} superclass. It's thin, but still valuable to have other similar types standardise on the add_to_schema method and the metadata= constructor param.

There is one XXX left in the code. I'm not sure how to quote a string literal for a DDL statement.

[Mike Bayer]

im still a little antsy on the __metadatas collection. its almost always harmless but there are some cases where we've had "memory leak" issues, for applications which make many engines or metadatas. Even though this is not the typical programming paradigm, it would be nice if there weren't a "surprise" memory leak if someone happened to use many metadatas and this particular type object. It would be better if we at some point added a "has_event()" type of hook to MetaData and Table for this kind of thing.

another thing missing here is that there's no hook for "does this type already exist ?" (i.e. in the actual PG database). otherwise its not possible here to call metadata.create_all() multiple times. If the PG dialect also implemented has_type() the way it implements has_table and has_sequence, the hook can be placed as the "on" callable within the CreateType/DropType objects which would only create/drop the type if it didn't/did exist.

also when would create_statement(), drop_statement(), __ddl_listener() on SchemaDefinedType be called ? I tried to follow each patch visually and it seems like these are still there (I may have missed something though).

[Former user]

OK, I did a bit more for those last things (got a bit carried away). If the multiple patches are confusing (consider git-svn) I'll add one big diff as well, though that lacks commit messages.

The {{{__metadatas}}} now keeps just weak references. The type is just a listener and doesn't need more.

I also added support to check for existence, now create_all and drop_all can be called multiple times. There is also SQL schema support for this.

I tweaked the create_statement stuff slightly, now the {{{DDLElement}}}s listens directly and {{{__ddl_listener}}} got removed. They get called multiple types, though once would work as well.

[Mike Bayer]

OK its very very good. I still think the __metadata is a bit superfluous but I will only resist that so much.

Any reason for the @property / def somename / self.__somename system there ? we allow direct attribute access if no functionality is required (i.e. self.name = name).

[Former user]

The @property access is mostly a habit. I'm not sure where to put docstrings without.

There's some important type checking for value and name, that should be documented if we don't validate access. The type checking could be removed if you know how to get the string literal quoting done right (that XXX comment).

[Former user]

I meant, just {{{values}}}.

[Mike Bayer]

here's some notes on this:

class Enum(SchemaLevel):
    def __init__(self, *enums, **kw):
        pass

    def _set_parent(self, column):
        column._on_table_attach(self._set_table)

     def _set_table(self, table):
         table.append_ddl_listener('before-create', self._on_table_create)
         table.append_ddl_listener('after-drop', self._on_table_drop)
         table.metadata.append_ddl_listener('before-create', self._on_metadata_create)
         table.metadata.append_ddl_listener('after-drop', self._on_metadata_drop)

    def _on_table_create(self, event, target, bind):
        pass

    def _on_table_drop(self, event, target, bind):
        pass

    def _on_metadata_create(self, event, target, bind):
        pass

    def _on_metadata_drop(self, event, target, bind):
        pass

[Mike Bayer]

I've made usage of the nuts and bolts of this patch in aa557982fa2518e6d520ce17894093d5ed03c0eb. The new PG type is called ENUM despite its not-quite correspondence there, but general usage is via the generic Enum type in sqlalchemy.types.

This type:

• can be used generically as types.Enum
• supports unicode, turned on if the given labels are unicode objects
• supports reflection
• supports individual create() drop() calls like any other schema construct
• automatically creates/drops itself on selected Table and MetaData events
• the has_type() logic is wired into the create/drop logic

I'm going to leave #1109 open until I add at least a "VARCHAR" version for sqlite and Oracle (Oracle's at least will also generate a CHECK constraint). Thanks for all the effort on this !

[Mike Bayer]

Removing milestone: 0.6.0 (automated comment)