Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP:window function: Window with RANGE N PRECEDING/FOLLOWING frame requir… #11107

Closed
wants to merge 1 commit into from

Conversation

tcmichael
Copy link

@tcmichael tcmichael commented Jul 5, 2019

What problem does this PR solve?

window function: Window with RANGE N PRECEDING/FOLLOWING frame requires exactly one ORDER BY expression, of numeric or temporal type
Closes #11010

What is changed and how it works?

In the absence of a frame clause, handleDefaultFrame does not ignore the frame clause when spec.Frame.Type is RANGE

Check List

Tests

  • Unit test

…es exactly one ORDER BY expression, of numeric or temporal type
@codecov
Copy link

codecov bot commented Jul 5, 2019

Codecov Report

Merging #11107 into master will decrease coverage by 0.425%.
The diff coverage is 100%.

@@              Coverage Diff               @@
##            master    #11107        +/-   ##
==============================================
- Coverage   81.534%   81.109%   -0.4251%     
==============================================
  Files          420       420                
  Lines        91352     89466      -1886     
==============================================
- Hits         74483     72565      -1918     
- Misses       11576     11644        +68     
+ Partials      5293      5257        -36

@tcmichael
Copy link
Author

tcmichael commented Jul 5, 2019

I think I have fixed the bug. However, there is still a few questions in my mind: in what case, handleDefaultFrame should ignore the frame clause.

  1. As planner, executor: handle default frame for window functions #9544 mentioned:

When there is no frame specification and the function needs frame

 I think there is a frame when `spec.Frame != nil`
  1. The comment "For functions that operate on the entire partition, the frame clause will be ignored." also makes me confused. em....The judgement !needFrame && spec.Frame != nil is not necessary for the entire partition.

@SunRunAway
Copy link
Contributor

See the documentation defined window function frame of MYSQL, https://dev.mysql.com/doc/refman/8.0/en/window-functions-frames.html

In the absence of a frame clause, the default frame depends on whether an ORDER BY clause is present, as described later in this section.

Furthermore, some functions always use the entire partition.

Standard SQL specifies that window functions that operate on the entire partition should have no frame clause. MySQL permits a frame clause for such functions but ignores it. These functions use the entire partition even if a frame is specified

@tcmichael tcmichael changed the title window function: Window with RANGE N PRECEDING/FOLLOWING frame requir… WIP:window function: Window with RANGE N PRECEDING/FOLLOWING frame requir… Jul 5, 2019
@tcmichael tcmichael closed this Jul 5, 2019
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor.
Projects
None yet
3 participants