[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/359#issuecomment-73042551 Skipping the validation on raw types makes total 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/359#issuecomment-73013430
I do not quite get what this means now for the input validation.
Assume two classes, `A` and `B`, where `B` is a subclass of `A`.
```java
DataSetB data = ...;
data.map(new MapFunctionA, Long() {
...
});
```
I though that the validation previously checked that the MapFunction's
input parameter is assignable from the data set type. So this is skipped now?
It is not a big deal, I am just wondering why...
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/359#discussion_r24085603
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/typeutils/TypeExtractor.java
---
@@ -680,10 +680,20 @@ private static void validateInputType(Type t,
TypeInformation? inType) {
}
}
- private static void validateInputType(Class? baseClass, Class?
clazz, int inputParamPos, TypeInformation? inType) {
+ private static void validateInputType(Class? baseClass, Class?
clazz, int inputParamPos, TypeInformation? inTypeInfo) {
ArrayListType typeHierarchy = new ArrayListType();
+
+ // try to get generic parameter
+ Type inType;
+ try {
+ inType = getParameterType(baseClass, typeHierarchy,
clazz, inputParamPos);
+ }
+ catch (Exception e) {
+ return; // skip input validation e.g. for raw types
--- End diff --
I'll change it and merge it ;)
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/359#discussion_r24085111
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/typeutils/TypeExtractor.java
---
@@ -680,10 +680,20 @@ private static void validateInputType(Type t,
TypeInformation? inType) {
}
}
- private static void validateInputType(Class? baseClass, Class?
clazz, int inputParamPos, TypeInformation? inType) {
+ private static void validateInputType(Class? baseClass, Class?
clazz, int inputParamPos, TypeInformation? inTypeInfo) {
ArrayListType typeHierarchy = new ArrayListType();
+
+ // try to get generic parameter
+ Type inType;
+ try {
+ inType = getParameterType(baseClass, typeHierarchy,
clazz, inputParamPos);
+ }
+ catch (Exception e) {
+ return; // skip input validation e.g. for raw types
--- End diff --
Actually yes, the input validation is only a nice to have feature and
helps the normal user. If the getParameterType() does not work, the
validation should be skipped. We can also change it to InvalidTypeException
if you want.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/359#discussion_r24085204
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/typeutils/TypeExtractor.java
---
@@ -680,10 +680,20 @@ private static void validateInputType(Type t,
TypeInformation? inType) {
}
}
- private static void validateInputType(Class? baseClass, Class?
clazz, int inputParamPos, TypeInformation? inType) {
+ private static void validateInputType(Class? baseClass, Class?
clazz, int inputParamPos, TypeInformation? inTypeInfo) {
ArrayListType typeHierarchy = new ArrayListType();
+
+ // try to get generic parameter
+ Type inType;
+ try {
+ inType = getParameterType(baseClass, typeHierarchy,
clazz, inputParamPos);
+ }
+ catch (Exception e) {
+ return; // skip input validation e.g. for raw types
--- End diff --
I know its annoying to change pull requests for these minor changes.
If you want you can change it and push it to `master`.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
