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 notMatching for BelongsToMany with composite keys #11073

Merged
merged 1 commit into from Aug 21, 2017

Conversation

Projects
None yet
3 participants
@chinpei215
Member

chinpei215 commented Aug 21, 2017

Fixed the following error:

SQLSTATE[HY000]: General error: 1 sub-select returns 2 columns - expected 1

By the way, Sqlserver and Sqlite (< 3.15.0) don't support row values expression like (1,2) = (1,2), it will raise another error anyway. Perhaps, we could use NOT EXISTS instead of NOT IN?

@chinpei215 chinpei215 added this to the 3.5.1 milestone Aug 21, 2017

@@ -513,7 +513,7 @@ protected function _appendNotMatching($query, $options)
->andWhere(function ($exp) use ($subquery, $conds) {
$identifiers = [];
foreach (array_keys($conds) as $field) {
$identifiers = new IdentifierExpression($field);
$identifiers[] = new IdentifierExpression($field);

This comment has been minimized.

@lorenzo

lorenzo Aug 21, 2017

Member

wow, I made a sad mistake here

@lorenzo

lorenzo Aug 21, 2017

Member

wow, I made a sad mistake here

@lorenzo

This comment has been minimized.

Show comment
Hide comment
@lorenzo

lorenzo Aug 21, 2017

Member

By the way, Sqlserver and Sqlite (< 3.15.0) don't support row values expression like (1,2) = (1,2)

I created the TupleComparison class for that reason

Member

lorenzo commented Aug 21, 2017

By the way, Sqlserver and Sqlite (< 3.15.0) don't support row values expression like (1,2) = (1,2)

I created the TupleComparison class for that reason

@chinpei215

This comment has been minimized.

Show comment
Hide comment
@chinpei215

chinpei215 Aug 21, 2017

Member

I created the TupleComparison class for that reason

Oh good to hear. So if the TupleComparison could also emulate (1,2) IN (SELECT a, b FROM ...), people could execute the query for any databases.

Member

chinpei215 commented Aug 21, 2017

I created the TupleComparison class for that reason

Oh good to hear. So if the TupleComparison could also emulate (1,2) IN (SELECT a, b FROM ...), people could execute the query for any databases.

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Aug 21, 2017

Codecov Report

Merging #11073 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #11073   +/-   ##
=========================================
  Coverage     94.85%   94.85%           
  Complexity    12837    12837           
=========================================
  Files           437      437           
  Lines         32729    32729           
=========================================
  Hits          31046    31046           
  Misses         1683     1683
Impacted Files Coverage Δ Complexity Δ
src/ORM/Association/BelongsToMany.php 100% <100%> (ø) 166 <0> (ø) ⬇️

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 fffda82...e13c9fb. Read the comment docs.

codecov-io commented Aug 21, 2017

Codecov Report

Merging #11073 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #11073   +/-   ##
=========================================
  Coverage     94.85%   94.85%           
  Complexity    12837    12837           
=========================================
  Files           437      437           
  Lines         32729    32729           
=========================================
  Hits          31046    31046           
  Misses         1683     1683
Impacted Files Coverage Δ Complexity Δ
src/ORM/Association/BelongsToMany.php 100% <100%> (ø) 166 <0> (ø) ⬇️

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 fffda82...e13c9fb. Read the comment docs.

@lorenzo

This comment has been minimized.

Show comment
Hide comment
@lorenzo

lorenzo Aug 21, 2017

Member

@chinpei215 I think this is good for merging now. Do you want to try using TupleComparison in another pull request?

Member

lorenzo commented Aug 21, 2017

@chinpei215 I think this is good for merging now. Do you want to try using TupleComparison in another pull request?

@chinpei215

This comment has been minimized.

Show comment
Hide comment
@chinpei215

chinpei215 Aug 21, 2017

Member

Thank you.

Do you want to try using TupleComparison in another pull request?

I am not planning to do it at this point.

Member

chinpei215 commented Aug 21, 2017

Thank you.

Do you want to try using TupleComparison in another pull request?

I am not planning to do it at this point.

@lorenzo lorenzo merged commit 961774f into cakephp:master Aug 21, 2017

5 checks passed

codecov/patch 100% of diff hit (target 94.85%)
Details
codecov/project 94.85% (+0%) compared to fffda82
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
stickler-ci No lint errors found.

@chinpei215 chinpei215 deleted the chinpei215:fix-not-matching branch Aug 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment