accomodate new MySQL explicit_defaults_for_timestamp flag

Issue #3155 resolved
David Lents created an issue

e.g: last_access = Column(TIMESTAMP(), nullable=False, server_default=text('CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP')))

results in: last_access timestamp NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP

This appears to be caused by dialects/mysql/base.py line 1679:

if not column.nullable and not is_timestamp: colspec.append('NOT NULL')

This doesn't seem correct, as NOT NULL changes the behavior of timestamp type in mysql if I'm not mistaken.

Comments (13)

  1. Mike Bayer repo owner

    this bug report is very unclear:

    1. It is incorrect to say server_default=text('CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP'). That is an attempt to "game" the DDL generation and is unnecessary. Use server_onupdate for the UPDATE part.

    2. The DDL as given will not emit "NULL":

    from sqlalchemy import MetaData, TIMESTAMP, Table, Column, text
    
    m = MetaData()
    t = Table(
        'ts_test', m,
        Column(
            "last_access", TIMESTAMP(), nullable=False,
            server_default=text(
                'CURRENT_TIMESTAMP '
                'ON UPDATE CURRENT_TIMESTAMP')
        )
    )
    
    
    from sqlalchemy import create_engine
    e = create_engine("mysql://scott:tiger@localhost/test", echo=True)
    m.create_all(e)
    

    output:

    CREATE TABLE ts_test (
        last_access TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP
    )
    

    the NOT NULL clause, normally suggested by nullable=False, is not stated in the case of TIMESTAMP. MySQL itself reports this clause as NOT NULL:

    mysql> SHOW CREATE TABLE ts_test;
    +---------+---------------------------------
    | Table   | Create Table                                                                                                                                        `
    +---------+---------------------------------
    | ts_test | CREATE TABLE `ts_test` (
      `last_access` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP
    ) ENGINE=MyISAM DEFAULT CHARSET=latin1 |
    +---------+----------------------------------
    

    this is why you see code where the logic for rendering NULL / NOT NULL is reversed in the case of TIMESTAMP - MySQL itself has the reverse behavior.

    There is one potential issue I identified in reviewing this functionality - if you do in fact want a TIMESTAMP column, with a default, to also allow holding of a NULL, this is not supported at the DDL level, yet MySQL itself supports this.

    Please specify accurately and carefully what specific DDL you are attempting to emit and include a test case which illustrates an actual end result not being produced correctly, thanks.

    I've added a new documentation section explaining the current behavior in 4ddc4c456ce1a6cc3e27bd.

  2. David Lents reporter

    Thanks for the quick response, and apologies for not including specific examples, hopefully this followup will be better.

    First, the MySQL functionality I'm looking for is described here, specifically:

    In addition, you can initialize or update any TIMESTAMP column to the current date and time by assigning it a NULL value, unless it has been defined with the NULL attribute to permit NULL values.

    Second, I have tried using the server_onupdate=, but I can't get it to emit anything, so I looked up a workaround for "gaming" the DDL generation, as you point out (example of this provided below).

    I was able to reproduce your results with the DDL you provided, so perhaps there is something wrong with mine. My python is a bit rusty, so feel free to correct me as needed. Also, I'm using python 3.4.1 and (obviously) oursql with MySQL server version 5.6.17. This code does everything I want except that last_access does not get created as NOT NULL:

    from sqlalchemy import MetaData, TIMESTAMP, Integer, Enum, Column, text
    from sqlalchemy.ext.declarative import declarative_base
    
    m = MetaData()
    from sqlalchemy import create_engine
    
    e = create_engine("mysql+oursql://david:tiger@localhost/a_ts_test", echo=True)
    Base = declarative_base(bind=e, metadata=m)
    
    
    class TsTest(Base):
        __tablename__ = 'ts_test'
        id = Column(Integer, primary_key=True)
        last_access = Column(
            TIMESTAMP(), nullable=False,
            server_default=text(
                'CURRENT_TIMESTAMP '
                'ON UPDATE CURRENT_TIMESTAMP')
        )
        priority = Column(Enum('low', 'medium', 'high'), nullable=False)
    
        def __init__(self, priority):
            self.priority = priority
    
    
    m.drop_all(e)
    m.create_all(e)
    

    output:

    CREATE TABLE ts_test (
        id INTEGER NOT NULL AUTO_INCREMENT, 
        last_access TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, 
        priority ENUM('low','medium','high') NOT NULL, 
        PRIMARY KEY (id)
    )
    

    MySQL:

    mysql> SHOW CREATE TABLE ts_test\G
    *************************** 1. row ***************************
           Table: ts_test
    Create Table: CREATE TABLE `ts_test` (
      `id` int(11) NOT NULL AUTO_INCREMENT,
      `last_access` timestamp NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
      `priority` enum('low','medium','high') NOT NULL,
      PRIMARY KEY (`id`)
    ) ENGINE=InnoDB DEFAULT CHARSET=utf8
    

    For the server_onupdate example, all code is the same except this:

    last_access = Column(
        TIMESTAMP(), nullable=False,
            server_default=text('CURRENT_TIMESTAMP'),
            server_onupdate=text('CURRENT_TIMESTAMP')
    )
    

    Yields:

    CREATE TABLE ts_test (
        id INTEGER NOT NULL AUTO_INCREMENT, 
        last_access TIMESTAMP DEFAULT CURRENT_TIMESTAMP, 
        priority ENUM('low','medium','high') NOT NULL, 
        PRIMARY KEY (id)
    )
    

    MySQL:

    mysql> SHOW CREATE TABLE ts_test\G
    *************************** 1. row ***************************
           Table: ts_test
    Create Table: CREATE TABLE `ts_test` (
      `id` int(11) NOT NULL AUTO_INCREMENT,
      `last_access` timestamp NULL DEFAULT CURRENT_TIMESTAMP,
      `priority` enum('low','medium','high') NOT NULL,
      PRIMARY KEY (`id`)
    ) ENGINE=InnoDB DEFAULT CHARSET=utf8
    

    What I want is:

    CREATE TABLE `ts_test` (
      `id` int(11) NOT NULL AUTO_INCREMENT,
      `last_access` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
      `priority` enum('low','medium','high') NOT NULL,
      PRIMARY KEY (`id`)
    ) ENGINE=InnoDB DEFAULT CHARSET=utf8
    
  3. David Lents reporter

    On further research I have found the cause of the different behavior I was seeing! I'm doing development locally using the MySQL server from AMPPS, and they have the server variable explicit_defaults_for_timestamp set ON by default, which is apparently now recommended as of MySQL v5.6.6 (see MySQL upgrading link below).

    Here's a link to the docs for explicit_defaults_for_timestamp. This is also mentioned in the upgrading to 5.6 document here (search for "TIMESTAMP with implicit DEFAULT value"). It looks like the current nonstandard behavior is being deprecated.

    Based on this new info, IMO you should honor an explicit nullable setting in the DDL, so the expected result is emitted regardless of whether the explicit_defaults_for_timestamp server variable exists, or how it is set.

    So, you can reproduce my results by using MySQL v5.6 and in my.cnf add setting:

    explicit_defaults_for_timestamp=1

  4. Mike Bayer repo owner

    OK looking again, server_onupdate doesn't have that purpose right now, there does not seem to be anything built in to the MySQL dialect in order to render the non-SQL-standard "ON UPDATE" clause. So it's funny that they are deprecating "nonstandard" behavior but "ON UPDATE" is still nonstandard.

    Right now it appears like we'd win if we just render NULL/NOT NULL in all cases for TIMESTAMP, how is that? that would be:

    @@ -1765,12 +1761,12 @@ class MySQLDDLCompiler(compiler.DDLCompiler):
                 colspec.append('DEFAULT ' + default)
    
             is_timestamp = isinstance(column.type, sqltypes.TIMESTAMP)
    -        if not column.nullable and not is_timestamp:
    +        if not column.nullable:
                 colspec.append('NOT NULL')
    
             # see: http://docs.sqlalchemy.org/en/latest/dialects/
             #   mysql.html#mysql_timestamp_null
    -        elif column.nullable and is_timestamp and default is None:
    +        elif column.nullable and is_timestamp:
                 colspec.append('NULL')
    
             if column is column.table._autoincrement_column and \
    
  5. David Lents reporter

    Yes, I think this is the correct solution. It doesn't seem safe to make assumptions about MySQL's implicit defaults ;)

    Thanks!

  6. Mike Bayer repo owner
    • The MySQL dialect now renders TIMESTAMP with NULL / NOT NULL in all cases, so that MySQL 5.6.6 with the explicit_defaults_for_timestamp flag enabled will will allow TIMESTAMP to continue to work as expected when nullable=False. Existing applications are unaffected as SQLAlchemy has always emitted NULL for a TIMESTAMP column that is nullable=True. fixes #3155

    → <<cset 503a40ad7080>>

  7. Log in to comment