You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by huawei-flink <gi...@git.apache.org> on 2017/02/14 01:25:09 UTC
[GitHub] flink pull request #3302: [FLINK-5710] Add ProcTime() function to indicate S...
GitHub user huawei-flink opened a pull request:
https://github.com/apache/flink/pull/3302
[FLINK-5710] Add ProcTime() function to indicate StreamSQL
Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide](http://flink.apache.org/how-to-contribute.html).
In addition to going through the list, please provide a meaningful description of your changes.
- [ ] General
- The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
- The pull request addresses only one issue
- Each commit in the PR has a meaningful commit message (including the JIRA id)
- [ ] Documentation
- Documentation has been added for new functionality
- Old documentation affected by the pull request has been updated
- JavaDoc for public methods has been added
- [ ] Tests & Build
- Functionality added by the pull request is covered by tests
- `mvn clean verify` has been executed successfully locally or a Travis build has passed
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/huawei-flink/flink FLINK-5710
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3302.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3302
----
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by huawei-flink <gi...@git.apache.org>.
Github user huawei-flink commented on the issue:
https://github.com/apache/flink/pull/3302
I managed to merge the changes from PR #3370 into my branch after rebase, and the test works. I will push the code later today. sorry if I am a little latent, but I am travelling and connectivity is sometimes a problem.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3302
That would be great @huawei-flink! There are a few contributions waiting for this addition.
Thank you!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by huawei-flink <gi...@git.apache.org>.
Github user huawei-flink commented on the issue:
https://github.com/apache/flink/pull/3302
Hi Fabian,
I will follow up in the next days, probably early next week. is it ok?
Stefano
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3302
Hi @huawei-flink,
thanks for the update! The changes are now pretty much aligned with #3252. I'm in the process of merging #3252 (running last tests). It would be great if you could rebase your changes on top of the master once #3252 has been merged.
In order to test the feature, you can integrate `proctime()` into the `LogicalWindowAggregateRule` and extend the `WindowAggregateTest` for processing time tumbling windows.
Thanks, Fabian
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by huawei-flink <gi...@git.apache.org>.
Github user huawei-flink commented on the issue:
https://github.com/apache/flink/pull/3302
@fhueske no problem, I understand. It was bad timing, as I was on the run for more than two weeks, with little time to follow this. We'll contribute on other issues.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3302
Thanks for reopening the PR!
I made a few comments on #3271 before and after it was closed that should be addressed.
Thanks, Fabian
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by haohui <gi...@git.apache.org>.
Github user haohui commented on the issue:
https://github.com/apache/flink/pull/3302
FYI: #3370 is the commit we use internally for this feature. Please feel free to take it if it helps implementing this PR.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3302
FYI: PR #3252 was just merged.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3302: [FLINK-5710] Add ProcTime() function to indicate S...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/3302
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3302
Hi, it looks like something went wrong when updating the PR.
Can you rebase your changes on the latest master?
Thanks
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3302
Thanks @huawei-flink
Looking forward to your contributions!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by huawei-flink <gi...@git.apache.org>.
Github user huawei-flink commented on the issue:
https://github.com/apache/flink/pull/3302
@fhueske I've addressed most of the points, however there is a thing that is not clear to me yet. So far, the procTime() function generates a timestamp. My understanding is that this is not correct, and it should be something else. could it be a default timestamp (e.g. epoch)? the actual timestamp normalized to the second? what is the best option in your opinion?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3302
Hi @huawei-flink,
as I said before, there are a few contributions waiting for this patch.
I decided to move one and merge PR #3370.
Thanks a again for working on this and sorry for the inconvenience,
Fabian
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3302: [FLINK-5710] Add ProcTime() function to indicate StreamSQ...
Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3302
Hi @huawei-flink, do you plan to follow up on this PR?
Otherwise, I'd merge #3370 after adding a test.
Thanks, Fabian
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---