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

fixes #6792, use a simpler composite in condition implementation #7055

Conversation

nineinchnick
Copy link
Contributor

The (a,b) IN ((1,2)) notation is not supported by MySQL/MariaDB and SQLite, only PostgreSQL.

fixes #6792

@cebe cebe added this to the 2.0.3 milestone Jan 28, 2015
@qiangxue
Copy link
Member

The (a,b) IN ((1,2)) notation is not supported by MySQL/MariaDB

It works well for MySQL 5.1 and above. See http://sqlfiddle.com/#!2/72132/4/0
It doesn't work with SQLite, though.

@nineinchnick
Copy link
Contributor Author

Right, somehow I didn't find it in the docs before and didn't actually run the query. Should we test other databases before deciding which implementation should be default?

For reference, it's the ROW() expression syntax mentioned on the expressions manual page.

Now it occured to me that maybe a more verbose ROW() form should be used instead of () only, like `ROW(a, b) IN (ROW(1,2)). Should I make a separate PR for this?

@qiangxue
Copy link
Member

Yes, we should test other DBMS (sqlfiddle should allow us to try different DBMS, but I didn't have time to try them).

@nineinchnick
Copy link
Contributor Author

After some research, I've updated the PR and here are my findings.

Only MSSQL and SQLite doesn't support this notation.

References:

The composite IN condition for a list of values can be implemented using simple OR/AND operators. There is a problem with subqueries. It is possible, but require aliased column names to be used in the condition. Since I don't think it's possible to extract a valid alias to prepend to column names I added throwing exceptions in that case.

The solution would be to build a EXISTS (SELECT 1 FROM (subquery) a WHERE a.column1 = topQuery.column1 AND a.column2 = topQuery.column2) condition, but there's a problem with the topQuery alias. The EXISTS (subquery) is supported by both MSSQL and SQLite.

@@ -1067,22 +1067,7 @@ public function buildInCondition($operator, $operands, &$params)
}

if ($values instanceof Query) {
// sub-query
list($sql, $params) = $this->build($values, $params);
$column = (array) $column;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a bug, making the else part below never to execute. I've removed this cast.

@qiangxue
Copy link
Member

Good job!

@qiangxue qiangxue closed this in 84b20d5 Feb 1, 2015
@qiangxue
Copy link
Member

qiangxue commented Feb 1, 2015

Thank you for your excellent work!

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d87f636 on nineinchnick:composite-in-condition-fix-6792 into * on yiisoft:master*.

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.

ActiveQuery joinWith eager loading compound relation error
4 participants