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?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
