[GitHub] flink issue #5032: [FLINK-8090] [DataStream] Improve the error message for d...

2017-11-27 Thread aljoscha
Github user aljoscha commented on the issue:

https://github.com/apache/flink/pull/5032
  
We could check based on the state descriptor.


---


[GitHub] flink issue #5032: [FLINK-8090] [DataStream] Improve the error message for d...

2017-11-27 Thread xccui
Github user xccui commented on the issue:

https://github.com/apache/flink/pull/5032
  
Thanks for the suggestion @aljoscha. 

The problem is the state type is provided via a generic type parameter `S 
extends State`, which will be erased in runtime. Thus it's hard to do type 
checking in `AbstractKeyedStateBackend` unless we explicitly store and check 
the type for each state name (and that may affect the performance). The 
existing "leave alone" solution seems to be the most efficient way, but we can 
only get a `ClassCastException` with that. What do you think?


---


[GitHub] flink issue #5032: [FLINK-8090] [DataStream] Improve the error message for d...

2017-11-27 Thread aljoscha
Github user aljoscha commented on the issue:

https://github.com/apache/flink/pull/5032
  
I think catching the `ClassCastException` is not the proper way to fix 
this. This might be masking other (real) bugs that lead to 
`ClassCastExceptions`. We should detect whether the wrong type is used for 
accessing a state that was already registered in the part of the code that does 
this, probably in `AbstractKeyedStateBackend`.


---


[GitHub] flink issue #5032: [FLINK-8090] [DataStream] Improve the error message for d...

2017-11-24 Thread xccui
Github user xccui commented on the issue:

https://github.com/apache/flink/pull/5032
  
Hi @aljoscha, I wonder if you could help review this PR when you are 
convenient.

Thanks, Xingcan


---


[GitHub] flink issue #5032: [FLINK-8090] [DataStream] Improve the error message for d...

2017-11-18 Thread bowenli86
Github user bowenli86 commented on the issue:

https://github.com/apache/flink/pull/5032
  
sounds good


---


[GitHub] flink issue #5032: [FLINK-8090] [DataStream] Improve the error message for d...

2017-11-18 Thread xccui
Github user xccui commented on the issue:

https://github.com/apache/flink/pull/5032
  
Hi @bowenli86, thanks for the review. This PR only changes the message of 
`RuntimeException`, thus may not be easily verified. To improve that, we'd add 
some dedicated exceptions for that. What do you think?


---


[GitHub] flink issue #5032: [FLINK-8090] [DataStream] Improve the error message for d...

2017-11-17 Thread bowenli86
Github user bowenli86 commented on the issue:

https://github.com/apache/flink/pull/5032
  
Can you please add a unit test for this?


---