[FLINK-3856] [core] Create types for java.sql.Date/Time/Timestamp #1959
Conversation
|
Can these type infos exist independent of the BasicTypeInfo? |
|
These types would also be useful for |
| @Override | ||
| protected Timestamp[] getSortedTestData() { | ||
| return new Timestamp[] { | ||
| Timestamp.valueOf("1970-01-01 00:00:00.000"), |
There was a problem hiding this comment.
Add Timestamps that only differ in the nanos?
|
Just a few minor comments. What do you think about moving the types to a |
|
I will move it to |
twalthr
force-pushed
the
DateTimeTimestamp
branch
2 times, most recently
from
790ee12
to
182fabc
Compare
| * Type information for Java SQL Date/Time/Timestamp. | ||
| */ | ||
| @PublicEvolving | ||
| public class SqlTimeTypeInfo<T> extends TypeInformation<T> implements AtomicType<T> { |
There was a problem hiding this comment.
Do we want to implement a new TypeInformation which is basically a copy of BasicTypeInfo?
Alternatively we could make SqlTimeTypeInfo just a holder for three public static final BasicTypeInfos of the three time types. The protected constructor of BasicTypeInfo is visible if SqlTimeTypeInfo is in the same package as BasicTypeInfo.
There was a problem hiding this comment.
I also thought about that. But this would also mean that BasicTypeInfo's getInfoFor needs to support types that are declared in an other class. So we have to add the classes to the TYPES of BasicTypeInfo. I think complete separation is a nicer design.
There was a problem hiding this comment.
I agree. Let's stick to this solution then.
There was a problem hiding this comment.
Ok, I will fix the unused import issues and merge once travis build passed.
|
@twalthr, should |
twalthr
force-pushed
the
DateTimeTimestamp
branch
from
7e4d701
to
20246e9
Compare
|
Merging |
…me/Timestamp This closes apache#1959
…me/Timestamp This closes apache#1959
…me/Timestamp This closes apache#1959
Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the How To Contribute guide.
In addition to going through the list, please provide a meaningful description of your changes.
mvn clean verifyhas been executed successfully locally or a Travis build has passedThis PR adds java.sql.Date/Time/Timestamp as basic types. I declared them PublicEvolving, therefore I didn't add the types to the documentation. I improved the Date serialization to use Long.MIN_VALUE instead of -1. But it still does not solve FLINK-3858 completely.