Add window frame constructs - alternative #506
Conversation
0e2c3d0 to
947e18f
Compare
Codecov Report
@@ Coverage Diff @@
## master #506 +/- ##
==========================================
- Coverage 76.07% 76.06% -0.02%
==========================================
Files 155 156 +1
Lines 26544 26720 +176
==========================================
+ Hits 20194 20325 +131
- Misses 6350 6395 +45
Continue to review full report at Codecov.
|
| @@ -0,0 +1,337 @@ | |||
| // Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
This mod is coupled with sqlparser AST and doesn't seem to be physical specific. Maybe it's better to move this mod into logical plane instead and reimport in physical plane?
There was a problem hiding this comment.
I agree it would make more sense for this module to be in logical_plan (I think it would also be fine to do as a follow on PR)
There was a problem hiding this comment.
will do a follow up pull request when I start working on implementing window frames (after order by and partition by)
| @@ -0,0 +1,337 @@ | |||
| // Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
I agree it would make more sense for this module to be in logical_plan (I think it would also be fine to do as a follow on PR)
| impl TryFrom<ast::WindowFrame> for WindowFrame { | ||
| type Error = DataFusionError; | ||
|
|
||
| fn try_from(value: ast::WindowFrame) -> Result<Self> { |
There was a problem hiding this comment.
👍 I really like this structure
| /// There are five ways to describe starting and ending frame boundaries: | ||
| /// | ||
| /// 1. UNBOUNDED PRECEDING | ||
| /// 2. <expr> PRECEDING |
78d49b8 to
31ec1ee
Compare
…#506) ## Which issue does this PR close? Closes apache#485 ## Rationale for this change Compatibility with how Spark handles logarithms of values <=0. ## What changes are included in this PR? Use IfExpr to check when input to log2 is <=0 and return null. This is done to match Spark's behavior, which in turn is implemented to match Hive's behavior. ## How are these changes tested? The existing test for `ln`, `log2` and `log10` was modified so that it includes negative numbers as part of the inputs being tested.
Bumps [syn](https://github.com/dtolnay/syn) from 2.0.35 to 2.0.37. - [Release notes](https://github.com/dtolnay/syn/releases) - [Commits](dtolnay/syn@2.0.35...2.0.37) --- updated-dependencies: - dependency-name: syn dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Which issue does this PR close?
Related #361
Based on #463
Closes #492
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?