[GitHub] flink issue #2490: [FLINK-4609] Remove redundant check for null in CrossOper...

2016-09-20 Thread greghogan
Github user greghogan commented on the issue:

https://github.com/apache/flink/pull/2490
  
Merging ...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #2490: [FLINK-4609] Remove redundant check for null in CrossOper...

2016-09-20 Thread StephanEwen
Github user StephanEwen commented on the issue:

https://github.com/apache/flink/pull/2490
  
All right, +1 then


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #2490: [FLINK-4609] Remove redundant check for null in CrossOper...

2016-09-19 Thread greghogan
Github user greghogan commented on the issue:

https://github.com/apache/flink/pull/2490
  
This isn't improving performance but moving the null checks before first 
access and removing the duplicate `DataSet` references.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #2490: [FLINK-4609] Remove redundant check for null in CrossOper...

2016-09-19 Thread StephanEwen
Github user StephanEwen commented on the issue:

https://github.com/apache/flink/pull/2490
  
Playing Devil's advocate here:

Does this really improve anything meaningful? This is an API-level 
operator, not anything runtime related, so the additional null check is not 
really important. The code is not getting simpler in my opinion as well. It 
worked well before, and every change may introduce another bug.

Why not focus energies on improvements where the system really gains?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #2490: [FLINK-4609] Remove redundant check for null in CrossOper...

2016-09-19 Thread fhueske
Github user fhueske commented on the issue:

https://github.com/apache/flink/pull/2490
  
Thanks for the fast update @apivovarov. 
PR is good to merge


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #2490: [FLINK-4609] Remove redundant check for null in CrossOper...

2016-09-19 Thread fhueske
Github user fhueske commented on the issue:

https://github.com/apache/flink/pull/2490
  
Thanks for the contribution @apivovarov. The PR looks good except for the 
modified test which fails.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #2490: [FLINK-4609] Remove redundant check for null in CrossOper...

2016-09-10 Thread apivovarov
Github user apivovarov commented on the issue:

https://github.com/apache/flink/pull/2490
  
Ok, I added null check for input1 and input2 with a message before calling 
super in DefaultCross



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---