[ 
https://issues.apache.org/jira/browse/FLINK-1512?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14377000#comment-14377000
 ] 

ASF GitHub Bot commented on FLINK-1512:
---------------------------------------

Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/426#issuecomment-85261367
  
    Hi @chiwanpark, the PR improved a lot! Besides a few inline comments, the 
logic and test coverage looks pretty good. Unfortunately, the PR does not seem 
to build correctly (Check 
[Travis](https://travis-ci.org/apache/flink/builds/54853098)). I'll do another 
quick pass tomorrow and have a closer look at the error messages. 
    
    IMO, there are just two things left to be discussed before we can test it 
on a cluster:
    
    **Order of POJO fields**
    This is actually a crucial point. We need to make sure, that the field 
order is deterministic for all setups. I think (not sure!) right now, the order 
of the fields depends on the order in which the fields are returned by Java's 
reflection tools. This order needs to be standardized to ensure that all JVMs 
that obey the standard are compatible. However, even a standardized order 
(lexicographic order) might not be very helpful.
    
    There are several options here:
    1. If there the order of fields is standardized, just use that by default.
    2. If not, we can deterministically sort the fields ourselves in the 
PojoTypeInfo.
    3. We make the definition of a field order mandatory until we can define 
the order of POJO fields (e.g., via the proposed `@Position` annotation).
    I am leaning towards option 3. 
    
    **Changed Inheritance of ScalaCsvInputFormat**
    This is a very good observation, however I would keep the original 
inheritance for now. At the moment, GenericCsvInputFormat is extended by 
CsvInputFormat (for Java API), ScalaCsvInputFormat (for Scala API), 
CsvInputFormat (for Record API), and a test class. We will soon remove the 
RecordAPI such that only the Java and the Scala API will remain. I think we can 
then move all common operations to GenericCsvInputFormat (maybe we can do that 
already...). @chiwanpark does that make sense to you or do you disagree?
    
    @chiwanpark What do you think? Any other opinions?


> Add CsvReader for reading into POJOs.
> -------------------------------------
>
>                 Key: FLINK-1512
>                 URL: https://issues.apache.org/jira/browse/FLINK-1512
>             Project: Flink
>          Issue Type: New Feature
>          Components: Java API, Scala API
>            Reporter: Robert Metzger
>            Assignee: Chiwan Park
>            Priority: Minor
>              Labels: starter
>
> Currently, the {{CsvReader}} supports only TupleXX types. 
> It would be nice if users were also able to read into POJOs.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to