Github user fhueske commented on the issue:

    https://github.com/apache/flink/pull/6253
  
    It's a good question whether to add a new class or not. Right now, 
extending the implementation of `ExistingField` seems like a better approach to 
me. Once we add a `ParsingExistingField` extractor that can be configured with 
a timestamp format, the ISO date String support in `ExistingField` would be 
obsolete, but IMO that's not a big problem. 
    
    +1 to change the implementation to extend `ExistingField`.
    
    Regarding the tests, I think `ExistingField` is used in a few ITCases, but 
there are no unit tests yet.
    Adding unit tests is a bit tricky, because we would need to integrate it 
with the code generator, etc.
    So, a big +1 if you would like to look into that, but I'd also be fine by 
adding another ITCase.
    
    For the documentation, we might want to extend the bullet point about 
`timestampExtractor` in the [Defining a Rowtime 
Attribute](https://ci.apache.org/projects/flink/flink-docs-release-1.5/dev/table/sourceSinks.html#defining-a-rowtime-attribute)
 section.
    
    Thanks, Fabian


---

Reply via email to