You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Deng Xiaodong <xd...@gmail.com> on 2018/10/19 16:32:16 UTC

Re: explicit_defaults_for_timestamp for mysql

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> 
> >> 
> >> 
> >> 
> 

Re: explicit_defaults_for_timestamp for mysql

Posted by Bolke de Bruin <bd...@gmail.com>.
Please test this indeed. As mentioned I have tested this extensively and I could not make it work without.

A possible workaround for MySQL could be is that we revert to “plain” datetimes and assume MySQL always runs in UTC. In addition we could have the converter take care of any mismanagement by MySQL of datetimes.

B.

> On 19 Oct 2018, at 18:48, Deng Xiaodong <xd...@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 <as...@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...@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>
>>>>>> 
>>>>>> 
>>>>>> 
>> 
>> 


Re: explicit_defaults_for_timestamp for mysql

Posted by Bolke de Bruin <bd...@gmail.com>.
I’m ok with it if you say you are running it (prod?). We don’t use MySQL so I cannot vouch for it.

Cheers
Bolke

> On 29 Oct 2018, at 23:14, Feng Lu <fe...@google.com> wrote:
> 
> 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 <bdbruin@gmail.com <ma...@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 <fenglu@google.com <ma...@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 <http://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 <http://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 <http://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 <bdbruin@gmail.com <ma...@gmail.com>> wrote:
>> O and reading 
>> 
>> https://dev.mysql.com/doc/refman/5.6/en/server-system-variables.html#sysvar_explicit_defaults_for_timestamp <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.deng.r@gmail.com <ma...@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 <ash@apache.org <ma...@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.deng.r@gmail.com <ma...@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 <http://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 <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>
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>> 
>>>> 
>> 


Re: explicit_defaults_for_timestamp for mysql

Posted by Feng Lu <fe...@google.com.INVALID>.
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 <bd...@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 <fe...@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 <bd...@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...@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 <as...@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...@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>
>>
>>
>>
>>
>>
>>
>>
>>

Re: explicit_defaults_for_timestamp for mysql

Posted by Bolke de Bruin <bd...@gmail.com>.
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 <fe...@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 <bd...@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...@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 <as...@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...@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>
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>> 
>>>> 
>> 

Re: explicit_defaults_for_timestamp for mysql

Posted by Feng Lu <fe...@google.com.INVALID>.
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 <bd...@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...@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 <as...@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...@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>
>
>
>
>
>
>
>
>

Re: explicit_defaults_for_timestamp for mysql

Posted by Bolke de Bruin <bd...@gmail.com>.
O and reading 

https://dev.mysql.com/doc/refman/5.6/en/server-system-variables.html#sysvar_explicit_defaults_for_timestamp <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...@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 <as...@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...@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>
>>>>>> 
>>>>>> 
>>>>>> 
>> 
>> 


Re: explicit_defaults_for_timestamp for mysql

Posted by Deng Xiaodong <xd...@gmail.com>.
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 <as...@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...@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>
> >>>>
> >>>>
> >>>>
>
>

Re: explicit_defaults_for_timestamp for mysql

Posted by Ash Berlin-Taylor <as...@apache.org>.
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...@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> 
>>>> 
>>>> 
>>>>