You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by s1ck <gi...@git.apache.org> on 2015/12/18 22:08:08 UTC
[GitHub] flink pull request: [FLINK-3118] [Gelly] Consider ResultTypeQuerya...
GitHub user s1ck opened a pull request:
https://github.com/apache/flink/pull/1471
[FLINK-3118] [Gelly] Consider ResultTypeQueryable in Message and GatherFunction
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/s1ck/flink FLINK-3118
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1471.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 #1471
----
commit 67fb836cf68d7385c38d708b05d7d8826b53a34e
Author: Martin Junghanns <m....@mailbox.org>
Date: 2015-12-18T14:01:24Z
[FLINK-3118] [Gelly] Consider ResultTypeQueryable in MessageFunction and GatherFunction
----
---
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-3118] [Gelly] Consider ResultTypeQuerya...
Posted by s1ck <gi...@git.apache.org>.
Github user s1ck commented on the pull request:
https://github.com/apache/flink/pull/1471#issuecomment-169421134
Hi, sorry for the delay. I'll add the test asap.
---
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-3118] [Gelly] Consider ResultTypeQuerya...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/1471#issuecomment-166338426
Changes look good to me. Maybe we could add a test case which verifies the behaviour. Apart from that, +1 for merging :-)
---
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-3118] [Gelly] Consider ResultTypeQuerya...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1471#discussion_r48158308
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/TypeExtractor.java ---
@@ -340,6 +340,15 @@ protected TypeExtractor() {
}
return ti;
}
+
+ @SuppressWarnings("unchecked")
+ public static <OUT> TypeInformation<OUT> createTypeInfo(Class<?> baseClass, Class<?> clazz, int returnParamPos, Object instance) {
--- End diff --
I would maybe move the `instance` parameter to the front of the parameter list. Then it's similar to `getUnaryOperatorReturnType`.
---
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-3118] [Gelly] Consider ResultTypeQuerya...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1471
---
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-3118] [Gelly] Consider ResultTypeQuerya...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1471#discussion_r48158234
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/TypeExtractor.java ---
@@ -340,6 +340,15 @@ protected TypeExtractor() {
}
return ti;
}
+
+ @SuppressWarnings("unchecked")
+ public static <OUT> TypeInformation<OUT> createTypeInfo(Class<?> baseClass, Class<?> clazz, int returnParamPos, Object instance) {
--- End diff --
Missing java docs
---
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-3118] [Gelly] Consider ResultTypeQuerya...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1471#discussion_r48155988
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/TypeExtractor.java ---
@@ -340,6 +340,15 @@ protected TypeExtractor() {
}
return ti;
}
+
+ @SuppressWarnings("unchecked")
+ public static <OUT> TypeInformation<OUT> createTypeInfo(Class<?> baseClass, Class<?> clazz, int returnParamPos, Object instance) {
+ if (instance != null && instance instanceof ResultTypeQueryable) {
--- End diff --
`x instanceof T` is automatically false if `x == null`.
---
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-3118] [Gelly] Consider ResultTypeQuerya...
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/1471#issuecomment-171201543
Merging this!
---
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-3118] [Gelly] Consider ResultTypeQuerya...
Posted by s1ck <gi...@git.apache.org>.
Github user s1ck commented on the pull request:
https://github.com/apache/flink/pull/1471#issuecomment-166341381
Sorry for missing javadoc, I was inspired by the other methods in `TypeExtractor` ;) The test will be a generic version of `LabelPropagation`, described in https://issues.apache.org/jira/browse/FLINK-3122 Would that be enough for a test?
---
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-3118] [Gelly] Consider ResultTypeQuerya...
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/1471#issuecomment-168386683
Thanks a lot for the PR @s1ck!
How about adding a test case in `TypeExtractorTest`, where you create an instance that implements `ResultTypeQueryable` and one that doesn't and you check that the new method derives the type information correctly? This should be really easy to add :)
---
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.
---