You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by vasia <gi...@git.apache.org> on 2015/11/17 20:41:01 UTC
[GitHub] flink pull request: [FLINK-3002] Add Either type to the Java API
GitHub user vasia opened a pull request:
https://github.com/apache/flink/pull/1371
[FLINK-3002] Add Either type to the Java API
Implemented as discussed in the corresponding JIRA.
I haven't added this type to the docs yet. The data types section in the programming guide contains the following categories:
- Java Tuples and Scala Case Classes
- Java POJOs
- Primitive Types
- Regular Classes
- Values
- Hadoop Writables
I don't think `Either` belongs to any of these. Shall I add it as its own category?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vasia/flink either-java-type
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1371.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 #1371
----
----
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-158027623
Looks good, +1 to merge 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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-157784970
Integrating it with the TypeExtractor means that the APIs recognize the type and choose the type info properly.
I think @twalthr is probably the best to give pointers on how to integrate this with the TypeExtractor.
BTW: Would be nice if we could make custom type integration easier by defining an interface/static method that classes can implement to create their own type information. That gives users an easy extension point.
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45249202
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/Either.java ---
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+/**
+ * This type represents a value of one two possible types, Left or Right
+ * (a disjoint union), inspired by Scala's Either type.
+ *
+ * @param <L> the type of Left
+ * @param <R> the type of Right
+ */
+public abstract class Either<L, R> {
+
+ /**
+ * Create a Left value of Either
+ */
+ public static <L, R> Either<L, R> left(L value) {
+ return new Left<L, R>(value);
+ }
+
+ /**
+ * Create a Right value of Either
+ */
+ public static <L, R> Either<L, R> right(R value) {
+ return new Right<L, R>(value);
+ }
+
+ /**
+ * Retrieve the Left value of Either.
+ * @return the Left value
+ * @throws IllegalStateException if called on a Right
+ */
+ public abstract L left() throws IllegalStateException;
+
+ /**
+ * Retrieve the Right value of Either.
+ * @return the Right value
+ * @throws IllegalStateException if called on a Left
+ */
+ public abstract R right() throws IllegalStateException;
+
+ /**
+ *
+ * @return true if this is a Left value, false if this is a Right value
+ */
+ public final boolean isLeft() {
+ return getClass() == Left.class;
+ }
+
+ /**
+ *
+ * @return true if this is a Right value, false if this is a Left value
+ */
+ public final boolean isRight() {
+ return getClass() == Right.class;
+ }
+
+ private static class Left<L, R> extends Either<L, R> {
+ final L value;
--- End diff --
Field can be private.
---
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-3002] Add Either type to the Java API
Posted by gyfora <gi...@git.apache.org>.
Github user gyfora commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-157711434
Otherwise big :+1: for this, it is very useful, also we should add the java Optional :)
---
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-3002] Add Either type to the Java API
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-158051108
Hey @twalthr, thanks!
I've created FLINK-3046 for the Either integration and I think FLINK-3042 is about custom type interfaces.
---
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-3002] Add Either type to the Java API
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45250671
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/runtime/EitherSerializer.java ---
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils.runtime;
+
+import java.io.IOException;
+
+import org.apache.flink.api.common.typeutils.TypeSerializer;
+import org.apache.flink.api.java.typeutils.Either;
+import org.apache.flink.core.memory.DataInputView;
+import org.apache.flink.core.memory.DataOutputView;
+
+/**
+ * A {@link TypeSerializer} for the {@ link Either} type of the Java class.
+ *
+ * @param <L> the Left value type
+ * @param <R> the Right value type
+ */
+public class EitherSerializer<L, R> extends TypeSerializer<Either<L, R>> {
+
+ private static final long serialVersionUID = 1L;
+
+ private final Class<Either<L, R>> typeClass;
--- End diff --
Right, thanks!
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45248610
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/Either.java ---
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+/**
+ * This type represents a value of one two possible types, Left or Right
+ * (a disjoint union), inspired by Scala's Either type.
+ *
+ * @param <L> the type of Left
+ * @param <R> the type of Right
+ */
+public abstract class Either<L, R> {
+
+ /**
+ * Create a Left value of Either
+ */
+ public static <L, R> Either<L, R> left(L value) {
--- End diff --
I think it is better to return the specific type here `public static <L, R> Left<L, R> left(L value)`. More specific types allow people to assign them to more specific variables.
---
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-3002] Add Either type to the Java API
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-157767153
Thanks a lot for the review. I've updated the PR.
@gyfora @StephanEwen what do you mean by adding this to the type extractor? What do I need to do?
Also, any comment on how to document this (see the PR description)?
Thanks!
---
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-3002] Add Either type to the Java API
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1371
---
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 #1371: [FLINK-3002] Add Either type to the Java API
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r100804216
--- Diff: flink-java/src/test/java/org/apache/flink/api/java/typeutils/EitherTypeInfoTest.java ---
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+import org.apache.flink.api.common.typeinfo.BasicTypeInfo;
+import org.apache.flink.api.java.tuple.Tuple2;
+import org.apache.flink.util.TestLogger;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class EitherTypeInfoTest extends TestLogger {
+
+ Either<Integer, String> intEither = Either.left(1);
+ Either<Integer, String> stringEither = Either.right("boo");
+ Either<Integer, Tuple2<Double, Long>> tuple2Either = Either.right(new Tuple2<Double, Long>(42.0, 2l));
+
+ @Test
+ public void testEitherTypeEquality() {
+ EitherTypeInfo<Integer, String> eitherInfo1 = new EitherTypeInfo<Integer, String>(
+ BasicTypeInfo.INT_TYPE_INFO, BasicTypeInfo.STRING_TYPE_INFO);
+
+ EitherTypeInfo<Integer, String> eitherInfo2 = new EitherTypeInfo<Integer, String>(
+ BasicTypeInfo.INT_TYPE_INFO, BasicTypeInfo.STRING_TYPE_INFO);
+
+ assertEquals(eitherInfo1, eitherInfo2);
+ assertEquals(eitherInfo1.hashCode(), eitherInfo2.hashCode());
+ }
+
+ @Test
+ public void testEitherTypeInEquality() {
+ EitherTypeInfo<Integer, String> eitherInfo1 = new EitherTypeInfo<Integer, String>(
+ BasicTypeInfo.INT_TYPE_INFO, BasicTypeInfo.STRING_TYPE_INFO);
+
+ EitherTypeInfo<Integer, Tuple2<Double, Long>> eitherInfo2 = new EitherTypeInfo<Integer, Tuple2<Double, Long>>(
+ BasicTypeInfo.INT_TYPE_INFO, new TupleTypeInfo<Tuple2<Double, Long>>(
+ TypeExtractor.getForClass(Double.class), TypeExtractor.getForClass(String.class)));
+
+ assertNotEquals(eitherInfo1, eitherInfo2);
+ assertNotEquals(eitherInfo1.hashCode(), eitherInfo2.hashCode());
--- End diff --
You're right @helfper. I will fix the issue. Thanks a lot for reporting 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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45198787
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/Either.java ---
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+/**
+ * This type represents a value of one two possible types, Left or Right
+ * (a disjoint union), inspired by Scala's Either type.
+ *
+ * @param <L> the type of Left
+ * @param <R> the type of Right
+ */
+public abstract class Either<L, R> {
+
+ /**
+ * Create a Left value of Either
+ */
+ public static <L, R> Either<L, R> left(L value) {
+ return new Left<L, R>(value);
+ }
+
+ /**
+ * Create a Right value of Either
+ */
+ public static <L, R> Either<L, R> right(R value) {
+ return new Right<L, R>(value);
+ }
+
+ /**
+ * Retrieve the Left value of Either.
+ * @return the Left value
+ * @throws IllegalStateException if called on a Right
+ */
+ public abstract L left() throws IllegalStateException;
+
+ /**
+ * Retrieve the Right value of Either.
+ * @return the Right value
+ * @throws IllegalStateException if called on a Left
+ */
+ public abstract R right() throws IllegalStateException;
+
+ /**
+ *
+ * @return true if this is a Left value, false if this is a Right value
+ */
+ public abstract boolean isLeft();
--- End diff --
You could even make this method final and implement it as `return getClass() == Left.class". May get you a little bit of performance...
---
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-3002] Add Either type to the Java API
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45250469
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/Either.java ---
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+/**
+ * This type represents a value of one two possible types, Left or Right
+ * (a disjoint union), inspired by Scala's Either type.
+ *
+ * @param <L> the type of Left
+ * @param <R> the type of Right
+ */
+public abstract class Either<L, R> {
+
+ /**
+ * Create a Left value of Either
+ */
+ public static <L, R> Either<L, R> left(L value) {
--- End diff --
hmm I was thinking that `Left` and `Right` make no sense on their own, that's why I have declared the classes private, i.e. I don't see anyone writing `Left<L, R> left = Either.left(...)`. And it would be awkward to have to define 2 types, no?
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45198671
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/Either.java ---
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+/**
+ * This type represents a value of one two possible types, Left or Right
+ * (a disjoint union), inspired by Scala's Either type.
+ *
+ * @param <L> the type of Left
+ * @param <R> the type of Right
+ */
+public abstract class Either<L, R> {
+
+ /**
+ * Create a Left value of Either
+ */
+ public static <L, R> Either<L, R> left(L value) {
+ return new Left<L, R>(value);
+ }
+
+ /**
+ * Create a Right value of Either
+ */
+ public static <L, R> Either<L, R> right(R value) {
+ return new Right<L, R>(value);
+ }
+
+ /**
+ * Retrieve the Left value of Either.
+ * @return the Left value
+ * @throws IllegalStateException if called on a Right
+ */
+ public abstract L left() throws IllegalStateException;
+
+ /**
+ * Retrieve the Right value of Either.
+ * @return the Right value
+ * @throws IllegalStateException if called on a Left
+ */
+ public abstract R right() throws IllegalStateException;
+
+ /**
+ *
+ * @return true if this is a Left value, false if this is a Right value
+ */
+ public abstract boolean isLeft();
--- End diff --
How about adding a `isRight() = !isLeft()" for completeness (make this a final method).
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45199245
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/runtime/EitherSerializer.java ---
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils.runtime;
+
+import java.io.IOException;
+
+import org.apache.flink.api.common.typeutils.TypeSerializer;
+import org.apache.flink.api.java.typeutils.Either;
+import org.apache.flink.core.memory.DataInputView;
+import org.apache.flink.core.memory.DataOutputView;
+
+/**
+ * A {@link TypeSerializer} for the {@ link Either} type of the Java class.
+ *
+ * @param <L> the Left value type
+ * @param <R> the Right value type
+ */
+public class EitherSerializer<L, R> extends TypeSerializer<Either<L, R>> {
+
+ private static final long serialVersionUID = 1L;
+
+ private final Class<Either<L, R>> typeClass;
+
+ private final TypeSerializer<L> leftSerializer;
+
+ private final TypeSerializer<R> rightSerializer;
+
+ public EitherSerializer(Class<Either<L, R>> typeClass, TypeSerializer<L> leftSerializer, TypeSerializer<R> rightSerializer) {
+ this.typeClass = typeClass;
+ this.leftSerializer = leftSerializer;
+ this.rightSerializer = rightSerializer;
+ }
+
+ @Override
+ public boolean isImmutableType() {
+ return true;
--- End diff --
I think this needs to be `leftSerializer.isImmutableType() & rightSerializer.isImmutableType()`.
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45198942
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/Either.java ---
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+/**
+ * This type represents a value of one two possible types, Left or Right
+ * (a disjoint union), inspired by Scala's Either type.
+ *
+ * @param <L> the type of Left
+ * @param <R> the type of Right
+ */
+public abstract class Either<L, R> {
+
+ /**
+ * Create a Left value of Either
+ */
+ public static <L, R> Either<L, R> left(L value) {
+ return new Left<L, R>(value);
+ }
+
+ /**
+ * Create a Right value of Either
+ */
+ public static <L, R> Either<L, R> right(R value) {
+ return new Right<L, R>(value);
+ }
+
+ /**
+ * Retrieve the Left value of Either.
+ * @return the Left value
+ * @throws IllegalStateException if called on a Right
+ */
+ public abstract L left() throws IllegalStateException;
+
+ /**
+ * Retrieve the Right value of Either.
+ * @return the Right value
+ * @throws IllegalStateException if called on a Left
+ */
+ public abstract R right() throws IllegalStateException;
+
+ /**
+ *
+ * @return true if this is a Left value, false if this is a Right value
+ */
+ public abstract boolean isLeft();
+
+ private static class Left<L, R> extends Either<L, R> {
+ final L value;
+
+ public Left(L value) {
+ this.value = value;
+ }
+
+ @Override
+ public L left() {
+ return value;
+ }
+
+ @Override
+ public R right() {
+ throw new IllegalStateException("Cannot retrieve Right value on a Left");
+ }
+
+ @Override
+ public boolean equals(Object object) {
+ if (object instanceof Left<?, ?>) {
+ final Left<?, ?> other = (Left<?, ?>) object;
+ return value.equals(other.value);
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return value.hashCode();
+ }
+
+ @Override
+ public String toString() {
+ return "Either.left(" + value.toString() + ")";
--- End diff --
I think the Scala Either just prints "Left()", which is a little nicer, IMHO, but totally up to you...
---
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-3002] Add Either type to the Java API
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-158042526
Thanks. I'll squash the commits and merge later today.
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45251503
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/Either.java ---
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+/**
+ * This type represents a value of one two possible types, Left or Right
+ * (a disjoint union), inspired by Scala's Either type.
+ *
+ * @param <L> the type of Left
+ * @param <R> the type of Right
+ */
+public abstract class Either<L, R> {
+
+ /**
+ * Create a Left value of Either
+ */
+ public static <L, R> Either<L, R> left(L value) {
--- End diff --
But yeah, the Left and Right classes would need to be public then. Feel free to ignore this, up to you ;-)
---
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-3002] Add Either type to the Java API
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45180384
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/runtime/EitherSerializer.java ---
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils.runtime;
+
+import java.io.IOException;
+
+import org.apache.flink.api.common.typeutils.TypeSerializer;
+import org.apache.flink.api.java.typeutils.Either;
+import org.apache.flink.core.memory.DataInputView;
+import org.apache.flink.core.memory.DataOutputView;
+
+/**
+ * A {@link TypeSerializer} for the {@ link Either} type of the Java class.
+ *
+ * @param <L> the Left value type
+ * @param <R> the Right value type
+ */
+public class EitherSerializer<L, R> extends TypeSerializer<Either<L, R>> {
+
+ private static final long serialVersionUID = 1L;
+
+ private final Class<Either<L, R>> typeClass;
+
+ private final TypeSerializer<L> leftSerializer;
+
+ private final TypeSerializer<R> rightSerializer;
+
+ public EitherSerializer(Class<Either<L, R>> typeClass, TypeSerializer<L> leftSerializer, TypeSerializer<R> rightSerializer) {
+ this.typeClass = typeClass;
+ this.leftSerializer = leftSerializer;
+ this.rightSerializer = rightSerializer;
+ }
+
+ @Override
+ public boolean isImmutableType() {
+ return true;
+ }
+
+ @Override
+ public TypeSerializer<Either<L, R>> duplicate() {
+ TypeSerializer<L> duplicateLeft = leftSerializer.duplicate();
+ TypeSerializer<R> duplicateRight = rightSerializer.duplicate();
+
+ if ((leftSerializer != duplicateLeft) || (rightSerializer != duplicateRight)) {
+ // stateful
+ return new EitherSerializer<L, R>(typeClass, duplicateLeft, duplicateRight);
+ }
+ else {
+ return this;
+ }
+ }
+
+
+ @Override
+ public Either<L, R> createInstance() {
+ // We arbitrarily always create a Left value instance.
+ return Either.left(leftSerializer.createInstance());
--- End diff --
Assuming that the failure case occurs seldom.
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45248963
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/runtime/EitherSerializer.java ---
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils.runtime;
+
+import java.io.IOException;
+
+import org.apache.flink.api.common.typeutils.TypeSerializer;
+import org.apache.flink.api.java.typeutils.Either;
+import org.apache.flink.core.memory.DataInputView;
+import org.apache.flink.core.memory.DataOutputView;
+
+/**
+ * A {@link TypeSerializer} for the {@ link Either} type of the Java class.
+ *
+ * @param <L> the Left value type
+ * @param <R> the Right value type
+ */
+public class EitherSerializer<L, R> extends TypeSerializer<Either<L, R>> {
+
+ private static final long serialVersionUID = 1L;
+
+ private final Class<Either<L, R>> typeClass;
--- End diff --
You can probably drop this class as well, I don't see it needed anywhere.
---
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-3002] Add Either type to the Java API
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45180256
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/runtime/EitherSerializer.java ---
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils.runtime;
+
+import java.io.IOException;
+
+import org.apache.flink.api.common.typeutils.TypeSerializer;
+import org.apache.flink.api.java.typeutils.Either;
+import org.apache.flink.core.memory.DataInputView;
+import org.apache.flink.core.memory.DataOutputView;
+
+/**
+ * A {@link TypeSerializer} for the {@ link Either} type of the Java class.
+ *
+ * @param <L> the Left value type
+ * @param <R> the Right value type
+ */
+public class EitherSerializer<L, R> extends TypeSerializer<Either<L, R>> {
+
+ private static final long serialVersionUID = 1L;
+
+ private final Class<Either<L, R>> typeClass;
+
+ private final TypeSerializer<L> leftSerializer;
+
+ private final TypeSerializer<R> rightSerializer;
+
+ public EitherSerializer(Class<Either<L, R>> typeClass, TypeSerializer<L> leftSerializer, TypeSerializer<R> rightSerializer) {
+ this.typeClass = typeClass;
+ this.leftSerializer = leftSerializer;
+ this.rightSerializer = rightSerializer;
+ }
+
+ @Override
+ public boolean isImmutableType() {
+ return true;
+ }
+
+ @Override
+ public TypeSerializer<Either<L, R>> duplicate() {
+ TypeSerializer<L> duplicateLeft = leftSerializer.duplicate();
+ TypeSerializer<R> duplicateRight = rightSerializer.duplicate();
+
+ if ((leftSerializer != duplicateLeft) || (rightSerializer != duplicateRight)) {
+ // stateful
+ return new EitherSerializer<L, R>(typeClass, duplicateLeft, duplicateRight);
+ }
+ else {
+ return this;
+ }
+ }
+
+
+ @Override
+ public Either<L, R> createInstance() {
+ // We arbitrarily always create a Left value instance.
+ return Either.left(leftSerializer.createInstance());
--- End diff --
I would rather create a `Either.Right` instance per default. The reason is that `Either.Left` is used for failures (by convention) and `Either.Right` is akin to `Some`.
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-157838535
Some small comments, otherwise +1 to merge.
Integration into the TypeExctractor can be a separate issue, in my opinion.
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-157716591
Looks pretty good, with a few comments inline!
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-157839083
Concerning the docs, we have often talked about "special types", which would also include Scala's Option, Either, and Try. Would be great to add that section.
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45251332
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/Either.java ---
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+/**
+ * This type represents a value of one two possible types, Left or Right
+ * (a disjoint union), inspired by Scala's Either type.
+ *
+ * @param <L> the type of Left
+ * @param <R> the type of Right
+ */
+public abstract class Either<L, R> {
+
+ /**
+ * Create a Left value of Either
+ */
+ public static <L, R> Either<L, R> left(L value) {
--- End diff --
Depends, on whether you expect people to declare directly Left and Right variables. I was simply thinking, why forbid this, maybe someone finds a good case to do that...
---
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-3002] Add Either type to the Java API
Posted by gyfora <gi...@git.apache.org>.
Github user gyfora commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-157711193
I think we should add this to the TypeExtractor, otherwise it becomes very clumsy.
I am using the same logic in my applications and defining the TypeInfo in returns gets awkward after a while and is also harder for the users who are not very familiar with the system (how to use the TypeExtractor.getForObject etc).
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-157713609
+1 for including it into the type extractor
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45198871
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/Either.java ---
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+/**
+ * This type represents a value of one two possible types, Left or Right
+ * (a disjoint union), inspired by Scala's Either type.
+ *
+ * @param <L> the type of Left
+ * @param <R> the type of Right
+ */
+public abstract class Either<L, R> {
+
+ /**
+ * Create a Left value of Either
+ */
+ public static <L, R> Either<L, R> left(L value) {
+ return new Left<L, R>(value);
+ }
+
+ /**
+ * Create a Right value of Either
+ */
+ public static <L, R> Either<L, R> right(R value) {
+ return new Right<L, R>(value);
+ }
+
+ /**
+ * Retrieve the Left value of Either.
+ * @return the Left value
+ * @throws IllegalStateException if called on a Right
+ */
+ public abstract L left() throws IllegalStateException;
+
+ /**
+ * Retrieve the Right value of Either.
+ * @return the Right value
+ * @throws IllegalStateException if called on a Left
+ */
+ public abstract R right() throws IllegalStateException;
+
+ /**
+ *
+ * @return true if this is a Left value, false if this is a Right value
+ */
+ public abstract boolean isLeft();
+
+ private static class Left<L, R> extends Either<L, R> {
+ final L value;
+
+ public Left(L value) {
+ this.value = value;
--- End diff --
Since you rely on non-null values, may make sense to check this via `this.value = java.util.Objects.requireNonNull(value);`.
Same for the Right...
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45199091
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/EitherTypeInfo.java ---
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+import org.apache.flink.api.common.ExecutionConfig;
+import org.apache.flink.api.common.typeinfo.TypeInformation;
+import org.apache.flink.api.common.typeutils.TypeSerializer;
+import org.apache.flink.api.java.typeutils.runtime.EitherSerializer;
+
+/**
+ * A {@link TypeInformation} for the {@link Either} type of the Java API.
+ *
+ * @param <L> the Left value type
+ * @param <R> the Right value type
+ */
+public class EitherTypeInfo<L, R> extends TypeInformation<Either<L, R>> {
+
+ private static final long serialVersionUID = 1L;
+
+ private final Class<Either<L, R>> typeClass;
+
+ private final TypeInformation<L> leftType;
+
+ private final TypeInformation<R> rightType;
+
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ public EitherTypeInfo(Class<Either> typeClass,
--- End diff --
You can probably drop the parameter "typeClass" and simply adjust the "getTypeClass()" method below (see below).
---
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 #1371: [FLINK-3002] Add Either type to the Java API
Posted by helfper <gi...@git.apache.org>.
Github user helfper commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r100800288
--- Diff: flink-java/src/test/java/org/apache/flink/api/java/typeutils/EitherTypeInfoTest.java ---
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+import org.apache.flink.api.common.typeinfo.BasicTypeInfo;
+import org.apache.flink.api.java.tuple.Tuple2;
+import org.apache.flink.util.TestLogger;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class EitherTypeInfoTest extends TestLogger {
+
+ Either<Integer, String> intEither = Either.left(1);
+ Either<Integer, String> stringEither = Either.right("boo");
+ Either<Integer, Tuple2<Double, Long>> tuple2Either = Either.right(new Tuple2<Double, Long>(42.0, 2l));
+
+ @Test
+ public void testEitherTypeEquality() {
+ EitherTypeInfo<Integer, String> eitherInfo1 = new EitherTypeInfo<Integer, String>(
+ BasicTypeInfo.INT_TYPE_INFO, BasicTypeInfo.STRING_TYPE_INFO);
+
+ EitherTypeInfo<Integer, String> eitherInfo2 = new EitherTypeInfo<Integer, String>(
+ BasicTypeInfo.INT_TYPE_INFO, BasicTypeInfo.STRING_TYPE_INFO);
+
+ assertEquals(eitherInfo1, eitherInfo2);
+ assertEquals(eitherInfo1.hashCode(), eitherInfo2.hashCode());
+ }
+
+ @Test
+ public void testEitherTypeInEquality() {
+ EitherTypeInfo<Integer, String> eitherInfo1 = new EitherTypeInfo<Integer, String>(
+ BasicTypeInfo.INT_TYPE_INFO, BasicTypeInfo.STRING_TYPE_INFO);
+
+ EitherTypeInfo<Integer, Tuple2<Double, Long>> eitherInfo2 = new EitherTypeInfo<Integer, Tuple2<Double, Long>>(
+ BasicTypeInfo.INT_TYPE_INFO, new TupleTypeInfo<Tuple2<Double, Long>>(
+ TypeExtractor.getForClass(Double.class), TypeExtractor.getForClass(String.class)));
+
+ assertNotEquals(eitherInfo1, eitherInfo2);
+ assertNotEquals(eitherInfo1.hashCode(), eitherInfo2.hashCode());
--- End diff --
This assert seems conceptually wrong to me (even if it works in this specific case), as the contract for `hashCode` is `t1.equals(t2) => t1.hashCode() == t2.hashCode()`, so given `!t1.equals(t2)`, we can't actually conclude anything about their `hashCode`'s. I think it should be removed from the `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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45199167
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/EitherTypeInfo.java ---
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+import org.apache.flink.api.common.ExecutionConfig;
+import org.apache.flink.api.common.typeinfo.TypeInformation;
+import org.apache.flink.api.common.typeutils.TypeSerializer;
+import org.apache.flink.api.java.typeutils.runtime.EitherSerializer;
+
+/**
+ * A {@link TypeInformation} for the {@link Either} type of the Java API.
+ *
+ * @param <L> the Left value type
+ * @param <R> the Right value type
+ */
+public class EitherTypeInfo<L, R> extends TypeInformation<Either<L, R>> {
+
+ private static final long serialVersionUID = 1L;
+
+ private final Class<Either<L, R>> typeClass;
+
+ private final TypeInformation<L> leftType;
+
+ private final TypeInformation<R> rightType;
+
+ @SuppressWarnings({ "unchecked", "rawtypes" })
+ public EitherTypeInfo(Class<Either> typeClass,
+ TypeInformation<L> leftType,TypeInformation<R> rightType) {
+ this.typeClass = (Class<Either<L, R>>) (Class<?>) typeClass;
+ this.leftType = leftType;
+ this.rightType = rightType;
+ }
+
+ @Override
+ public boolean isBasicType() {
+ return false;
+ }
+
+ @Override
+ public boolean isTupleType() {
+ return false;
+ }
+
+ @Override
+ public int getArity() {
+ return 1;
+ }
+
+ @Override
+ public int getTotalFields() {
+ return 1;
+ }
+
+ @Override
+ public Class<Either<L, R>> getTypeClass() {
+ return this.typeClass;
--- End diff --
I would change this to `return (Class<Either<L, R>>) (Class<?>) Either.class`. You can then get rid of the field `this.typeClass`.
---
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-3002] Add Either type to the Java API
Posted by vasia <gi...@git.apache.org>.
Github user vasia commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-158003742
Thanks! Updated. I'll open a JIRA for integrating it into the TypeExtractor.
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/1371#discussion_r45248629
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/typeutils/Either.java ---
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.typeutils;
+
+/**
+ * This type represents a value of one two possible types, Left or Right
+ * (a disjoint union), inspired by Scala's Either type.
+ *
+ * @param <L> the type of Left
+ * @param <R> the type of Right
+ */
+public abstract class Either<L, R> {
+
+ /**
+ * Create a Left value of Either
+ */
+ public static <L, R> Either<L, R> left(L value) {
+ return new Left<L, R>(value);
+ }
+
+ /**
+ * Create a Right value of Either
+ */
+ public static <L, R> Either<L, R> right(R value) {
--- End diff --
Same as above.
---
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-3002] Add Either type to the Java API
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-157713501
The Java Optional comes only in Java 8, so we would need to implement this in a reflective way. But I would like that a lot as well (replacement for nullable fields).
---
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-3002] Add Either type to the Java API
Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/1371#issuecomment-158048821
@vasia I can implement the TypeExtractor support.
@StephanEwen Custom type integration interfaces sound very good. I can open an issue for that.
---
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.
---