[GitHub] flink issue #2490: [FLINK-4609] Remove redundant check for null in CrossOper...
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...
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...
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...
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...
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...
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...
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. ---