You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by twalthr <gi...@git.apache.org> on 2018/05/17 15:39:20 UTC
[GitHub] flink pull request #6037: [FLINK-6909] [types] Fix error message in CsvReade...
GitHub user twalthr opened a pull request:
https://github.com/apache/flink/pull/6037
[FLINK-6909] [types] Fix error message in CsvReader for wrong type class
## What is the purpose of the change
Fixes a minor issue that was reported by a user. See FLINK-6909.
## Brief change log
Check type before casting.
## Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/twalthr/flink FLINK-6909_3
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/6037.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #6037
----
commit eb99dc3402e1f4885c1a8a356af55c1669346d0d
Author: Timo Walther <tw...@...>
Date: 2018-05-17T15:35:55Z
[FLINK-6909] [types] Fix error message in CsvReader for wrong type class
----
---
[GitHub] flink pull request #6037: [FLINK-6909] [types] Fix error message in CsvReade...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/6037#discussion_r189803238
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/io/CsvReader.java ---
@@ -316,14 +317,18 @@ public CsvReader ignoreInvalidLines(){
Preconditions.checkNotNull(pojoType, "The POJO type class must not be null.");
Preconditions.checkNotNull(pojoFields, "POJO fields must be specified (not null) if output type is a POJO.");
- @SuppressWarnings("unchecked")
- PojoTypeInfo<T> typeInfo = (PojoTypeInfo<T>) TypeExtractor.createTypeInfo(pojoType);
+ final TypeInformation<T> ti = TypeExtractor.createTypeInfo(pojoType);
+ if (!(ti instanceof PojoTypeInfo)) {
+ throw new IllegalArgumentException(
+ "The specified class is not a POJO. The type class must meet the POJO requirements. Found: " + ti);
--- End diff --
Do we have any document explaining what the requirements are? Should we maybe go for a more extensive error message listing all requirements?
---
[GitHub] flink issue #6037: [FLINK-6909] [types] Fix error message in CsvReader for w...
Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on the issue:
https://github.com/apache/flink/pull/6037
Merging...
---
[GitHub] flink pull request #6037: [FLINK-6909] [types] Fix error message in CsvReade...
Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/6037#discussion_r189885082
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/io/CsvReader.java ---
@@ -316,14 +317,18 @@ public CsvReader ignoreInvalidLines(){
Preconditions.checkNotNull(pojoType, "The POJO type class must not be null.");
Preconditions.checkNotNull(pojoFields, "POJO fields must be specified (not null) if output type is a POJO.");
- @SuppressWarnings("unchecked")
- PojoTypeInfo<T> typeInfo = (PojoTypeInfo<T>) TypeExtractor.createTypeInfo(pojoType);
+ final TypeInformation<T> ti = TypeExtractor.createTypeInfo(pojoType);
+ if (!(ti instanceof PojoTypeInfo)) {
+ throw new IllegalArgumentException(
+ "The specified class is not a POJO. The type class must meet the POJO requirements. Found: " + ti);
--- End diff --
Yes, somewhere hidden in the documentation. I will fix this shortcoming as part of FLINK-8293 for 1.6.
---
[GitHub] flink pull request #6037: [FLINK-6909] [types] Fix error message in CsvReade...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/6037
---