[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-25 Thread pnowojski
Github user pnowojski commented on the issue: https://github.com/apache/flink/pull/6231 Thanks for verification and reporting issue! ---

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-24 Thread yanghua
Github user yanghua commented on the issue: https://github.com/apache/flink/pull/6231 PR #6392 fixed this issue. ---

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-23 Thread yanghua
Github user yanghua commented on the issue: https://github.com/apache/flink/pull/6231 @pnowojski thanks for giving a solution, I will try to verify it in our inner Flink version. ---

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-23 Thread pnowojski
Github user pnowojski commented on the issue: https://github.com/apache/flink/pull/6231 True, `checkNotNull(nestedSerializers);` is useless. Allowing nulls without enabled compile errors on violated `@Nullable` checks (this we can not enable in Flink) always leads to some null

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-23 Thread yanghua
Github user yanghua commented on the issue: https://github.com/apache/flink/pull/6231 @pnowojski I have said it is because of the constructor : ``` CompositeTypeSerializerConfigSnapshot(TypeSerializer... nestedSerializers) ``` used [`varargs ` in JIRA descri

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-23 Thread pnowojski
Github user pnowojski commented on the issue: https://github.com/apache/flink/pull/6231 Nope, you are not quite correct. This: ``` def this() = this(null)//scala ``` translates to ``` CompositeTypeSerializerConfigSnapshot(null); ``` But because of va

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-19 Thread yanghua
Github user yanghua commented on the issue: https://github.com/apache/flink/pull/6231 hi @pnowojski I did not call the `CompositeTypeSerializerConfigSnapshot(TypeSerializer... nestedSerializers)` constructor explicitly, the caller is Flink itself, see [here](https://github.com/apache

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-19 Thread pnowojski
Github user pnowojski commented on the issue: https://github.com/apache/flink/pull/6231 I agree with @zentol and I do not see reason for supporting nulls here. This fix looks like hiding underlying implementation problem. Default constructor of `CRowSerializerConfigSnapshot` could use

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-11 Thread yanghua
Github user yanghua commented on the issue: https://github.com/apache/flink/pull/6231 @zentol we really caused this exception, in our inner Flink version, we customized flink-table and implemented stream and dim table join. I think the default constructor is needed by deserialization.

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-11 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/6231 This doesn't look correct to me, the serializer should always be non-null, and silently dropping null serializers most likely just causes other problems. Did you actually run into this NPE whe

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-07-10 Thread yanghua
Github user yanghua commented on the issue: https://github.com/apache/flink/pull/6231 @pnowojski can you review this? ---

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-06-30 Thread yanghua
Github user yanghua commented on the issue: https://github.com/apache/flink/pull/6231 cc @twalthr and @fhueske ---

[GitHub] flink issue #6231: [FLINK-9694] Potentially NPE in CompositeTypeSerializerCo...

2018-06-30 Thread zhangminglei
Github user zhangminglei commented on the issue: https://github.com/apache/flink/pull/6231 Looks good from my side @yanghua :) ---