I guess, removing .fromElements(Object..) would fix the problem. Not sure so, if we can remove the method due to API stability...
I don't see any other good solution (even if the current implementation gives a nice behavior by accident...): If you have a complex class hierarchy, it would be quite complex to find out the correct common sub-type. Using only .fromElemenst(Class<X>, X...) requires to specify the correct sub-type and has the additional advantage, the the compiler can check the type already (instead of a potential later runtime error). -Matthias On 04/27/2016 03:07 PM, Till Rohrmann wrote: > You’re completely right Mathias. The compiler shouldn’t allow something > like env.fromElements(SubClass.class, new ParentClass()) if it weren’t for > the overloaded method. Thus, the test case is somewhat bogus. > > I’m actually wondering why the initial problem > https://issues.apache.org/jira/browse/FLINK-3444 was solved this way. I > think it would be better to automatically infer the common super type of > all provided elements. Otherwise, you run into problems you’ve found out > about. > > Consequently, I think it is fine if you remove the fromElementsWithBaseType2 > test case. > > Cheers, > Till > > > On Wed, Apr 27, 2016 at 1:22 PM, Matthias J. Sax <mj...@apache.org> wrote: > >> Hi Till, >> >> but StreamExecutionEnvironmentTest.fromElementWithBaseTypeTest2 does not >> test was you describe -- even if it is intended to test it. >> >> It would test your describe scenario, if fromElements(Class<X>, X...) >> would be called, But this call is not possible because X is defined a >> type Subclass and thus the provided object of Parentclass cannot be >> handed over as type X. Therefore, fromElements(Object...) is called: of >> course, this fails too, because now the type is derived as >> Class<Subclass> (and not Subclass) and neither Subclass nor Parentclass >> inherit from Class<Subclass>. >> >> The scenario you describe will never work -- if you remove the overload >> fromElements(Object...) the code would not even compile as the compiler >> can figure out from the generics that the call >> fromElments(Subclass.class, new Parentclass()) is invalid. >> >> It is only possible to hand in "reverse inheritance types" for >> fromElemenst(Object...). In this case, the first given Object defines >> the type. Thus, if you call fromElements(new Subclass(), new >> Parentclass()), the call will fail, as Parentclass is no subtype of >> Subtype -- the call fromElements(new Parentclass() new Subclass()) would >> succeed. >> >> Makes sense? >> >> Still no idea how to make it compile in Eclipse... >> >> -Matthias >> >> On 04/27/2016 10:21 AM, Till Rohrmann wrote: >>> Thanks for looking into this problem Mathias. I think the Scala test >> should >>> be fixed as you've proposed. >>> >>> Concerning the >> StreamExecutionEnvironmentTest.fromElementWithBaseTypeTest2, >>> I think it shouldn't be changed. The reason is that the class defines the >>> common base class of the elements. And the test makes sure that the >>> fromElements call fails if you provide instances which are not of the >>> specified type or a subclass of it. Thus, we should find another way to >>> make it work with Eclipse. >>> >>> Cheers, >>> Till >>> >>> On Tue, Apr 26, 2016 at 9:41 PM, Matthias J. Sax <mj...@apache.org> >> wrote: >>> >>>> Even if the fix works, I still have two issues in my Eclipse build... >>>> >>>> In >>>> >>>> >>>> >> flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/base/AcceptPFTestBase.scala >>>> >>>> Eclipse cannot infer the integer type. It could be fixed if you make the >>>> type explicit (as this is only a test, it might be nice to fix this -- >>>> let me know if I can push this or not) >>>> >>>>> diff --git >>>> >> a/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/base/AcceptPFTestBase.scala >>>> >> b/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/base/AcceptPFTestBase.scala >>>>> index c2e13fe..f9ce3b8 100644 >>>>> --- >>>> >> a/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/base/AcceptPFTestBase.scala >>>>> +++ >>>> >> b/flink-scala/src/test/scala/org/apache/flink/api/scala/extensions/base/AcceptPFTestBase.scala >>>>> @@ -29,7 +29,7 @@ private[extensions] abstract class AcceptPFTestBase >>>> extends TestLogger with JUni >>>>> >>>>> private val env = ExecutionEnvironment.getExecutionEnvironment >>>>> >>>>> - protected val tuples = env.fromElements(1 -> "hello", 2 -> "world") >>>>> + protected val tuples = env.fromElements(new Integer(1) -> "hello", >>>> new Integer(2) -> "world") >>>>> protected val caseObjects = env.fromElements(KeyValuePair(1, >>>> "hello"), KeyValuePair(2, "world")) >>>>> >>>>> protected val groupedTuples = tuples.groupBy(_._1) >>>> >>>> Furthermore, in >>>> >>>> >> flink-java/src/test/java/org/apache/flink/api/java/io/FromElementsTest.java >>>> >>>>> @Test >>>>> public void fromElementsWithBaseTypeTest1() { >>>>> ExecutionEnvironment executionEnvironment = >>>> ExecutionEnvironment.getExecutionEnvironment(); >>>>> executionEnvironment.fromElements(ParentType.class, new >> SubType(1, >>>> "Java"), new ParentType(1, "hello")); >>>>> } >>>> >>>> and in >>>> >>>> >>>> >> flink-streaming-java/src/test/java/org/apache/flink/streaming/api/StreamExecutionEnvironmentTest.java >>>> >>>>> @Test >>>>> public void fromElementsWithBaseTypeTest1() { >>>>> StreamExecutionEnvironment env = >>>> StreamExecutionEnvironment.getExecutionEnvironment(); >>>>> env.fromElements(ParentClass.class, new SubClass(1, "Java"), new >>>> ParentClass(1, "hello")); >>>>> } >>>> >>>> In both cases, I get the error: >>>> >>>> The method .fromElements(Object[]) is ambiguous >>>> >>>> No clue how to fix this, and why Eclipse does not bind to >>>> .fromElements(Class<X>, X). Any ideas? >>>> >>>> I also digger a little bit and for both test-classes there is a second >>>> test method called "fromElementsWithBaseTypeTest2". If I understand this >>>> test correctly, it also tries to bind to .fromElements(Class<X>, X), but >>>> this does not happen and .fromElemenst(Object[]) is called. Even if >>>> there is still an exception, I got the impression that this test does >>>> not what the intention was. >>>> >>>> If might be good to change fromElementsWithBaseTypeTest2 to >>>> >>>>> env.fromElements(new SubClass(1, "Java"), new ParentClass(1, "hello")); >>>> >>>> (ie, remove the first Class parameter). Any comments on this? >>>> >>>> -Matthias >>>> >>>> >>>> >>>> On 04/25/2016 01:42 PM, Robert Metzger wrote: >>>>> Cool, thank you for working on this! >>>>> >>>>> On Mon, Apr 25, 2016 at 1:37 PM, Matthias J. Sax <mj...@apache.org> >>>> wrote: >>>>> >>>>>> I can confirm that the SO answer works. >>>>>> >>>>>> I will add a note to the Eclipse setup guide at the web site. >>>>>> >>>>>> -Matthias >>>>>> >>>>>> >>>>>> On 04/25/2016 11:33 AM, Robert Metzger wrote: >>>>>>> It seems that the user resolved the issue on SO, right? >>>>>>> >>>>>>> On Mon, Apr 25, 2016 at 11:31 AM, Ufuk Celebi <u...@apache.org> >> wrote: >>>>>>> >>>>>>>> On Mon, Apr 25, 2016 at 12:14 AM, Matthias J. Sax <mj...@apache.org >>> >>>>>>>> wrote: >>>>>>>>> What do you think about this? >>>>>>>> >>>>>>>> Hey Matthias! >>>>>>>> >>>>>>>> Thanks for bringing this up. >>>>>>>> >>>>>>>> I think it is very desirable to keep support for Eclipse. It's >> quite a >>>>>>>> high barrier for new contributors to enforce a specific IDE >> (although >>>>>>>> IntelliJ is gaining quite the user base I think :P). >>>>>>>> >>>>>>>> Do you have time to look into this? >>>>>>>> >>>>>>>> – Ufuk >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> >>> >> >> >
signature.asc
Description: OpenPGP digital signature