[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16399641#comment-16399641
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

Github user bowenli86 closed the pull request at:

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


> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16393314#comment-16393314
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

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


> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16392961#comment-16392961
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

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 all state is wrapped 
into a `UserFacingListState` in the `DefaultKeyedStateStore`.

So, is this a non-issue, actually? Something that may only affect test 
implementations of `ListState`



> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16360389#comment-16360389
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

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 ListState more similar to how MapState 
handles traversing - In MapState, it has `values()` and `iterator()` that both 
traverse entries in the map. 


> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16350371#comment-16350371
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

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 take care of `null` and in the other case 
not. It is always nicer to keep the core user interfaces slim. I would prefer 
to not include this change, and remember that this should be fixed if we come 
to the point of a API-breaking release.


> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16346406#comment-16346406
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

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 and confusing. As a user, instinctively, I'd expect an empty 
Iterable when there's no state value. In practice, I had to debug several times 
and ended up finding the true semantics of `get()`. This is where 
`iterator()`'s value lies - to help users avoid such cases.

Frankly, the `iterator()` API is not hard to maintain at all. It only 
invokes `get()`, handles the `null` situation, and returns users an empty 
iterable.


> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16343410#comment-16343410
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

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 is introduced by this PR. If we 
cannot change the old method, then I do not see a big benefit and this might 
not be worth having more methods in the API to maintain. What do you think 
@fhueske and @aljoscha ?


> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16343380#comment-16343380
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

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.


> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16343373#comment-16343373
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

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 that be more straight forward?


> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16340559#comment-16340559
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

Github user bowenli86 commented on the issue:

https://github.com/apache/flink/pull/5356
  
cc @fhueske  @aljoscha 


> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (FLINK-8364) Add iterator() to ListState which returns empty iterator when it has no value

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16338862#comment-16338862
 ] 

ASF GitHub Bot commented on FLINK-8364:
---

GitHub user bowenli86 opened a pull request:

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

[FLINK-8364][state backend] Add iterator() to ListState which returns empty 
iterator when it has no value

## What is the purpose of the change

Add iterator() to ListState which returns empty iterator when it has no 
value

## Brief change log

Add iterator() to ListState which returns empty iterator when it has no 
value

## Verifying this change

This change added tests and can be verified by extending 
`StateBackendTestBase#testListStateAPIs()`

## Does this pull request potentially affect one of the following parts:

  - The public API, i.e., is any changed class annotated with 
`@Public(Evolving)`: (yes)

## Documentation

  - Does this pull request introduce a new feature? (yes)
  - If yes, how is the feature documented? (docs / JavaDocs)


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

$ git pull https://github.com/bowenli86/flink FLINK-8364

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

https://github.com/apache/flink/pull/5356.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 #5356


commit fe3ae4e4798fb3f1a440dee737298718143edaf3
Author: Bowen Li 
Date:   2018-01-25T07:01:46Z

[FLINK-8364] Add iterator() to ListState which returns empty iterator when 
it has no value

commit c6a3e6bdb276b910e679bfa12a1abadaf2c9fcf8
Author: Bowen Li 
Date:   2018-01-25T07:41:46Z

add doc and unit test




> Add iterator() to ListState which returns empty iterator when it has no value
> -
>
> Key: FLINK-8364
> URL: https://issues.apache.org/jira/browse/FLINK-8364
> Project: Flink
>  Issue Type: Improvement
>  Components: State Backends, Checkpointing
>Affects Versions: 1.4.0
>Reporter: Bowen Li
>Assignee: Bowen Li
>Priority: Major
> Fix For: 1.5.0
>
>
> Add iterator() to ListState which returns empty iterator when it has no value



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)