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 <fen...@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 <bdbr...@gmail.com > <mailto: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 > <mailto: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 <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 <bdbr...@gmail.com >> <mailto: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 >> >> <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 >>> <mailto: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 >>> <mailto: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 >>>>> <mailto: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 >>>>>> <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> >>>>>>>> >>>>>>>> >>>>>>>> >>>> >>>> >>
