[GitHub] [flink] carp84 commented on issue #9255: [FLINK-13034] Introduce isEmpty method for MapState

2019-09-25 Thread GitBox
carp84 commented on issue #9255: [FLINK-13034] Introduce isEmpty method for 
MapState
URL: https://github.com/apache/flink/pull/9255#issuecomment-535334308
 
 
   > What's more, since this adds a method in `MapState` interface which is 
`PublicEvolving`, we will need a release note.
   
   And don't forget about the release note.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [flink] carp84 commented on issue #9255: [FLINK-13034] Introduce isEmpty method for MapState

2019-09-24 Thread GitBox
carp84 commented on issue #9255: [FLINK-13034] Introduce isEmpty method for 
MapState
URL: https://github.com/apache/flink/pull/9255#issuecomment-534844656
 
 
   What's more, I think we should split the `StateBackendTestBase. 
testMapState` case to explicitly test each `MapState` operation interface, but 
this is out of the scope for this PR/JIRA and worth a dedicated one.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [flink] carp84 commented on issue #9255: [FLINK-13034] Introduce isEmpty method for MapState

2019-09-24 Thread GitBox
carp84 commented on issue #9255: [FLINK-13034] Introduce isEmpty method for 
MapState
URL: https://github.com/apache/flink/pull/9255#issuecomment-534844195
 
 
   > There have been some tests for `MapState.isEmpty` in 
`StateBackendTestBase.testMapState`, do you actually mean other tests for 
`MapState.isEmpty` could be ignored? @carp84
   
   I meant to add an explicit `testMapStateIsEmpty` case like 
`testListStateAddNull` in `StateBackendTestBase` instead of squeezing 
everything in `testMapState`. Similarly, add an explicit `testIsEmpty` method 
in `ImmutableMapStateTest` instead of scattered statements in existing cases.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [flink] carp84 commented on issue #9255: [FLINK-13034] Introduce isEmpty method for MapState

2019-09-10 Thread GitBox
carp84 commented on issue #9255: [FLINK-13034] Introduce isEmpty method for 
MapState
URL: https://github.com/apache/flink/pull/9255#issuecomment-530020615
 
 
   That's fine @Myasuka , just attach the patch here if anyone would like to 
try reproducing the result. Since this one claims to be a performance 
enhancement, I believe it's better to show some detailed data.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services