I haven't tested the part where database tables are created with one flag
but accessed under a different flag, the changes have been working for us
so far.
On Tue, Oct 23, 2018 at 10:09 PM Bolke de Bruin <bdbr...@gmail.com> wrote:
> We only need it at table creation time or alter table time during which an
> alembic script would fail if MySQL restarts I assume?
>
> I'm not sure if the PR in this way is required (but if it works and works
> well it's okay to me too just like consistency across DBS and no surprises
> with MySQL )
>
> Sent from my iPhone
>
> On 24 Oct 2018, at 05:18, Feng Lu <fen...@google.com> wrote:
>
> Sorry for the late reply.
> GCP (CloudSQL) does support setting this parameter at session level but
> the VM used to host the mysqld might be restarted at any time, so it can't
> be done reliably.
>
> Haotian (cc-ed) in my team has looked into the needed schema changes to
> make Airflow 1.10 timestamp support to work with mysql without setting
> the exlicit_defaults_for_timestamp flag in mysql, details below:
>
> @@ -40,10 +40,6 @@ conn = op.get_bind() if conn.dialect.name ==
> 'mysql': conn.execute("SET time_zone = '+00:00'")- cur =
> conn.execute("SELECT @@explicit_defaults_for_timestamp")- res =
> cur.fetchall()- if res[0][0] == 0:- raise Exception("Global
> variable explicit_defaults_for_timestamp needs to be on (1) for mysql")
> op.alter_column(table_name='chart', column_name='last_modified',
> type_=mysql.TIMESTAMP(fsp=6))@@ -69,20 +65,28 @@
> op.alter_column(table_name='log', column_name='dttm',
> type_=mysql.TIMESTAMP(fsp=6)) op.alter_column(table_name='log',
> column_name='execution_date', type_=mysql.TIMESTAMP(fsp=6))-
> op.alter_column(table_name='sla_miss', column_name='execution_date',
> type_=mysql.TIMESTAMP(fsp=6), nullable=False)+
> op.alter_column(table_name='sla_miss', column_name='execution_date',
> type_=mysql.TIMESTAMP(fsp=6), \+ nullable=False,
> server_default=sa.text('CURRENT_TIMESTAMP(6)'))
> op.alter_column(table_name='sla_miss', column_name='timestamp',
> type_=mysql.TIMESTAMP(fsp=6))- op.alter_column(table_name='task_fail',
> column_name='execution_date', type_=mysql.TIMESTAMP(fsp=6))+
> op.alter_column(table_name='task_fail', column_name='execution_date',
> type_=mysql.TIMESTAMP(fsp=6), \+ nullable=False,
> server_default=sa.text('CURRENT_TIMESTAMP(6)'))
> op.alter_column(table_name='task_fail', column_name='start_date',
> type_=mysql.TIMESTAMP(fsp=6)) op.alter_column(table_name='task_fail',
> column_name='end_date', type_=mysql.TIMESTAMP(fsp=6))-
> op.alter_column(table_name='task_instance', column_name='execution_date',
> type_=mysql.TIMESTAMP(fsp=6), nullable=False)+
> op.alter_column(table_name='task_instance', column_name='execution_date',
> type_=mysql.TIMESTAMP(fsp=6), \+ nullable=False,
> server_default=sa.text('CURRENT_TIMESTAMP(6)'))
> op.alter_column(table_name='task_instance', column_name='start_date',
> type_=mysql.TIMESTAMP(fsp=6))
> op.alter_column(table_name='task_instance', column_name='end_date',
> type_=mysql.TIMESTAMP(fsp=6))
> op.alter_column(table_name='task_instance', column_name='queued_dttm',
> type_=mysql.TIMESTAMP(fsp=6))- op.alter_column(table_name='xcom',
> column_name='timestamp', type_=mysql.TIMESTAMP(fsp=6))-
> op.alter_column(table_name='xcom', column_name='execution_date',
> type_=mysql.TIMESTAMP(fsp=6))+ op.alter_column(table_name='xcom',
> column_name='timestamp', type_=mysql.TIMESTAMP(fsp=6), \+
> nullable=False, server_default=sa.text('CURRENT_TIMESTAMP(6)'))+
> op.alter_column(table_name='xcom', column_name='execution_date',
> type_=mysql.TIMESTAMP(fsp=6), \+ nullable=False,
> server_default=sa.text('CURRENT_TIMESTAMP(6)'))+ conn.execute("alter
> table task_instance alter column execution_date drop default")+
> conn.execute("alter table sla_miss alter column execution_date drop
> default")+ conn.execute("alter table task_fail alter column
> execution_date drop default") else: # sqlite datetime is fine as
> is not converting if conn.dialect.name == 'sqlite':---
> migrations/versions/f23433877c24_fix_mysql_not_null_constraint.py+++
> migrations/versions/f23433877c24_fix_mysql_not_null_constraint.py@@ -39,10
> +39,15 @@ conn = op.get_bind() if conn.dialect.name == 'mysql':
> conn.execute("SET time_zone = '+00:00'")-
> op.alter_column('task_fail', 'execution_date',
> existing_type=mysql.TIMESTAMP(fsp=6), nullable=False)-
> op.alter_column('xcom', 'execution_date',
> existing_type=mysql.TIMESTAMP(fsp=6), nullable=False)-
> op.alter_column('xcom', 'timestamp', existing_type=mysql.TIMESTAMP(fsp=6),
> nullable=False)-+ op.alter_column('task_fail', 'execution_date',
> existing_type=mysql.TIMESTAMP(fsp=6), \+ nullable=False,
> server_default=sa.text('CURRENT_TIMESTAMP(6)'))+
> op.alter_column('xcom', 'execution_date',
> existing_type=mysql.TIMESTAMP(fsp=6), \+ nullable=False,
> server_default=sa.text('CURRENT_TIMESTAMP(6)'))+
> op.alter_column('xcom', 'timestamp', existing_type=mysql.TIMESTAMP(fsp=6), \+
> nullable=False, server_default=sa.text('CURRENT_TIMESTAMP(6)'))+
> conn.execute("alter table task_fail alter column execution_date drop
> default")+ conn.execute("alter table xcom alter column execution_date
> drop default")+ conn.execute("alter table xcom alter column timestamp
> drop default")
>
>
> He will make a PR for this soon.
>
> Feng
>
> On Fri, Oct 19, 2018 at 10:26 AM Bolke de Bruin <bdbr...@gmail.com> wrote:
>
>> O and reading
>>
>>
>> https://dev.mysql.com/doc/refman/5.6/en/server-system-variables.html#sysvar_explicit_defaults_for_timestamp
>>
>> indicates that it can be set on the session level as well. So we could
>> just change the alembic scripts do try it. However
>> MariaDB does not support it in a session so we always need to check the
>> variable. We will also need to set it at *every*
>> alembic script that deals with datetimes in the future. Nevertheless this
>> might be the easiest solution.
>>
>> Does GCP’s MySQL also allow this setting in the session scope?
>>
>> B.
>>
>> On 19 Oct 2018, at 18:48, Deng Xiaodong <xd.den...@gmail.com> wrote:
>>
>> I'm ok to test this.
>>
>> @ash, may you kindly give some examples of what exact behaviour the
>> testers
>> should pay attention to? Since people like me may not know the full
>> background of having introduced this restriction & check, or what issue it
>> was trying to address.
>>
>> @Feng Lu, may you please advise if you are still interested to prepare
>> this
>> PR?
>>
>> Thanks!
>>
>>
>> XD
>>
>> On Sat, Oct 20, 2018 at 12:38 AM Ash Berlin-Taylor <a...@apache.org>
>> wrote:
>>
>> This sounds sensible and would mean we could also run on GCP's MySQL
>> offering too.
>>
>> This would need someone to try out and check that timezones behave
>> sensibly with this change made.
>>
>> Any volunteers?
>>
>> -ash
>>
>> On 19 Oct 2018, at 17:32, Deng Xiaodong <xd.den...@gmail.com> wrote:
>>
>> Wondering if there is any further thoughts about this proposal kindly
>>
>> raised by Feng Lu earlier?
>>
>>
>> If we can skip this check & allow explicit_defaults_for_timestamp to be
>>
>> 0, it would be helpful, especially for enterprise users in whose
>> environments it’s really hard to ask for a database global variable change
>> (like myself…).
>>
>>
>>
>> XD
>>
>> On 2018/08/28 15:23:10, Feng Lu <f...@google.com.INVALID> wrote:
>>
>> Bolke, a gentle ping..>
>> Thank you.>
>>
>> On Thu, Aug 23, 2018, 23:01 Feng Lu <fe...@google.com> wrote:>
>>
>> Hi all,>
>>
>>
>> After reading the MySQL documentation on the>
>> exlicit_defaults_for_timestamp, it appears that we can skip the check
>>
>> on explicit_defaults_for_timestamp>
>>
>> = 1>
>> <
>>
>>
>> https://github.com/apache/incubator-airflow/blob/master/airflow/migrations/versions/0e2a74e0fc9f_add_time_zone_awareness.py#L43
>> >
>> by>
>>
>> setting the column to accept NULL explicitly. For example:>
>>
>>
>> op.alter_column(table_name='chart', column_name='last_modified',>
>> type_=mysql.TIMESTAMP(fsp=6)) -->>
>> op.alter_column(table_name='chart', column_name='last_modified',>
>> type_=mysql.TIMESTAMP(fsp=6), nullable=True)>
>>
>>
>> Here's why:>
>> From MySQL doc (when explicit_defaults_for_timestamp is set to True):>
>> "TIMESTAMP columns not explicitly declared with the NOT NULL attribute
>>
>> are>
>>
>> automatically declared with the NULL attribute and permit NULL
>>
>> values.>
>>
>> Assigning such a column a value of NULL sets it to NULL, not the
>>
>> current>
>>
>> timestamp.">
>>
>>
>> Thanks and happy to shoot a PR if it makes sense.>
>>
>>
>> Feng>
>>
>>
>>
>>
>>
>>
>>
>>
