Thanks Leonard for the further investigation. I think we all agree we should correct the return value of CURRENT_TIMESTAMP. Regarding the return type of CURRENT_TIMESTAMP, I also agree TIMESTAMP_LTZ would be more worldwide useful. This may need more effort, but if this is the right direction, we should do it.
Regarding the CURRENT_TIME, if CURRENT_TIMESTAMP returns TIMESTAMP_LTZ, then I think CURRENT_TIME shouldn't return TIME_TZ. Otherwise, CURRENT_TIME will be quite special and strange. Thus I think it has to return TIME type. Given that we already have CURRENT_DATE which returns DATE WITHOUT TIME ZONE, I think it's fine to return TIME WITHOUT TIME ZONE for CURRENT_TIME. In a word, the updated FLIP looks good to me. I especially like the proposed new function TO_TIMESTAMP_LTZ(numeric, [,scale]). This will be very convenient to define rowtime on a long value which is a very common case and has been complained a lot in mailing list. Best, Jark On Mon, 25 Jan 2021 at 21:12, Kurt Young <ykt...@gmail.com> wrote: > Thanks Leonard for the detailed response and also the bad case about option > 1, these all > make sense to me. > > Also nice catch about conversion support of LocalZonedTimestampType, I > think it actually > makes sense to support java.sql.Timestamp as well as > java.time.LocalDateTime. It also has > a slight benefit that we might have a chance to run the udf which took them > as input parameter > after we change the return type. > > Regarding to the return type of CURRENT_TIME, I also think timezone > information is not useful. > To not expand this FLIP further, I'm lean to keep it as it is. > > Best, > Kurt > > > On Mon, Jan 25, 2021 at 8:50 PM Leonard Xu <xbjt...@gmail.com> wrote: > > > Hi, All > > > > Thanks for your comments. I think all of the thread have agreed that: > > (1) The return values of CURRENT_TIME/CURRENT_TIMESTAMP/NOW()/PROCTIME() > > are wrong. > > (2) The LOCALTIME/LOCALTIMESTAMP and CURRENT_TIME/CURRENT_TIMESTAMP > should > > be different whether from SQL standard’s perspective or mature systems. > > (3) The semantics of three TIMESTAMP types in Flink SQL follows the SQL > > standard and also keeps the same with other 'good' vendors. > > TIMESTAMP => A literal in > > ‘yyyy-MM-dd HH:mm:ss’ format to describe a time, does not contain > timezone > > info, can not represent an absolute time point. > > TIMESTAMP WITH LOCAL ZONE => Records the elapsed time from absolute > > time point origin, can represent an absolute time point, requires local > > time zone when expressed with ‘yyyy-MM-dd HH:mm:ss’ format. > > TIMESTAMP WITH TIME ZONE => Consists of time zone info and a > > literal in ‘yyyy-MM-dd HH:mm:ss’ format to describe time, can represent > an > > absolute time point. > > > > > > Currently we've two ways to correct > > CURRENT_TIME/CURRENT_TIMESTAMP/NOW()/PROCTIME(). > > > > option (1): As the FLIP proposed, change the return value from UTC > > timezone to local timezone. > > Pros: (1) The change looks smaller to users and developers (2) > > There're many SQL engines adopted this way > > Cons: (1) connector devs may confuse the underlying value of > > TimestampData which needs to change according to data type (2) I thought > > about this weekend. Unfortunately I found a bad case: > > > > The proposal is fine if we only use it in FLINK SQL world, but we need to > > consider the conversion between Table/DataStream, assume a record > produced > > in UTC+0 timezone with TIMESTAMP '1970-01-01 08:00:44' and the Flink SQL > > processes the data with session time zone 'UTC+8', if the sql program > need > > to convert the Table to DataStream, then we need to calculate the > timestamp > > in StreamRecord with session time zone (UTC+8), then we will get 44 in > > DataStream program, but it is wrong because the expected value should be > (8 > > * 60 * 60 + 44). The corner case tell us that the ROWTIME/PROCTIME in > Flink > > are based on UTC+0, when correct the PROCTIME() function, the better way > is > > to use TIMESTAMP WITH LOCAL TIME ZONE which keeps same long value with > time > > based on UTC+0 and can be expressed with local timezone. > > > > option (2) : As we considered in the FLIP as well as @Timo suggested, > > change the return type to TIMESTAMP WITH LOCAL TIME ZONE, the expressed > > value depends on the local time zone. > > Pros: (1) Make Flink SQL more close to SQL standard (2) Can deal > > the conversion between Table/DataStream well > > Cons: (1) We need to discuss the return value/type of > CURRENT_TIME > > function (2) The change is bigger to users, we need to support TIMESTAMP > > WITH LOCAL TIME ZONE in connectors/formats as well as custom connectors. > > (3)The TIMESTAMP WITH LOCAL TIME ZONE support is weak > > in Flink, thus we need some improvement，but the workload does not matter > > as long as we are doing the right thing ^_^ > > > > Due to the above bad case for option (1). I think option 2 should be > > adopted, > > But we also need to consider some problems: > > (1) More conversion classes like LocalDateTime, sql.Timestamp should be > > supported for LocalZonedTimestampType to resolve the UDF compatibility > issue > > (2) The timezone offset for window size of one day should still be > > considered > > (3) All connectors/formats should supports TIMESTAMP WITH LOCAL TIME ZONE > > well and we also should record in document > > I’ll update these sections of FLIP-162. > > > > > > > > We also need to discuss the CURRENT_TIME function. I know the standard > way > > is using TIME WITH TIME ZONE(there's no TIME WITH LOCAL TIME ZONE), but > we > > don't support this type yet and I don't see strong motivation to support > it > > so far. > > Compared to CURRENT_TIMESTAMP, the CURRENT_TIME can not represent an > > absolute time point which should be considered as a string consisting of > a > > time with 'HH:mm:ss' format and time zone info. We have several options > > for this: > > (1) We can forbid CURRENT_TIME as @Timo proposed to make all Flink SQL > > functions follow the standard well, in this way, we need to offer some > > guidance for user upgrading Flink versions. > > (2) We can also support it from a user's perspective who has used > > CURRENT_DATE/CURRENT_TIME/CURRENT_TIMESTAMP, btw，Snowflake also returns > > TIME type. > > (3) Returns TIMESTAMP WITH LOCAL TIME ZONE to make it equal to > > CURRENT_TIMESTAMP as Calcite did. > > > > I can image (1) which we don't want to left a bad smell in Flink SQL, > and > > I also accept (2) because I think users do not consider time zone issues > > when they use CURRENT_DATE/CURRENT_TIME, and the timezone info in time is > > not very useful. > > > > I don’t have a strong opinion for them. What do others think? > > > > > > I hope I've addressed your concerns. @Timo @Kurt > > > > Best， > > Leonard > > > > > > > > > Most of the mature systems have a clear difference between > > CURRENT_TIMESTAMP and LOCALTIMESTAMP. I wouldn't take Spark or Hive as a > > good example. Snowflake decided for TIMESTAMP WITH LOCAL TIME ZONE. As I > > mentioned in the last comment, I could also imagine this behavior for > > Flink. But in any case, there should be some time zone information > > considered in order to cast to all other types. > > > > > > >>> The function CURRENT_DATE/CURRENT_TIME is supporting in SQL > > standard, but > > > >>> LOCALDATE not, I don’t think it’s a good idea that dropping > > functions which > > > >>> SQL standard supported and introducing a replacement which SQL > > standard not > > > >>> reminded. > > > > > > We can still add those functions in the future. But since we don't > offer > > a TIME WITH TIME ZONE, it is better to not support this function at all > for > > now. And by the way, this is exactly the behavior that also Microsoft SQL > > Server does: it also just supports CURRENT_TIMESTAMP (but it returns > > TIMESTAMP without a zone which completes the confusion). > > > > > > >>> I also agree returning TIMESTAMP WITH LOCAL TIME ZONE for PROCTIME > > has > > > >>> more clear semantics, but I realized that user didn’t care the type > > but > > > >>> more about the expressed value they saw, and change the type from > > TIMESTAMP > > > >>> to TIMESTAMP WITH LOCAL TIME ZONE brings huge refactor that we need > > > >>> consider all places where the TIMESTAMP type used > > > > > > From a UDF perspective, I think nothing will change. The new type > system > > and type inference were designed to support all these cases. There is a > > reason why Java has adopted Joda time, because it is hard to come up > with a > > good time library. That's why also we and the other Hadoop ecosystem > folks > > have decided for 3 different kinds of LocalDateTime, ZonedDateTime, and > > Instance. It makes the library more complex, but time is a complex topic. > > > > > > I also doubt that many users work with only one time zone. Take the US > > as an example, a country with 3 different timezones. Somebody working > with > > US data cannot properly see the data points with just LOCAL TIME ZONE. > But > > on the other hand, a lot of event data is stored using a UTC timestamp. > > > > > > > > > >> Before jumping into technique details, let's take a step back to > > discuss > > > >> user experience. > > > >> > > > >> The first important question is what kind of date and time will > Flink > > > >> display when users call > > > >> CURRENT_TIMESTAMP and maybe also PROCTIME (if we think they are > > similar). > > > >> > > > >> Should it always display the date and time in UTC or in the user's > > time > > > >> zone? > > > > > > @Kurt: I think we all agree that the current behavior with just showing > > UTC is wrong. Also, we all agree that when calling CURRENT_TIMESTAMP or > > PROCTIME a user would like to see the time in it's current time zone. > > > > > > As you said, "my wall clock time". > > > > > > However, the question is what is the data type of what you "see". If > you > > pass this record on to a different system, operator, or different > cluster, > > should the "my" get lost or materialized into the record? > > > > > > TIMESTAMP -> completely lost and could cause confusion in a different > > system > > > > > > TIMESTAMP WITH LOCAL TIME ZONE -> at least the UTC is correct, so you > > can provide a new local time zone > > > > > > TIMESTAMP WITH TIME ZONE -> also "your" location is persisted > > > > > > Regards, > > > Timo > > > > > > > > > > > > > > > On 22.01.21 09:38, Kurt Young wrote: > > >> Forgot one more thing. Continue with displaying in UTC. As a user, if > > Flink > > >> want to display the timestamp > > >> in UTC, why don't we offer something like UTC_TIMESTAMP? > > >> Best, > > >> Kurt > > >> On Fri, Jan 22, 2021 at 4:33 PM Kurt Young <ykt...@gmail.com> wrote: > > >>> Before jumping into technique details, let's take a step back to > > discuss > > >>> user experience. > > >>> > > >>> The first important question is what kind of date and time will Flink > > >>> display when users call > > >>> CURRENT_TIMESTAMP and maybe also PROCTIME (if we think they are > > similar). > > >>> > > >>> Should it always display the date and time in UTC or in the user's > time > > >>> zone? I think this part is the > > >>> reason that surprised lots of users. If we forget about the type and > > >>> internal representation of these > > >>> two methods, as a user, my instinct tells me that these two methods > > should > > >>> display my wall clock time. > > >>> > > >>> Display time in UTC? I'm not sure, why I should care about UTC time? > I > > >>> want to get my current timestamp. > > >>> For those users who have never gone abroad, they might not even be > > able to > > >>> realize that this is affected > > >>> by the time zone. > > >>> > > >>> Best, > > >>> Kurt > > >>> > > >>> > > >>> On Fri, Jan 22, 2021 at 12:25 PM Leonard Xu <xbjt...@gmail.com> > wrote: > > >>> > > >>>> Thanks @Timo for the detailed reply, let's go on this topic on this > > >>>> discussion, I've merged all mails to this discussion. > > >>>> > > >>>>> LOCALDATE / LOCALTIME / LOCALTIMESTAMP > > >>>>> > > >>>>> --> uses session time zone, returns DATE/TIME/TIMESTAMP > > >>>> > > >>>>> > > >>>>> CURRENT_DATE/CURRENT_TIME/CURRENT_TIMESTAMP > > >>>>> > > >>>>> --> uses session time zone, returns DATE/TIME/TIMESTAMP > > >>>>> > > >>>>> I'm very sceptical about this behavior. Almost all mature systems > > >>>> (Oracle, Postgres) and new high quality systems (Presto, Snowflake) > > use a > > >>>> data type with some degree of time zone information encoded. In a > > >>>> globalized world with businesses spanning different regions, I think > > we > > >>>> should do this as well. There should be a difference between > > >>>> CURRENT_TIMESTAMP and LOCALTIMESTAMP. And users should be able to > > choose > > >>>> which behavior they prefer for their pipeline. > > >>>> > > >>>> > > >>>> I know that the two series should be different at first glance, but > > >>>> different SQL engines can have their own explanations，for example, > > >>>> CURRENT_TIMESTAMP and LOCALTIMESTAMP are synonyms in Snowflake[1] > and > > has > > >>>> no difference, and Spark only supports the later one and doesn’t > > support > > >>>> LOCALTIME/LOCALTIMESTAMP[2]. > > >>>> > > >>>> > > >>>>> If we would design this from scatch, I would suggest the following: > > >>>>> > > >>>>> - drop CURRENT_DATE / CURRENT_TIME and let users pick LOCALDATE / > > >>>> LOCALTIME for materialized timestamp parts > > >>>> > > >>>> The function CURRENT_DATE/CURRENT_TIME is supporting in SQL > standard, > > but > > >>>> LOCALDATE not, I don’t think it’s a good idea that dropping > functions > > which > > >>>> SQL standard supported and introducing a replacement which SQL > > standard not > > >>>> reminded. > > >>>> > > >>>> > > >>>>> - CURRENT_TIMESTAMP should return a TIMESTAMP WITH TIME ZONE to > > >>>> materialize all session time information into every record. It it > the > > most > > >>>> generic data type and allows to cast to all other timestamp data > > types. > > >>>> This generic ability can be used for filter predicates as well > either > > >>>> through implicit or explicit casting. > > >>>> > > >>>> TIMESTAMP WITH TIME ZONE indeed contains more information to > describe > > a > > >>>> time point, but the type TIMESTAMP can cast to all other timestamp > > data > > >>>> types combining with session time zone as well, and it also can be > > used for > > >>>> filter predicates. For type casting between BIGINT and TIMESTAMP, I > > think > > >>>> the function way using TO_TIMEMTAMP()/FROM_UNIXTIMESTAMP() is more > > clear. > > >>>> > > >>>>> PROCTIME/ROWTIME should be time functions based on a long value. > Both > > >>>> System.currentMillis() and our watermark system work on long values. > > Those > > >>>> should return TIMESTAMP WITH LOCAL TIME ZONE because the main > > calculation > > >>>> should always happen based on UTC. > > >>>>> We discussed it in a different thread, but we should allow PROCTIME > > >>>> globally. People need a way to create instances of TIMESTAMP WITH > > LOCAL > > >>>> TIME ZONE. This is not considered in the current design doc. > > >>>>> Many pipelines contain UTC timestamps and thus it should be easy to > > >>>> create one. > > >>>>> Also, both CURRENT_TIMESTAMP and LOCALTIMESTAMP can work with this > > type > > >>>> because we should remember that TIMESTAMP WITH LOCAL TIME ZONE > > accepts all > > >>>> timestamp data types as casting target [1]. We could allow TIMESTAMP > > WITH > > >>>> TIME ZONE in the future for ROWTIME. > > >>>>> In any case, windows should simply adapt their behavior to the > passed > > >>>> timestamp type. And with TIMESTAMP WITH LOCAL TIME ZONE a day is > > defined by > > >>>> considering the current session time zone. > > >>>> > > >>>> I also agree returning TIMESTAMP WITH LOCAL TIME ZONE for PROCTIME > > has > > >>>> more clear semantics, but I realized that user didn’t care the type > > but > > >>>> more about the expressed value they saw, and change the type from > > TIMESTAMP > > >>>> to TIMESTAMP WITH LOCAL TIME ZONE brings huge refactor that we need > > >>>> consider all places where the TIMESTAMP type used, and many builtin > > >>>> functions and UDFs doest not support TIMESTAMP WITH LOCAL TIME ZONE > > type. > > >>>> That means both user and Flink devs need to refactor the code(UDF, > > builtin > > >>>> functions, sql pipeline), to be honest, I didn’t see strong > > motivation that > > >>>> we have to do the pretty big refactor from user’s perspective and > > >>>> developer’s perspective. > > >>>> > > >>>> In one word, both your suggestion and my proposal can resolve almost > > all > > >>>> user problems，the divergence is whether we need to spend pretty > > energy just > > >>>> to get a bit more accurate semantics? I think we need a tradeoff. > > >>>> > > >>>> > > >>>> Best, > > >>>> Leonard > > >>>> [1] > > >>>> > > https://trino.io/docs/current/functions/datetime.html#current_timestamp > < > > >>>> > > https://trino.io/docs/current/functions/datetime.html#current_timestamp> > > >>>> [2] https://issues.apache.org/jira/browse/SPARK-30374 < > > >>>> https://issues.apache.org/jira/browse/SPARK-30374> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>>> 2021-01-22，00:53，Timo Walther <twal...@apache.org> ： > > >>>>> > > >>>>> Hi Leonard, > > >>>>> > > >>>>> thanks for working on this topic. I agree that time handling is not > > >>>> easy in Flink at the moment. We added new time data types (and some > > are > > >>>> still not supported which even further complicates things like > > TIME(9)). We > > >>>> should definitely improve this situation for users. > > >>>>> > > >>>>> This is a pretty opinionated topic and it seems that the SQL > standard > > >>>> is not really deciding this but is at least supporting. So let me > > express > > >>>> my opinion for the most important functions: > > >>>>> > > >>>>> LOCALDATE / LOCALTIME / LOCALTIMESTAMP > > >>>>> > > >>>>> --> uses session time zone, returns DATE/TIME/TIMESTAMP > > >>>>> > > >>>>> I think those are the most obvious ones because the LOCAL indicates > > >>>> that the locality should be materialized into the result and any > time > > zone > > >>>> information (coming from session config or data) is not important > > >>>> afterwards. > > >>>>> > > >>>>> CURRENT_DATE/CURRENT_TIME/CURRENT_TIMESTAMP > > >>>>> > > >>>>> --> uses session time zone, returns DATE/TIME/TIMESTAMP > > >>>>> > > >>>>> I'm very sceptical about this behavior. Almost all mature systems > > >>>> (Oracle, Postgres) and new high quality systems (Presto, Snowflake) > > use a > > >>>> data type with some degree of time zone information encoded. In a > > >>>> globalized world with businesses spanning different regions, I think > > we > > >>>> should do this as well. There should be a difference between > > >>>> CURRENT_TIMESTAMP and LOCALTIMESTAMP. And users should be able to > > choose > > >>>> which behavior they prefer for their pipeline. > > >>>>> > > >>>>> If we would design this from scatch, I would suggest the following: > > >>>>> > > >>>>> - drop CURRENT_DATE / CURRENT_TIME and let users pick LOCALDATE / > > >>>> LOCALTIME for materialized timestamp parts > > >>>>> > > >>>>> - CURRENT_TIMESTAMP should return a TIMESTAMP WITH TIME ZONE to > > >>>> materialize all session time information into every record. It it > the > > most > > >>>> generic data type and allows to cast to all other timestamp data > > types. > > >>>> This generic ability can be used for filter predicates as well > either > > >>>> through implicit or explicit casting. > > >>>>> > > >>>>> PROCTIME/ROWTIME should be time functions based on a long value. > Both > > >>>> System.currentMillis() and our watermark system work on long values. > > Those > > >>>> should return TIMESTAMP WITH LOCAL TIME ZONE because the main > > calculation > > >>>> should always happen based on UTC. We discussed it in a different > > thread, > > >>>> but we should allow PROCTIME globally. People need a way to create > > >>>> instances of TIMESTAMP WITH LOCAL TIME ZONE. This is not considered > > in the > > >>>> current design doc. Many pipelines contain UTC timestamps and thus > it > > >>>> should be easy to create one. Also, both CURRENT_TIMESTAMP and > > >>>> LOCALTIMESTAMP can work with this type because we should remember > that > > >>>> TIMESTAMP WITH LOCAL TIME ZONE accepts all timestamp data types as > > casting > > >>>> target [1]. We could allow TIMESTAMP WITH TIME ZONE in the future > for > > >>>> ROWTIME. > > >>>>> > > >>>>> In any case, windows should simply adapt their behavior to the > passed > > >>>> timestamp type. And with TIMESTAMP WITH LOCAL TIME ZONE a day is > > defined by > > >>>> considering the current session time zone. > > >>>>> > > >>>>> If we would like to design this with less effort required, we could > > >>>> think about returning TIMESTAMP WITH LOCAL TIME ZONE also for > > >>>> CURRENT_TIMESTAMP. > > >>>>> > > >>>>> > > >>>>> I will try to involve more people into this discussion. > > >>>>> > > >>>>> Thanks, > > >>>>> Timo > > >>>>> > > >>>>> [1] > > >>>> > > > https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf/Data-Types.html#GUID-E7CA339A-2093-4FE4-A36E-1D09593591D3 > > >>>> < > > >>>> > > > https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf/Data-Types.html#GUID-E7CA339A-2093-4FE4-A36E-1D09593591D3 > > >>>>> > > >>>> > > >>>> > > >>>> > > >>>>> 2021-01-21，22:32，Leonard Xu <xbjt...@gmail.com> ： > > >>>>>> Before the changes, as I am writing this reply, the local time > here > > is > > >>>> 2021-01-21 12:03:35 (Beijing time, UTC+8). > > >>>>>> And I tried these 5 functions in sql client, and got: > > >>>>>> > > >>>>>> Flink SQL> select now(), PROCTIME(), CURRENT_TIMESTAMP, > > CURRENT_DATE, > > >>>> CURRENT_TIME; > > >>>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>>> | EXPR$0 | EXPR$1 | > > >>>> CURRENT_TIMESTAMP | CURRENT_DATE | CURRENT_TIME | > > >>>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>>> | 2021-01-21T04:03:35.228 | 2021-01-21T04:03:35.228 | > > >>>> 2021-01-21T04:03:35.228 | 2021-01-21 | 04:03:35.228 | > > >>>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>>> After the changes, the expected behavior will change to: > > >>>>>> > > >>>>>> Flink SQL> select now(), PROCTIME(), CURRENT_TIMESTAMP, > > CURRENT_DATE, > > >>>> CURRENT_TIME; > > >>>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>>> | EXPR$0 | EXPR$1 | > > >>>> CURRENT_TIMESTAMP | CURRENT_DATE | CURRENT_TIME | > > >>>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>>> | 2021-01-21T12:03:35.228 | 2021-01-21T12:03:35.228 | > > >>>> 2021-01-21T12:03:35.228 | 2021-01-21 | 12:03:35.228 | > > >>>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>>> The return type of now(), proctime() and CURRENT_TIMESTAMP still > be > > >>>> TIMESTAMP; > > >>>>> > > >>>>> To Kurt, thanks for the intuitive case, it really clear, you’re > > wright > > >>>> that I want to propose to change the return value of these > functions. > > It’s > > >>>> the most important part of the topic from user's perspective. > > >>>>> > > >>>>>> I think this definitely deserves a FLIP. > > >>>>> To Jark, nice suggestion, I prepared a FLIP for this topic, and > will > > >>>> start the FLIP discussion soon. > > >>>>> > > >>>>>>> If use the default Flink SQL, the window time range of the > > >>>>>>> statistics is incorrect, then the statistical results will > > naturally > > >>>> be > > >>>>>>> incorrect. > > >>>>> To zhisheng, sorry to hear that this problem influenced your > > production > > >>>> jobs, Could you share your SQL pattern? we can have more inputs > and > > try > > >>>> to resolve them. > > >>>>> > > >>>>> > > >>>>> Best, > > >>>>> Leonard > > >>>> > > >>>> > > >>>> > > >>>>> 2021-01-21，14:19，Jark Wu <imj...@gmail.com> ： > > >>>>> > > >>>>> Great examples to understand the problem and the proposed changes, > > >>>> @Kurt! > > >>>>> > > >>>>> Thanks Leonard for investigating this problem. > > >>>>> The time-zone problems around time functions and windows have > > bothered a > > >>>>> lot of users. It's time to fix them! > > >>>>> > > >>>>> The return value changes sound reasonable to me, and keeping the > > return > > >>>>> type unchanged will minimize the surprise to the users. > > >>>>> Besides that, I think it would be better to mention how this > affects > > the > > >>>>> window behaviors, and the interoperability with DataStream. > > >>>>> > > >>>>> I think this definitely deserves a FLIP. > > >>>>> > > >>>>> ==================================================== > > >>>>> > > >>>>> Hi zhisheng, > > >>>>> > > >>>>> Do you have examples to illustrate which case will get the wrong > > window > > >>>>> boundaries? > > >>>>> That will help to verify whether the proposed changes can solve > your > > >>>>> problem. > > >>>>> > > >>>>> Best, > > >>>>> Jark > > >>>> > > >>>> > > >>>> > > >>>> > > >>>>> 2021-01-21，12:54，zhisheng <173855...@qq.com> ： > > >>>>> > > >>>>> Thanks to Leonard Xu for discussing this tricky topic. At present, > > >>>> there are many Flink jobs in our production environment that are > used > > to > > >>>> count day-level reports (eg: count PV/UV ). > > >>>>> > > >>>>> If use the default Flink SQL, the window time range of the > > >>>> statistics is incorrect, then the statistical results will naturally > > be > > >>>> incorrect. > > >>>>> > > >>>>> The user needs to deal with the time zone manually in order to > solve > > >>>> the problem. > > >>>>> > > >>>>> If Flink itself can solve these time zone issues, then I think it > > will > > >>>> be user-friendly. > > >>>>> > > >>>>> Thank you > > >>>>> > > >>>>> Best!; > > >>>>> zhisheng > > >>>> > > >>>> > > >>>> > > >>>> > > >>>>> 2021-01-21，12:11，Kurt Young <ykt...@gmail.com> ： > > >>>>> > > >>>>> cc this to user & user-zh mailing list because this will affect > lots > > of > > >>>> users, and also quite a lot of users > > >>>>> were asking questions around this topic. > > >>>>> > > >>>>> Let me try to understand this from user's perspective. > > >>>>> > > >>>>> Your proposal will affect five functions, which are: > > >>>>> PROCTIME() > > >>>>> NOW() > > >>>>> CURRENT_DATE > > >>>>> CURRENT_TIME > > >>>>> CURRENT_TIMESTAMP > > >>>>> Before the changes, as I am writing this reply, the local time here > > is > > >>>> 2021-01-21 12:03:35 (Beijing time, UTC+8). > > >>>>> And I tried these 5 functions in sql client, and got: > > >>>>> > > >>>>> Flink SQL> select now(), PROCTIME(), CURRENT_TIMESTAMP, > CURRENT_DATE, > > >>>> CURRENT_TIME; > > >>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>> | EXPR$0 | EXPR$1 | > > >>>> CURRENT_TIMESTAMP | CURRENT_DATE | CURRENT_TIME | > > >>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>> | 2021-01-21T04:03:35.228 | 2021-01-21T04:03:35.228 | > > >>>> 2021-01-21T04:03:35.228 | 2021-01-21 | 04:03:35.228 | > > >>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>> After the changes, the expected behavior will change to: > > >>>>> > > >>>>> Flink SQL> select now(), PROCTIME(), CURRENT_TIMESTAMP, > CURRENT_DATE, > > >>>> CURRENT_TIME; > > >>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>> | EXPR$0 | EXPR$1 | > > >>>> CURRENT_TIMESTAMP | CURRENT_DATE | CURRENT_TIME | > > >>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>> | 2021-01-21T12:03:35.228 | 2021-01-21T12:03:35.228 | > > >>>> 2021-01-21T12:03:35.228 | 2021-01-21 | 12:03:35.228 | > > >>>>> > > >>>> > > > +-------------------------+-------------------------+-------------------------+--------------+--------------+ > > >>>>> The return type of now(), proctime() and CURRENT_TIMESTAMP still be > > >>>> TIMESTAMP; > > >>>>> > > >>>>> Best, > > >>>>> Kurt > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > > > > > > >
