+1

I think we can mark the temporary config option as deprecated when we
introduce it.
So we can remove it after 2 minor releases (1.19, 1.20), i.e., drop in 2.0
release.

A minor comment about the config option, I would suggest to use
"avro.timestamp-mapping.legacy"
instead of "avro.timestamp_mapping.legacy". Because Flink community prefer
dash than underline[1].

Best.,
Jark

[1]:
https://flink.apache.org/how-to-contribute/code-style-and-quality-components/#configuration-changes

On Sat, 4 Nov 2023 at 21:40, Gyula Fóra <gyula.f...@gmail.com> wrote:

> +1
>
> Gyula
>
> On Thu, Nov 2, 2023 at 6:18 AM Martijn Visser <martijnvis...@apache.org>
> wrote:
>
> > +1
> >
> > On Thu, Nov 2, 2023 at 12:44 PM Leonard Xu <xbjt...@gmail.com> wrote:
> > >
> > >
> > > > Thanks @Leonard Xu <mailto:xbjt...@gmail.com>. Two minor versions
> are
> > definitely needed for flip the configs.
> > >
> > > Sorry, Peter. I thought the next minor versions are 1.19、2.0, but
> > actually it should be 1.19、1.20、2.0 from current community version plan
> > IIUC, so remove the config in 2.0 should be okay if the 1.20 version
> exists
> > .
> > >
> > > Best,
> > > Leonard
> > >
> > >
> > > >
> > > > On Mon, Oct 30, 2023 at 8:55 PM Leonard Xu <xbjt...@gmail.com
> <mailto:
> > xbjt...@gmail.com>> wrote:
> > > > Thanks @Peter for driving this FLIP
> > > >
> > > > +1 from my side, the timestamp semantics mapping looks good to me.
> > > >
> > > > >  In the end, the legacy behavior will be dropped in
> > > > > Flink 2.0
> > > > > I don’t think we can drop this option which introduced in 1.19 and
> > drop in 2.0, the API removal requires at least two minor versions.
> > > >
> > > >
> > > > Best,
> > > > Leonard
> > > >
> > > > > 2023年10月31日 上午11:18,Peter Huang <huangzhenqiu0...@gmail.com
> <mailto:
> > huangzhenqiu0...@gmail.com>> 写道:
> > > > >
> > > > > Hi Devs,
> > > > >
> > > > > Currently, Flink Avro Format doesn't support the Avro time
> > (milli/micros)
> > > > > with local timezone type.
> > > > > Although the Avro timestamp (millis/micros) type is supported and
> is
> > mapped
> > > > > to flink timestamp without timezone,
> > > > > it is not compliant to semantics defined in Consistent timestamp
> > types in
> > > > > Hadoop SQL engines
> > > > > <
> >
> https://docs.google.com/document/d/1gNRww9mZJcHvUDCXklzjFEQGpefsuR_akCDfWsdE35Q/edit#heading=h.n699ftkvhjlo
> > <
> >
> https://docs.google.com/document/d/1gNRww9mZJcHvUDCXklzjFEQGpefsuR_akCDfWsdE35Q/edit#heading=h.n699ftkvhjlo
> > >>
> > > > > .
> > > > >
> > > > > I propose to support Avro timestamps with the compliance to the
> > mapping
> > > > > semantics [1] by using a configuration flag.
> > > > > To keep back compatibility, current mapping is kept as default
> > behavior.
> > > > > Users can explicitly turn on the new mapping
> > > > > by setting it to false. In the end, the legacy behavior will be
> > dropped in
> > > > > Flink 2.0
> > > > >
> > > > > Looking forward to your feedback.
> > > > >
> > > > >
> > > > > [1]
> > > > >
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-378%3A+Support+Avro+timestamp+with+local+timezone
> > <
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-378%3A+Support+Avro+timestamp+with+local+timezone
> > >
> > > > >
> > > > >
> > > > > Best Regards
> > > > >
> > > > > Peter Huang
> > > >
> > >
> >
>

Reply via email to