Github user fhueske commented on the issue:
https://github.com/apache/flink/pull/3459
Hi @huawei-flink, sorry for the long delay. I focused on adding support for
retraction to the new UDAGG interface (PR #3470), support for time mode
functions (PR #3370, #3425), and the most simple OVER window case (PR #3397)
first. These additions should serve as a good basis for the more advanced OVER
window cases.
I had a look this PR and noticed a few things:
- The Table API is implemented in Scala. We have a few Java classes (code
copied & fixed from Calcite, Java examples, and tests) but do not intend to add
more. The core parts like operators, translation, and runtime code have to be
implemented in Scala. Mixing languages makes the maintenance more difficult.
- Please squash your commits. Usually, PRs are opened with a single commit
and new commits are added when feedback is addressed. Before merging a
committer squashes these commits. However, it is too much effort to squash more
than 50 commits including merge commits which can cause trouble. If multiple
contributors worked on a PR, figure out how you can separate the work into one
commit per author.
- We have recently reworked our aggregation interface which will also serve
as the interface for user-defined aggregations which should also be usable in
OVER windows. Please use these aggregation functions. When designing the
interface we had their use in OVER windows in mind. If you find that the
interface is lacking a method, please start a discussion. However, we cannot
have several incompatible aggregation interfaces in the Table API / SQL. Please
rebase to the current master and use the new aggregation functions.
- A couple of days ago, we added `PROCTIME()` and `ROWTIME()` methods which
should be used to identify the time mode.
- The first OVER window aggregation should serve as a blueprint for future
OVER window implementations. We should try to keep the implementations as close
as possible to share common code and make the overall maintenance of the code
easier.
Minor comments:
- Why did you check modifications to the `.gitignore` files?
- Please do not remove tests without replacing them with equivalent tests.
- Please do not reformat classes (see FunctionCatalog). These changes cause
additional effort when reviewing a PR.
- Do not change ScalaDoc comments to JavaDoc comments in Scala code.
ScalaDoc:
```
/**
* Comment
*/
```
JavaDoc:
```
/**
* Comment
*/
```
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
