You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by shixiaogang <gi...@git.apache.org> on 2017/02/14 06:23:02 UTC

[GitHub] flink pull request #3305: [FLINK-5790][StateBackend] Use list types when Lis...

GitHub user shixiaogang opened a pull request:

    https://github.com/apache/flink/pull/3305

    [FLINK-5790][StateBackend] Use list types when ListStateDescriptor extends StateDescriptor

    1. Now the state serializer, instead of the element serializer, is stored in `ListStateDescriptor`. 
    2. `ArrayListTypeInfo` is introduced to help create serializers with the element type.
    3. `ArrayListSerializer` is moved to the package org.apache.flink.api.common.typeutils.base to avoid cyclic dependencies.
    4. Old implementation of `ListStateDescriptor` is kept in the migration package for back compatibility.
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/alibaba/flink flink-5790

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/3305.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 #3305
    
----
commit e8e11b7965365178453ab6eab78c6d5ac98f3537
Author: xiaogang.sxg <xi...@alibaba-inc.com>
Date:   2017-02-14T05:39:30Z

    Use list types when ListStateDescriptor extends StateDescriptor

commit ba8cdc919fc2e66b3e81f6f8566140bef53a9b96
Author: xiaogang.sxg <xi...@alibaba-inc.com>
Date:   2017-02-14T06:22:25Z

    Support back compatibility for ListStateDescriptor

----


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101070139
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/common/typeutils/base/ListSerializer.java ---
    @@ -0,0 +1,131 @@
    +/*
    + * 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.common.typeutils.base;
    +
    +import org.apache.flink.api.common.typeutils.TypeSerializer;
    +import org.apache.flink.core.memory.DataInputView;
    +import org.apache.flink.core.memory.DataOutputView;
    +
    +import java.io.IOException;
    +import java.util.List;
    +import java.util.ArrayList;
    +
    +@SuppressWarnings("ForLoopReplaceableByForEach")
    +public class ListSerializer<T> extends TypeSerializer<List<T>> {
    +
    +	private static final long serialVersionUID = 1119562170939152304L;
    +
    +	private final TypeSerializer<T> elementSerializer;
    +
    +	public ListSerializer(TypeSerializer<T> elementSerializer) {
    +		this.elementSerializer = elementSerializer;
    +	}
    +
    +	public TypeSerializer<T> getElementSerializer() {
    +		return elementSerializer;
    +	}
    +
    +	@Override
    +	public boolean isImmutableType() {
    +		return false;
    +	}
    +
    +	@Override
    +	public TypeSerializer<List<T>> duplicate() {
    +		TypeSerializer<T> duplicateElement = elementSerializer.duplicate();
    +		return duplicateElement == elementSerializer ? this : new ListSerializer<T>(duplicateElement);
    +	}
    +
    +	@Override
    +	public List<T> createInstance() {
    +		return new ArrayList<>();
    +	}
    +
    +	@Override
    +	public List<T> copy(List<T> from) {
    +		List<T> newList = new ArrayList<>(from.size());
    +		for (int i = 0; i < from.size(); i++) {
    +			newList.add(elementSerializer.copy(from.get(i)));
    +		}
    +		return newList;
    +	}
    +
    +	@Override
    +	public List<T> copy(List<T> from, List<T> reuse) {
    +		return copy(from);
    +	}
    +
    +	@Override
    +	public int getLength() {
    +		return -1; // var length
    +	}
    +
    +	@Override
    +	public void serialize(List<T> list, DataOutputView target) throws IOException {
    +		final int size = list.size();
    +		target.writeInt(size);
    +		for (int i = 0; i < size; i++) {
    +			elementSerializer.serialize(list.get(i), target);
    +		}
    +	}
    +
    +	@Override
    +	public List<T> deserialize(DataInputView source) throws IOException {
    +		final int size = source.readInt();
    +		final List<T> list = new ArrayList<>(size);
    +		for (int i = 0; i < size; i++) {
    +			list.add(elementSerializer.deserialize(source));
    +		}
    +		return list;
    +	}
    +
    +	@Override
    +	public List<T> deserialize(List<T> reuse, DataInputView source) throws IOException {
    +		return deserialize(source);
    +	}
    +
    +	@Override
    +	public void copy(DataInputView source, DataOutputView target) throws IOException {
    +		// copy number of elements
    +		final int num = source.readInt();
    +		target.writeInt(num);
    +		for (int i = 0; i < num; i++) {
    +			elementSerializer.copy(source, target);
    +		}
    +	}
    +
    +	// --------------------------------------------------------------------
    +
    +	@Override
    +	public boolean equals(Object obj) {
    +		return obj == this ||
    +				(obj != null && obj.getClass() == getClass() &&
    +						elementSerializer.equals(((ListSerializer<?>) obj).elementSerializer));
    +	}
    +
    +	@Override
    +	public boolean canEqual(Object obj) {
    +		return true;
    +	}
    --- End diff --
    
    This is not the contract of `canEqual`. Given that this class is not final this method should define which objects can be equal to this type. So in this case it would be `obj instanceOf ListSerializer`. Furthermore the check has to be incorporated into the `equals` method where you check `((ListSerializer) obj).canEqual(this))`. Otherwise the symmetry of equals is violated.


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101067289
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/common/state/ListStateDescriptor.java ---
    @@ -62,10 +68,19 @@ public ListStateDescriptor(String name, TypeInformation<T> typeInfo) {
     	 * @param name The (unique) name for the state.
     	 * @param typeSerializer The type serializer for the list values.
     	 */
    +	@SuppressWarnings("unchecked")
     	public ListStateDescriptor(String name, TypeSerializer<T> typeSerializer) {
    -		super(name, typeSerializer, null);
    +		super(name, new ListSerializer<>(typeSerializer), null);
    +	}
    +
    +	public TypeSerializer<T> getElementSerializer() {
    +		if (!(serializer instanceof ListSerializer)) {
    +			throw new IllegalStateException();
    --- End diff --
    
    We should add a meaningful error message.


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101068466
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/common/typeutils/base/ListSerializer.java ---
    @@ -0,0 +1,131 @@
    +/*
    + * 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.common.typeutils.base;
    +
    +import org.apache.flink.api.common.typeutils.TypeSerializer;
    +import org.apache.flink.core.memory.DataInputView;
    +import org.apache.flink.core.memory.DataOutputView;
    +
    +import java.io.IOException;
    +import java.util.List;
    +import java.util.ArrayList;
    +
    +@SuppressWarnings("ForLoopReplaceableByForEach")
    +public class ListSerializer<T> extends TypeSerializer<List<T>> {
    +
    +	private static final long serialVersionUID = 1119562170939152304L;
    +
    +	private final TypeSerializer<T> elementSerializer;
    +
    +	public ListSerializer(TypeSerializer<T> elementSerializer) {
    +		this.elementSerializer = elementSerializer;
    --- End diff --
    
    `Preconditions.checkNotNull` would be good.


---
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 issue #3305: [FLINK-5790][StateBackend] Use list types when ListStateD...

Posted by shixiaogang <gi...@git.apache.org>.
Github user shixiaogang commented on the issue:

    https://github.com/apache/flink/pull/3305
  
    @tillrohrmann Thanks for your review. 
    
    Sorry for the reformatted code. It seems that my IDE will automatically reformat all the files I've edited. I will revert the reformatted code.


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101068148
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/common/state/ListStateDescriptor.java ---
    @@ -40,20 +44,22 @@
     	 * consider using the {@link #ListStateDescriptor(String, TypeInformation)} constructor.
     	 *
     	 * @param name The (unique) name for the state.
    -	 * @param typeClass The type of the values in the state.
    +	 * @param elementTypeClass The type of the elements in the state.
     	 */
    -	public ListStateDescriptor(String name, Class<T> typeClass) {
    -		super(name, typeClass, null);
    +	@SuppressWarnings("unchecked")
    --- End diff --
    
    Why these annotations?


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by shixiaogang <gi...@git.apache.org>.
Github user shixiaogang commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r100967108
  
    --- Diff: flink-core/src/main/java/org/apache/flink/migration/util/MigrationInstantiationUtil.java ---
    @@ -47,9 +47,16 @@ public ClassLoaderObjectInputStream(InputStream in, ClassLoader classLoader) thr
     
     			// the flink package may be at position 0 (regular class) or position 2 (array)
     			final int flinkPackagePos;
    -			if ((flinkPackagePos = className.indexOf(FLINK_BASE_PACKAGE)) == 0 ||
    -					(flinkPackagePos == 2 && className.startsWith(ARRAY_PREFIX)))
    -			{
    +			if (className.contains("org.apache.flink.runtime.state.ArrayListSerializer")) {
    --- End diff --
    
    The code here is a little tricky. I think we should use a Map to record all modified classes and their corresponding backups.


---
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 issue #3305: [FLINK-5790][StateBackend] Use list types when ListStateD...

Posted by shixiaogang <gi...@git.apache.org>.
Github user shixiaogang commented on the issue:

    https://github.com/apache/flink/pull/3305
  
    @StephanEwen @tillrohrmann  I found a problem that is the packages `ListTypeInfo` and `ListTypeSerializer` locate. Now `ListTypeInfo` is put in package "org.apache.flink.api.java.typeutils" and `ListSerializer` is put in package "org.apache.flink.api.common.typeutils.base". But i think it's better to put "ListTypeInfo" into package "org.apache.flink.api.common.typeinfo". What do you think?


---
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 issue #3305: [FLINK-5790][StateBackend] Use list types when ListStateD...

Posted by shixiaogang <gi...@git.apache.org>.
Github user shixiaogang commented on the issue:

    https://github.com/apache/flink/pull/3305
  
    @tillrohrmann Both `org.apache.flink.api.common.typeutils.base` and `"org.apache.flink.api.common.typeinfo` are in the module `flink-core`. 


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101070404
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/java/typeutils/ListTypeInfo.java ---
    @@ -0,0 +1,116 @@
    +/*
    + * 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.annotation.Public;
    +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.common.typeutils.base.ListSerializer;
    +
    +import java.util.List;
    +
    +/**
    + * A {@link TypeInformation} for the list types of the Java API.
    + *
    + * @param <T> The type of the elements in the list.
    + */
    +
    +
    +@Public
    +public final class ListTypeInfo<T> extends TypeInformation<List<T>> {
    +
    +	private final TypeInformation<T> elementTypeInfo;
    +
    +	public ListTypeInfo(Class<T> elementTypeClass) {
    +		this.elementTypeInfo = TypeExtractor.createTypeInfo(elementTypeClass);
    +	}
    +
    +	public ListTypeInfo(TypeInformation<T> elementTypeInfo) {
    +		this.elementTypeInfo = elementTypeInfo;
    --- End diff --
    
    null check missing


---
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 issue #3305: [FLINK-5790][StateBackend] Use list types when ListStateD...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:

    https://github.com/apache/flink/pull/3305
  
    Given our current usage where the `TypeInformation` is no longer a sole API concept and we already keep most of the type informations in `flink-core`, I think it is a good idea to add `ListTypeInfo` to `flink-core`.


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101070284
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/java/typeutils/ListTypeInfo.java ---
    @@ -0,0 +1,116 @@
    +/*
    + * 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.annotation.Public;
    +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.common.typeutils.base.ListSerializer;
    +
    +import java.util.List;
    +
    +/**
    + * A {@link TypeInformation} for the list types of the Java API.
    + *
    + * @param <T> The type of the elements in the list.
    + */
    +
    +
    --- End diff --
    
    Two line breaks


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101009073
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/java/typeutils/ArrayListTypeInfo.java ---
    @@ -0,0 +1,116 @@
    +/*
    + * 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.annotation.Public;
    +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.common.typeutils.base.ArrayListSerializer;
    +
    +import java.util.ArrayList;
    +
    +/**
    + * A {@link TypeInformation} for the list types of the Java API.
    + *
    + * @param <T> The type of the elements in the list.
    + */
    +
    +
    +@Public
    +public final class ArrayListTypeInfo<T> extends TypeInformation<ArrayList<T>> {
    +
    +	private final TypeInformation<T> elementTypeInfo;
    +
    +	public ArrayListTypeInfo(Class<T> elementTypeClass) {
    +		this.elementTypeInfo = TypeExtractor.createTypeInfo(elementTypeClass);
    +	}
    +
    +	public ArrayListTypeInfo(TypeInformation<T> elementTypeInfo) {
    +		this.elementTypeInfo = elementTypeInfo;
    +	}
    +
    +	public TypeInformation<T> getElementTypeInfo() {
    +		return elementTypeInfo;
    +	}
    +
    +	@Override
    +	public boolean isBasicType() {
    +		return false;
    +	}
    +
    +	@Override
    +	public boolean isTupleType() {
    +		return false;
    +	}
    +
    +	@Override
    +	public int getArity() {
    +		return 0;
    +	}
    +
    +	@Override
    +	public int getTotalFields() {
    +		return elementTypeInfo.getTotalFields();
    +	}
    +
    +	@SuppressWarnings("unchecked")
    +	@Override
    +	public Class<ArrayList<T>> getTypeClass() {
    +		return (Class<ArrayList<T>>)(new ArrayList<T>().getClass());
    --- End diff --
    
    You can simplify this (to avoid creating an `ArrayList<T>` instance) via:
    `return (Class<ArrayList<T>>)(Class<?>) ArrayList.class;`


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101070598
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/java/typeutils/ListTypeInfo.java ---
    @@ -0,0 +1,116 @@
    +/*
    + * 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.annotation.Public;
    +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.common.typeutils.base.ListSerializer;
    +
    +import java.util.List;
    +
    +/**
    + * A {@link TypeInformation} for the list types of the Java API.
    + *
    + * @param <T> The type of the elements in the list.
    + */
    +
    +
    +@Public
    +public final class ListTypeInfo<T> extends TypeInformation<List<T>> {
    --- End diff --
    
    No serial version uid defined.


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101008561
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/common/typeutils/base/ArrayListSerializer.java ---
    @@ -108,9 +112,13 @@ public void copy(DataInputView source, DataOutputView target) throws IOException
     
     	@Override
     	public boolean equals(Object obj) {
    +		if (obj.getClass() != getClass())  {
    +			System.out.println("other " + obj.getClass().getName() + ", this " + getClass().getName());
    --- End diff --
    
    This is probably left over code that should be removed.


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101071253
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/state/ArrayListSerializer.java ---
    @@ -109,8 +113,8 @@ public void copy(DataInputView source, DataOutputView target) throws IOException
     	@Override
     	public boolean equals(Object obj) {
     		return obj == this ||
    -			(obj != null && obj.getClass() == getClass() &&
    -				elementSerializer.equals(((ArrayListSerializer<?>) obj).elementSerializer));
    +				(obj != null && obj.getClass() == getClass() &&
    +						elementSerializer.equals(((ArrayListSerializer<?>) obj).elementSerializer));
    --- End diff --
    
    reformatting changes are discouraged


---
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 issue #3305: [FLINK-5790][StateBackend] Use list types when ListStateD...

Posted by shixiaogang <gi...@git.apache.org>.
Github user shixiaogang commented on the issue:

    https://github.com/apache/flink/pull/3305
  
    @StephanEwen Thanks a lot for your comments. I have updated the code as suggested. 


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/3305


---
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 issue #3305: [FLINK-5790][StateBackend] Use list types when ListStateD...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/3305
  
    Thanks a lot for the quick turnaround!
    This looks good, merging now...


---
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 #3305: [FLINK-5790][StateBackend] Use list types when Lis...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3305#discussion_r101070858
  
    --- Diff: flink-core/src/main/java/org/apache/flink/api/java/typeutils/ListTypeInfo.java ---
    @@ -0,0 +1,116 @@
    +/*
    + * 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.annotation.Public;
    +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.common.typeutils.base.ListSerializer;
    +
    +import java.util.List;
    +
    +/**
    + * A {@link TypeInformation} for the list types of the Java API.
    + *
    + * @param <T> The type of the elements in the list.
    + */
    +
    +
    +@Public
    +public final class ListTypeInfo<T> extends TypeInformation<List<T>> {
    +
    +	private final TypeInformation<T> elementTypeInfo;
    +
    +	public ListTypeInfo(Class<T> elementTypeClass) {
    +		this.elementTypeInfo = TypeExtractor.createTypeInfo(elementTypeClass);
    +	}
    +
    +	public ListTypeInfo(TypeInformation<T> elementTypeInfo) {
    +		this.elementTypeInfo = elementTypeInfo;
    +	}
    +
    +	public TypeInformation<T> getElementTypeInfo() {
    +		return elementTypeInfo;
    +	}
    +
    +	@Override
    +	public boolean isBasicType() {
    +		return false;
    +	}
    +
    +	@Override
    +	public boolean isTupleType() {
    +		return false;
    +	}
    +
    +	@Override
    +	public int getArity() {
    +		return 0;
    +	}
    +
    +	@Override
    +	public int getTotalFields() {
    +		return elementTypeInfo.getTotalFields();
    +	}
    +
    +	@SuppressWarnings("unchecked")
    +	@Override
    +	public Class<List<T>> getTypeClass() {
    +		return (Class<List<T>>)(Class<?>)List.class;
    +	}
    +
    +	@Override
    +	public boolean isKeyType() {
    +		return false;
    +	}
    +
    +	@Override
    +	public TypeSerializer<List<T>> createSerializer(ExecutionConfig config) {
    +		TypeSerializer<T> elementTypeSerializer = elementTypeInfo.createSerializer(config);
    +		return new ListSerializer<>(elementTypeSerializer);
    +	}
    +
    +	@Override
    +	public String toString() {
    +		return null;
    --- End diff --
    
    I think here we should return a more meaningful value. Something like `List<elementType.toString>`.


---
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.
---