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

Support for defining WITH clauses for Common Table Expressions (CTE) #39

Merged
merged 5 commits into from Jun 12, 2017

Conversation

Oscil8
Copy link
Contributor

@Oscil8 Oscil8 commented May 8, 2017

Thanks for this great library; I've found it very useful and filled my needs for query generation. Just found one missing element that did exist in some of the other major query generators which is support for Common Table Expressions (CTEs), i.e. WITH clauses. This includes both WITH and WITH RECURSIVE versions and applies them to SELECT, UPDATE, DELETE and INSERT queries (although I am just using them in the SELECT context).

These are a useful way to build efficient queries by re-using commonly used subparts. They are supported in PostgreSQL (https://www.postgresql.org/docs/current/static/queries-with.html) and SQLite (https://sqlite.org/lang_with.html); and I noticed in the upcoming MySQL 8.0 currently in Beta (https://dev.mysql.com/doc/refman/8.0/en/with.html). I'm using PostgreSQL.

Would appreciate your feedback on the way I have added the WITH clauses to goqu; as well as appropriate level of test and documentation. I aimed for a similar level as for the Union and related clauses.

Ariel Salomon added 3 commits May 7, 2017 18:41
 - conditional support in the default adapter
 - tests at dataset level and also examples
Note that the beta MySQL 8.0 supports CTE; as does MariaDB 10.x
@Oscil8 Oscil8 force-pushed the add-with-cte branch 3 times, most recently from 8a84a4e to cfc8e7c Compare May 8, 2017 04:49
@codecov
Copy link

codecov bot commented May 8, 2017

Codecov Report

Merging #39 into master will decrease coverage by 0.59%.
The diff coverage is 64.61%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #39     +/-   ##
=========================================
- Coverage   83.66%   83.06%   -0.6%     
=========================================
  Files          17       17             
  Lines        2020     2085     +65     
=========================================
+ Hits         1690     1732     +42     
- Misses        233      246     +13     
- Partials       97      107     +10
Impacted Files Coverage Δ
adapters.go 100% <ø> (ø) ⬆️
dataset_update.go 71.92% <0%> (-2.62%) ⬇️
dataset_insert.go 79.36% <0%> (-2.61%) ⬇️
dataset_select.go 87.19% <0%> (-0.87%) ⬇️
dataset_delete.go 56.75% <0%> (-3.25%) ⬇️
adapters/mysql/mysql.go 100% <100%> (ø) ⬆️
dataset.go 95.48% <100%> (+0.31%) ⬆️
expressions.go 75.04% <62.5%> (-0.21%) ⬇️
default_adapter.go 82.26% <71.42%> (-0.75%) ⬇️

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 55906e6...cfba83d. Read the comment docs.

@Oscil8
Copy link
Contributor Author

Oscil8 commented May 8, 2017

Found an issue with the Travis build; see my last commit for the fix for the "vanity" package name (from the Travis docs). Wondering what you recommend on the coverage test results -- are there any particular tests you'd recommend that I'm missing?

@doug-martin
Copy link
Owner

@Oscil8 Ill take a look at this either later today or tomorrow thanks for the pr!

@Oscil8
Copy link
Contributor Author

Oscil8 commented Jun 1, 2017

Any comments @doug-martin ?

Copy link
Owner

@doug-martin doug-martin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this it looks great! Please fix the one formatting issue and Ill merge this!

@@ -217,6 +225,8 @@ func NewDefaultAdapter(ds *Dataset) Adapter {
SelectClause: default_select_clause,
DeleteClause: default_delete_clause,
TruncateClause: default_truncate_clause,
WithFragment: default_with_fragment,
RecursiveFragment: default_recursive_fragment,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting

@Oscil8
Copy link
Contributor Author

Oscil8 commented Jun 7, 2017

Addressed the formatting issue

@doug-martin doug-martin merged commit 97a59b0 into doug-martin:master Jun 12, 2017
doug-martin added a commit that referenced this pull request Jun 12, 2017
* Support for defining WITH clauses for Common Table Expressions (CTE) #39 - @Oscil8
@doug-martin doug-martin mentioned this pull request Jun 12, 2017
doug-martin added a commit that referenced this pull request Jun 12, 2017
* Support for defining WITH clauses for Common Table Expressions (CTE) #39 - @Oscil8
doug-martin added a commit that referenced this pull request Jun 12, 2017
* Support for defining WITH clauses for Common Table Expressions (CTE) #39 - @Oscil8
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.

None yet

2 participants