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 incorrect parameters processing for JOIN subquery #133

Merged
merged 2 commits into from
Oct 10, 2023

Conversation

smelesh
Copy link
Contributor

@smelesh smelesh commented Oct 2, 2023

Fixes #117

Parameters are processed incorrectly for JOIN subquery:

  • when queryCache is enabled - ON statement should be included in query cache, otherwise parameters are missing;
  • when queryCache is disabled - JOIN statement should be parsed only once, otherwise parameters are duplicated.

@roxblnfk roxblnfk requested a review from msmakouz October 3, 2023 05:36
@smelesh
Copy link
Contributor Author

smelesh commented Oct 8, 2023

During the fix of failing test for SQLServer, I've spotted another issue - joins part must be executed exactly when it's added to the statement, otherwise an order of parameters parsed from JOIN tokens is broken.
So, I've reverted this part and only parse names and register aliases, but with a temporary QueryParameters container to avoid parameters duplication. This allows to resolve all names and aliases without affecting the result statement.

@codecov
Copy link

codecov bot commented Oct 8, 2023

Codecov Report

Merging #133 (2370e89) into 2.x (fc0c2da) will increase coverage by 0.02%.
Report is 4 commits behind head on 2.x.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                2.x     #133      +/-   ##
============================================
+ Coverage     94.68%   94.70%   +0.02%     
- Complexity     1666     1678      +12     
============================================
  Files            96       96              
  Lines          4494     4516      +22     
============================================
+ Hits           4255     4277      +22     
  Misses          239      239              
Files Coverage Δ
src/Driver/Compiler.php 96.10% <100.00%> (+0.05%) ⬆️
src/Driver/CompilerCache.php 88.96% <100.00%> (-0.08%) ⬇️
src/Driver/SQLServer/SQLServerCompiler.php 100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

@roxblnfk roxblnfk self-requested a review October 8, 2023 17:42
@roxblnfk roxblnfk merged commit 67d9b2d into cycle:2.x Oct 10, 2023
21 checks passed
@roxblnfk
Copy link
Member

Thanks!

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.

🐛 Parameters are duplicated for subquery when query cache is disabled
3 participants