Github user fpompermaier commented on the pull request:
https://github.com/apache/flink/pull/2007#issuecomment-221862570
Hi Stephan, thanks for reviewing this PR.
These are my reasons to not apply your suggestions to my PR, and I'd like
to hear your opinion (anyone interested by this discussion is also welcome)
before reissuing the PR with such modifications:
- rename `reset` to `resetErrorStateAndParse`: for almost all parsers this
doesn't do anything more than those 2 operations but what if for a further
parserImpl you need to do anything else before parsing?
- resetting the error state is something that must always be done, by all
parser implementations. The `parseFieldImpl` is a way to force this thing,
otherwise you should rely on classes using such parser to remember this thing
(as you suggested with "GenericCsvInputCormat would call the
resetErrorStateAndParse") or ensure that every parser call it at the very
beginning of the `parseField()`.
I'm really concerned about this. In this way, my PR doesn't break the
current APIs
Does it makes sense?
---
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.
---
