[GitHub] flink issue #5356: [FLINK-8364][state backend] Add iterator() to ListState w...

2018-03-09 Thread bowenli86
Github user bowenli86 commented on the issue: https://github.com/apache/flink/pull/5356 hmmm I think you are right, this actually might be a non-issue in the first place ---

[GitHub] flink issue #5356: [FLINK-8364][state backend] Add iterator() to ListState w...

2018-03-09 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/5356 I am wondering whether this discussion is a bit confused. All state facing the user in the APIs already has the behavior that there is no `null`, but only empty iterators. That's because

[GitHub] flink issue #5356: [FLINK-8364][state backend] Add iterator() to ListState w...

2018-02-11 Thread bowenli86
Github user bowenli86 commented on the issue: https://github.com/apache/flink/pull/5356 @StefanRRichter I think how `null` is handled is one major benefit, not all. Another benefit is that `iterator()` is more intuitive in traversing all values. For example, this change makes

[GitHub] flink issue #5356: [FLINK-8364][state backend] Add iterator() to ListState w...

2018-02-02 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5356 I still do not believe that this is a significant improvement, and may even increase confusion: Now you have two slightly different ways to do almost the same thing, in one case you need to

[GitHub] flink issue #5356: [FLINK-8364][state backend] Add iterator() to ListState w...

2018-01-31 Thread bowenli86
Github user bowenli86 commented on the issue: https://github.com/apache/flink/pull/5356 I also agree that changing `get()` API may not be good because it will break user's logic. From a user point of view, I found that receiving `null` from `get()` API is a bit inelegant

[GitHub] flink issue #5356: [FLINK-8364][state backend] Add iterator() to ListState w...

2018-01-29 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5356 Ok, that is not so nice IMHO for an API, but if you think this might break some code then we have to keep it. Nevertheless, I still doubt about any additional value from the new method that

[GitHub] flink issue #5356: [FLINK-8364][state backend] Add iterator() to ListState w...

2018-01-29 Thread fhueske
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/5356 @StefanRRichter that would be an (IMO unnecessary) API breaking change. See the comments on FLINK-8364. ---

[GitHub] flink issue #5356: [FLINK-8364][state backend] Add iterator() to ListState w...

2018-01-29 Thread StefanRRichter
Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/5356 I wonder why we need to introduce a separate method for an iterator instead of just changing the `get()` method to return `Collections.emptyList()` as iterable in case of null value. Wouldn't

[GitHub] flink issue #5356: [FLINK-8364][state backend] Add iterator() to ListState w...

2018-01-25 Thread bowenli86
Github user bowenli86 commented on the issue: https://github.com/apache/flink/pull/5356 cc @fhueske @aljoscha ---