add types.Identity, remove autoincrement=

Issue #1030 resolved
jek created an issue

Prefer types.Identity for generating SERIAL, AUTO INCREMENT and GENERATED AS IDENTITY keys; deprecate and remove implicit autoincrement. Identity can accept an explicit user-supplied sequence name for databases with no underlying support.

Otherwise the options can probably mirror the sql standard: generated=(None,always,by default}, data type= (dialects probably default to INT), start with=, increment by=, min value=, max value=, cycle=bool

Comments (7)

  1. Mike Bayer repo owner

    why would it be a Type ? this seems to conflict with our usage of "default". Do we deprecate Sequence, ColumnDefault, etc , and move those all into TypeEngine as well ? Isn't PG the only database that has a SERIAL type and therefore conflates the type of a column with its default behavior ? (others have either a separate keyword, or use sequences)

  2. jek reporter

    the sql for IDENTITY does hang off a type declaration, so it could be a cluster of options hanging off of the Column. either way.

    the main point is to replace the implicit magic autoincrement with an explicit declaration.

  3. Mike Bayer repo owner
    • changed milestone to 0.6.0

    seems a little extreme for the current state of 0.5.

    also not sure how we can remove the implicit magic autoincrement with a database like sqlite, which has such a feature on at all times ?

  4. Michael Trier

    +1 on this change. Implicit IDENTITIES seem to be bad news. If the user wants an IDENTITY (sequence) it should be explicit. One such situation that is causing a problem is the following:

            employees = Table('employees', metadata,
                Column('id', Integer),
                Column('soc', String(40)),
                Column('name', String(30)),
                PrimaryKeyConstraint('id', 'soc')
                )
    

    In this case on MSSQL the id becomes an IDENTITY whereas I would never make the id and IDENTITY. It's illogical because it's not like it's scoped. Certainly we can work around it on that dialect but still it seem the current implicit nature is a no win situation.

  5. Mike Bayer repo owner

    For your example, they'd set autoincrement=False on "id".

    I don't know if I'd characterize autoincrement as "implicit", its just default=True. Also, sqlite implicitly autoincrements a single-column primary key, so any kind of option type is meaningless there.

    If we go with extra arguments, that basically amounts to keeping "autoincrement", and defaulting it to False. Which is just punishing, the vast majority of examples and use cases would have to read Column('foo', Integer, primary_key=True, autoincrement=True). People aren't going to like that.

    Using an Identity type takes away the punishment of "Integer, primary_key=True, autoincrement=True" but then we start tripping over Sequence....does Identity create an implicit sequence name when used with Oracle/Firebird ? Or do those DBs still require Sequence to be used explicitly ?

  6. Mike Bayer repo owner

    autoincrement=True has been clarified in 0.6 (just needs the docs to be updated). it still defaults to True but if you have a composite integer primary key, you're probably going to want to set it appropriately. 0.6 is handling very intricate autoincrement scenarios now and afaic this is how it should be.

  7. Log in to comment