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

Window function support #148

Merged
merged 3 commits into from
Aug 25, 2019
Merged

Window function support #148

merged 3 commits into from
Aug 25, 2019

Conversation

doug-martin
Copy link
Owner

@doug-martin doug-martin force-pushed the window_function_support branch 5 times, most recently from b3ef211 to 1818910 Compare August 25, 2019 05:16
@codecov
Copy link

codecov bot commented Aug 25, 2019

Codecov Report

Merging #148 into master will increase coverage by 1.96%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #148      +/-   ##
==========================================
+ Coverage   91.86%   93.82%   +1.96%     
==========================================
  Files          53       55       +2     
  Lines        3589     3823     +234     
==========================================
+ Hits         3297     3587     +290     
+ Misses        273      217      -56     
  Partials       19       19
Impacted Files Coverage Δ
exp/exp.go 68.49% <ø> (ø) ⬆️
exp/select_clauses.go 95.6% <100%> (+0.39%) ⬆️
exp/col.go 86.66% <100%> (+11.11%) ⬆️
exp/window.go 100% <100%> (ø)
sqlgen/sql_dialect_options.go 100% <100%> (+18.75%) ⬆️
sqlgen/select_sql_generator.go 100% <100%> (ø) ⬆️
dialect/sqlite3/sqlite3.go 100% <100%> (ø) ⬆️
exp/func.go 33.33% <100%> (+14.58%) ⬆️
exp/window_func.go 100% <100%> (ø)
select_dataset.go 100% <100%> (ø) ⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b8f9834...939c031. Read the comment docs.

@doug-martin doug-martin force-pushed the window_function_support branch from 1818910 to 54a6da8 Compare August 25, 2019 05:21
@doug-martin
Copy link
Owner Author

doug-martin commented Aug 25, 2019

@Xuyuanp thank you for the pull request and including all the tests and examples great job!

I had to make a few changes to make the expressions and sql generation work. The most notable ones were:

  • Renamed SelectDataset#Windows to SelectDataset#Window to keep inline with the other naming conventions such as Select and Where
  • Moving the Over and OverName from SQLWindowFunctionExpression onto SQLFunctionExpression...after doing some research I discovered that you can use the window functions on most function so it made sense to move that to the normal SQLFunctionExpression.
  • I made most of the Window expressions use IdentifierExpressions instead of strings.
  • Moved the window sql generation to the new sql package.
  • Added unit tests for sql generation on all the new expression types.

@doug-martin
Copy link
Owner Author

I also want to note that I ended up going with the snake case function names like you suggested, It does read better and keeps with the already established function naming conventions.

@doug-martin doug-martin force-pushed the window_function_support branch from 54a6da8 to aedf6bd Compare August 25, 2019 05:48
* [ADDED] Window Function support #128 - @Xuyuanp
@doug-martin doug-martin force-pushed the window_function_support branch from aedf6bd to 939c031 Compare August 25, 2019 05:52
@doug-martin doug-martin merged commit ce5475c into master Aug 25, 2019
@Xuyuanp
Copy link
Contributor

Xuyuanp commented Aug 27, 2019

@doug-martin goqu is the best 'sql builder' lib I have found and I have used it in my project.
It's my honor to make it better.

@doug-martin doug-martin deleted the window_function_support branch October 16, 2021 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants