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 2015/02/03 22:55:47 UTC
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
GitHub user twalthr opened a pull request:
https://github.com/apache/flink/pull/359
[FLINK-1471][java-api] Fixes wrong input validation if function has no generics
FLINK-1471 was not implemented properly. See also #354.
This PR skips the input validation if no generic parameters are available.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/twalthr/flink UnParameterizedFunctions
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/359.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 #359
----
commit 7f70d7ef500cbb572a66f775d35609ac5903f767
Author: twalthr <tw...@apache.org>
Date: 2015-02-03T21:51:10Z
[FLINK-1471][java-api] Fixes wrong input validation if function has no generics
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/359
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Posted by twalthr <gi...@git.apache.org>.
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) {
ArrayList<Type> typeHierarchy = new ArrayList<Type>();
+
+ // 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/359#discussion_r24073064
--- 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) {
ArrayList<Type> typeHierarchy = new ArrayList<Type>();
+
+ // 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 --
are we sure that we can ignore all exceptions?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Posted by StephanEwen <gi...@git.apache.org>.
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
DataSet<B> data = ...;
data.map(new MapFunction<A, 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/359#issuecomment-72859235
Thank you @twalthr!
I've verified your change and its fixing the issue I was reporting originally.
Except for one minor question, the change is good to merge.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Posted by twalthr <gi...@git.apache.org>.
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) {
ArrayList<Type> typeHierarchy = new ArrayList<Type>();
+
+ // 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Posted by rmetzger <gi...@git.apache.org>.
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) {
ArrayList<Type> typeHierarchy = new ArrayList<Type>();
+
+ // 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/359#issuecomment-73028436
The discussion is how a Function without any generics (raw) should be treated.
In general we have 3 possiblities:
- Skip input validation completely if Function implements ResultTypeQueryable
- Introduce a Interface /Anntotation SkipInputValidation for special use cases
- This PR: Skip input validation if Function is a raw type.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1471][java-api] Fixes wrong input valid...
Posted by StephanEwen <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---