Jimexist edited a comment on pull request #334:
URL: https://github.com/apache/arrow-datafusion/pull/334#issuecomment-845184684
> I haven't had the time to go through yet;
>
> Since this is a large decomposable change, a suggestion here is to create
a branch and merge this on that branch, so that we do not have incomplete
features in master and allow PRs to that branch without an issue. Then merge
the branch into master once its first iteration is ready (e.g. logical and 1
physical plan).
>
> (This is something that I wished to have when implementing large features;
it gives the time to review in parts without the risk of having incomplete code
in master)
>
> With that said, I am also fine risk it and merg to master; @Jimexist do
you have any preference how would you prefer to work on this here? :)
I wish to:
1. add unit tests
2. fix some small comments like
1. adding filter clause to leave room for future room without API
disruption,
2. adding nth_value and tile
3. merge it into master as it
3 because although I agree with having a feature branch, the maintenance of
continuous rebasing is trouble some. if I can keep the merged code isolated and
structurally correct then there's little worry about breaking changes in
future: I believe this PR:
1. is _isolated_ because the _explain_ works, while actually executing the
window SQL returns zero rows, due to unimplemented exec plan, which I intend to
do next
2. is structurally correct if reviewers can focus on the phase and timing of
the window clause in the query planner. future changes of e.g. adding one or
more sort phases requires only modifications within the `.window` `fn` and thus
can be non-intrusive
--
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:
us...@infra.apache.org
