amaliujia commented on issue #1587: [CALCITE-3272] Support TUMBLE as Table Valued Function including an enumerable implementation, stream.iq and DESCRIPTOR URL: https://github.com/apache/calcite/pull/1587#issuecomment-568686794 @danny0405 thanks for your awesome work! I merged your change into this PR. I tried to squash this PR to two commits: one is my change and then one is yours so we can keep your credit in the git log. Unfortunately I messed that up and all the changes are merged into one single commit now. Please let me know if you want to have two commits to separate changes. I can try to rebuild and see if I can recover it. I was able to address one of the TODO. I didn't figure out this TODO: ``` if (isTumble) { // TODO: should move this code to RexImpTable and keep the code // in this class unchanged. return tableValuedFunctionWindowingImplement(implementor, pref); ``` I was guessing you want to use [1] to encapsulate some of the code into `RexImpTable`. As far as I can tell, `CallImplementor`'s interface is a little simple to encapsulate all the enumerable code of tumbling. I checked what is the existing practice. For example, I checked `EnumerableMergeJoin`, and `EnumerableMergeJoin` has the same practice as what is doing in `EnumerableTableFunctionScan`. [1]: https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/adapter/enumerable/CallImplementor.java#L38
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
