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

Fix CTE query expressions #761

Merged
merged 7 commits into from
Oct 14, 2023
Merged

Fix CTE query expressions #761

merged 7 commits into from
Oct 14, 2023

Conversation

Tigrov
Copy link
Member

@Tigrov Tigrov commented Sep 28, 2023

  1. Fix bug with Oracle when CTE aliases is not quoted.
  2. Fix bug with Oracle and MSSQL: RECURSIVE expression is not needed for CTE.

Related PRs

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues -

@what-the-diff
Copy link

what-the-diff bot commented Sep 28, 2023

PR Summary

  • Update to CHANGELOG.md
    A new entry has been added to the CHANGELOG to document the changes made related to Bug Fix CTE query expressions #761. This change specifically addresses the quoting of aliases (alternative names) used in certain types of queries called "WITH" queries.

  • Improvements to withQuery method
    The withQuery method in the Query.php and QueryPartsInterface.php files now accepts either an ExpressionInterface (a designated format for expressions within the system) or a string as the alias parameter. This change will make the code more flexible in handling different inputs.

  • Enhancement of AbstractDQLQueryBuilder.php
    Two significant changes were made to this file:

    • The buildWithQueries method now includes a call to a new function quoteCteAlias which ensures that the alias of Common Table Expressions (CTE, a type of temporary result sets) is properly formatted within the query.
    • A new method quoteCteAlias has been added to handle the quoting of CTE alias.
  • Addition of Test Cases
    Several new test cases have been added to the AbstractQueryBuilderTest.php and CommonQueryTest.php files. These tests ensure that the WITH query functions as expected when using an alias, both with and without column names, and in the case of recursive CTEs.

  • New Data Provider in QueryBuilderProvider.php
    A new data provider has been added to support different cases for the cteAliases test case. This means that more diverse scenarios can be tested, ensuring robustness of the code against various use-cases.

@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (071f46a) 99.05% compared to head (d362637) 99.73%.

❗ Current head d362637 differs from pull request most recent head 93ba140. Consider uploading reports for the commit 93ba140 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #761      +/-   ##
============================================
+ Coverage     99.05%   99.73%   +0.68%     
- Complexity     1268     1272       +4     
============================================
  Files            67       63       -4     
  Lines          3061     3056       -5     
============================================
+ Hits           3032     3048      +16     
+ Misses           29        8      -21     
Files Coverage Δ
src/Query/Query.php 100.00% <100.00%> (ø)
src/QueryBuilder/AbstractDQLQueryBuilder.php 100.00% <100.00%> (ø)

... and 10 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Tigrov Tigrov marked this pull request as ready for review September 28, 2023 11:13
@Tigrov Tigrov added the type:bug Bug label Sep 29, 2023
@darkdef
Copy link
Contributor

darkdef commented Oct 8, 2023

  1. Fix bug with Oracle when CTE aliases is not quoted.

    1. Fix bug with Oracle and MSSQL: RECURSIVE expression is not needed for CTE.

Q A
Is bugfix? ✔️
New feature? ❌
Breaks BC? ❌
Fixed issues -

Please add links to db-* PR's

@Tigrov
Copy link
Member Author

Tigrov commented Oct 8, 2023

Done

@Tigrov Tigrov merged commit 4f1dbb9 into yiisoft:master Oct 14, 2023
83 of 85 checks passed
@Tigrov Tigrov deleted the fix-cte branch October 14, 2023 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants