[jira] [Commented] (FLINK-13004) Correct the logic of needToCleanupState in KeyedProcessFunctionWithCleanupState
[ https://issues.apache.org/jira/browse/FLINK-13004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873994#comment-16873994 ] Jark Wu commented on FLINK-13004: - [~yunta] (y) > Correct the logic of needToCleanupState in > KeyedProcessFunctionWithCleanupState > --- > > Key: FLINK-13004 > URL: https://issues.apache.org/jira/browse/FLINK-13004 > Project: Flink > Issue Type: Bug > Components: Table SQL / Planner >Reporter: Yun Tang >Assignee: Yun Tang >Priority: Major > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > Current implementation of needToCleanupState in > KeyedProcessFunctionWithCleanupState actually has potention bug: > {code:java} > protected Boolean needToCleanupState(Long timestamp) throws IOException { >if (stateCleaningEnabled) { > Long cleanupTime = cleanupTimeState.value(); > // check that the triggered timer is the last registered processing > time timer. > return null != cleanupTime && timestamp == cleanupTime; >} else { > return false; >} > } > {code} > Please note that it directly use "==" to judge whether *Long* type timestamp > and cleanupTime equals. However, if that value is larger than 127L, the > result would actually return false instead of wanted true. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FLINK-13004) Correct the logic of needToCleanupState in KeyedProcessFunctionWithCleanupState
[ https://issues.apache.org/jira/browse/FLINK-13004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873815#comment-16873815 ] Yun Tang commented on FLINK-13004: -- [~jark], actually, the {{timestamp}} passed in was wrapper {{Long.}} For the comparison of wrapper and primitive value problem, you could refer to [question here|https://stackoverflow.com/questions/19485818/what-are-not-2-long-variables-equal-with-operator-to-compare-in-java]. I am preparing a PR to fix this and found previous tests have been modified to cater for this bug. > Correct the logic of needToCleanupState in > KeyedProcessFunctionWithCleanupState > --- > > Key: FLINK-13004 > URL: https://issues.apache.org/jira/browse/FLINK-13004 > Project: Flink > Issue Type: Bug > Components: Table SQL / Planner >Reporter: Yun Tang >Assignee: Yun Tang >Priority: Major > > Current implementation of needToCleanupState in > KeyedProcessFunctionWithCleanupState actually has potention bug: > {code:java} > protected Boolean needToCleanupState(Long timestamp) throws IOException { >if (stateCleaningEnabled) { > Long cleanupTime = cleanupTimeState.value(); > // check that the triggered timer is the last registered processing > time timer. > return null != cleanupTime && timestamp == cleanupTime; >} else { > return false; >} > } > {code} > Please note that it directly use "==" to judge whether *Long* type timestamp > and cleanupTime equals. However, if that value is larger than 127L, the > result would actually return false instead of wanted true. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FLINK-13004) Correct the logic of needToCleanupState in KeyedProcessFunctionWithCleanupState
[ https://issues.apache.org/jira/browse/FLINK-13004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16873756#comment-16873756 ] Jark Wu commented on FLINK-13004: - I think the {{timestamp}} parameter should be primitive long. Thanks for reporting this. Could your elaborate more about why larger than 127L, the result would return false? > Correct the logic of needToCleanupState in > KeyedProcessFunctionWithCleanupState > --- > > Key: FLINK-13004 > URL: https://issues.apache.org/jira/browse/FLINK-13004 > Project: Flink > Issue Type: Bug > Components: Table SQL / Planner >Reporter: Yun Tang >Assignee: Yun Tang >Priority: Major > > Current implementation of needToCleanupState in > KeyedProcessFunctionWithCleanupState actually has potention bug: > {code:java} > protected Boolean needToCleanupState(Long timestamp) throws IOException { >if (stateCleaningEnabled) { > Long cleanupTime = cleanupTimeState.value(); > // check that the triggered timer is the last registered processing > time timer. > return null != cleanupTime && timestamp == cleanupTime; >} else { > return false; >} > } > {code} > Please note that it directly use "==" to judge whether *Long* type timestamp > and cleanupTime equals. However, if that value is larger than 127L, the > result would actually return false instead of wanted true. -- This message was sent by Atlassian JIRA (v7.6.3#76005)