[GitHub] flink issue #4625: [FLINK-6233] [table] Support time-bounded stream inner jo...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/4625 Btw, I noticed I did not reply to your comments. I think it would be good to have the eager state cleaning in the initial version. Shouldn't be too much effort. Basically, getting the condition right and calling `remove()` on the `Map.Entry`. What do you mean by "distinguish the < and <=signs"? If there is an off-by-one issue in the computation of the window boundaries, it needs to be fixed with this PR. We shouldn't merge a semantically incorrect operator (of course there might be bugs...). Performance issues are OK but the semantics must be correct. Regarding the `"misc"` test failures, yes that can happen. No need to worry about that as long as the `""` libraries build passes. I'll run the tests anyway again before merging ;-) ---
[GitHub] flink issue #4625: [FLINK-6233] [table] Support time-bounded stream inner jo...
Github user xccui commented on the issue: https://github.com/apache/flink/pull/4625 Hi @fhueske, the PR has been updated. Temporarily, I keep the logic for dealing with the late data, as well as the fine-grained cache. For the late data semantics problem, I think we need to rethink it and make a final decision (maybe we should consult others). For the cache optimization problem, I want to leave it a future work. ---
[GitHub] flink issue #4625: [FLINK-6233] [table] Support time-bounded stream inner jo...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/4625 Thanks for the update @xccui. I'll have a look in the next days. ---
[GitHub] flink issue #4625: [FLINK-6233] [table] Support time-bounded stream inner jo...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/4625 Hi @xccui, I think aside from the equi join predicate limitation, all issues have been addressed. This can be fixed in a follow up task, IMO. I'll have a last look at the PR and hopefully merge it. Thanks for the great work, Fabian Follow ups will be (some might already have JIRAs): - enabling streaming joins for the Table API - blocking of state entries and clean-up improvements - support for windowed non-equi joins - support for windowed outer joins ---
[GitHub] flink issue #4625: [FLINK-6233] [table] Support time-bounded stream inner jo...
Github user xccui commented on the issue: https://github.com/apache/flink/pull/4625 Hi @fhueske, I really appreciate for your guidance with great care. Hopefully this prolonged work do not affect the schedule. I'll keep working on the follow-up issues. ---
[GitHub] flink issue #4625: [FLINK-6233] [table] Support time-bounded stream inner jo...
Github user fhueske commented on the issue: https://github.com/apache/flink/pull/4625 Merging ---
[GitHub] flink issue #4625: [FLINK-6233] [table] Support time-bounded stream inner jo...
Github user xccui commented on the issue: https://github.com/apache/flink/pull/4625 Thanks for the review, @fhueske. This PR is a little rough when I committed. I'll address your comments and submit a refined version as soon as possible. Best, Xingcan ---
[GitHub] flink issue #4625: [FLINK-6233] [table] Support time-bounded stream inner jo...
Github user xccui commented on the issue: https://github.com/apache/flink/pull/4625 Hi @fhueske, the PR has been updated. However, there are still some unfinished tasks, e.g., optimise the data caching and cleaning up policies and distinguish the `<` and `<=`signs. I want to leave them as future works. What do you think? BTW, I find most of the recent PRs are failed on the build jobs with `TEST = "misc"`. I'm not sure if there exist some problems with the CI. Here is a [build log](https://s3.amazonaws.com/archive.travis-ci.org/jobs/274508987/log.txt?X-Amz-Expires=30&X-Amz-Date=20170912T130101Z&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAJRYRXRSVGNKPKO5A/20170912/us-east-1/s3/aws4_request&X-Amz-SignedHeaders=host&X-Amz-Signature=af6d9a141ef245be9a7393e69c484785340a9bb1f1030e1039d4ed44f0344a42) for this PR. ---
